-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: improve shut down logic #89
Merged
domdom82
merged 5 commits into
cloudfoundry:main
from
sap-contributions:fix-agent-shutdown2
Jul 20, 2023
Merged
fix: improve shut down logic #89
domdom82
merged 5 commits into
cloudfoundry:main
from
sap-contributions:fix-agent-shutdown2
Jul 20, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maxmoehl
force-pushed
the
fix-agent-shutdown2
branch
from
July 19, 2023 13:15
359820f
to
0ffed35
Compare
maxmoehl
changed the title
fix: allow agent to shut down quickly
fix: improve shut down logic
Jul 19, 2023
maxmoehl
force-pushed
the
fix-agent-shutdown2
branch
from
July 19, 2023 13:22
4277ac7
to
cffa330
Compare
domdom82
previously approved these changes
Jul 19, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approving for ci
In any case we will wait for #35 and I will rebase once that's in. |
peanball
previously approved these changes
Jul 19, 2023
b1tamara
previously approved these changes
Jul 19, 2023
To be able to notice that the context has been canceled we rely on at least one more packet being read. In practice this should be fine but especially during testing it can cause the agent to hang because no more data is captured. This commit configures a timeout of one second for reading a single packet. This ensures that we check at least once per second if the capture has been stopped and allow the agent to stop capturing even if no further packets are captured. Co-Authored-By: Dominik Froehlich <dominik.froehlich@sap.com>
The API has the processing of messages received from the agent and the sending of control messages to the agent in one function `readMsgFromStream` which causes concurrency issues because we rely on the blocking read call from becoming unblocked in order to tell the agent to shut down. This, however, prevents the agent from ever shutting down essentially making it impossible to stop a capture unless there is a constant flow of packets. This commit separates the upstream and downstream flows by creating two separate routines when starting a capture on an agent. It also fixes smaller issues like misleading names and sending to closed channels.
maxmoehl
force-pushed
the
fix-agent-shutdown2
branch
from
July 19, 2023 16:49
d830609
to
9ca5093
Compare
domdom82
approved these changes
Jul 20, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See individual commits for details.