Complete empty uploads #10608
Merged
Complete empty uploads #10608
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
zmb3
added
audit-log
greedy52
approved these changes
Feb 25, 2022
fspmarshall
approved these changes
Feb 28, 2022
zmb3
force-pushed
the
zmb3/complete-empty-uploads
branch
6 times, most recently
from
fa920dd to
f3742bd
Compare
fspmarshall
approved these changes
Mar 3, 2022
zmb3
force-pushed
the
zmb3/complete-empty-uploads
branch
from
f3742bd to
0019ff5
Compare
|
Note to myself and others, please do not squash these commits - they were intentionally rebased and separated this way because some of them need to be backported further than others. |
zmb3
force-pushed
the
zmb3/complete-empty-uploads
branch
from
0019ff5 to
7667328
Compare
|
@zmb3 Looks like this was approved, can you merge? |
Just waiting on tests to pass. Rotate trusted clusters is still a bit flaky. |
The upload completer scans for uploads that need to be completed, likely due to an error or process restart. Prior to this change, it only completed uploads that had 1 or more parts. Since completing an upload is what cleans up the directory on disk (or in the case of cloud storage, finishes the multipart upload), it was possible for us to leave behind empty directories (or multipart uploads) for uploads with no parts. This change makes it valid to complete uploads with no parts, which ensures that these directories get cleaned up. Also fix an issue with the GCS uploader, which failed to properly calculate the upload ID from the path. This is because strings.Split(s, "/") returns an empty string as the last element when s ends with a /. Updates #9646
Ensure that the logic which checks for (and emits) a session.end event also applies for windows.desktop.session.end events. For this to work, we use the StreamSessionEvents API instead of GetEvents. This is because the streaming API works for any stream of audit events, and GetSessionEvents is specific to SSH and the chunks index format used by SSH sessions. Lastly, ensure that desktop related events are also captured in the session recording stream (but omitted when streaming the events to the browser during session playback). This allows us to use the start event in order to reconstruct a missing end event.
When completing a file-based upload, open the parts files one at a time and write them to the upload, closing each file before opening the next one. This is preferrable to opening them all at once and closing all files at the end, because it consumes less file descriptors. Updates #10660
zmb3
force-pushed
the
zmb3/complete-empty-uploads
branch
from
7667328 to
ac33851
Compare
This was referenced Mar 3, 2022
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.
The upload completer scans for uploads that need to be completed,
likely due to an error or process restart. Prior to this change,
it only completed uploads that had 1 or more parts. Since completing
an upload is what cleans up the directory on disk, it was possible
for us to leave behind empty directories for uploads with no parts.
This change makes it valid to complete uploads with no parts, which
ensures that these directories get cleaned up.
In addition, the upload completer checks the upload for a session.end
event, and emits one if there is not already an end event present.
This logic only handled session.end, and not
windows.desktop.session.end, so that has been updated as well.
Updates #9646