-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
re-add grace period to Upload completer (again) #44941
Merged
Merged
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
espadolini
force-pushed
the
espadolini/upload-completer-grace-time
branch
from
August 1, 2024 13:47
4686d99
to
b9ff9d9
Compare
zmb3
approved these changes
Aug 1, 2024
espadolini
force-pushed
the
espadolini/upload-completer-grace-time
branch
from
August 1, 2024 14:44
d9375b4
to
32809ff
Compare
espadolini
force-pushed
the
espadolini/upload-completer-grace-time
branch
from
August 1, 2024 14:51
32809ff
to
3806e0a
Compare
rosstimothy
approved these changes
Aug 1, 2024
zmb3
approved these changes
Aug 1, 2024
👍 |
public-teleport-github-review-bot
bot
removed request for
gzdunek and
smallinsky
August 1, 2024 18:09
@espadolini See the table below for backport results.
|
This was referenced Aug 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
audit-log
Issues related to Teleports Audit Log
backport/branch/v14
backport/branch/v15
backport/branch/v16
size/sm
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.
When a session completes, the corresponding session tracker is put into a terminated state and its expiry is updated so that it will naturally expire shortly in the future.
Teleport's upload completer runs periodically and looks for uploads for which there is not a session tracker. If it finds any, it assumes there was an error and that no more data will be uploaded so it completes or aborts the upload.
There is a race condition here - if the TTL of the terminated session tracker is shorter than the time it takes for the session to be uploaded or if the session tracker never existed in the first place, or is yet to be created, then the upload completer may mistakenly determine that a session tracker which just recently expired (and is still being uploaded) needs to be completed, causing the upload to be cut short.
This PR re-adds a 24 hour grace time to the upload completer, similar to the one that was removed in #11551 (and re-added for Teleport v9 backwards compatibility to v10 and v11 in #12471), but with the addition of a check against the time of the last uploaded part too, which means that the upload completer will only act on uncompleted sessions that haven't had a part uploaded in 24 hours, or that have no parts and whose "upload" is over 24 hours old.
Replaces #44910
changelog: fixed race condition between session recording uploads and session recording upload cleanup