[v18] Fixing blocking behavior when an encrypted recording upload fails #61774
Conversation
github-actions
bot
added
audit-log
lib/events/filesessions/fileasync.go
Outdated
| u.wg.Add(1) | ||
| go func() { | ||
| defer u.wg.Done() | ||
| u.wg.Go(func() { |
There was a problem hiding this comment.
This change needs to be reverted. branch/v18 is still using Go 1.24 which doesn't have waitgroup.Go :(.
There was a problem hiding this comment.
synctest was a pain too 😅 Ended up rolling those tests back to using fake clocks. I noticed we have a build tag for using synctest, but do those tests actually run anywhere?
There was a problem hiding this comment.
Yes we run them in CI. I think we should try to use synctest here too. You might benefit from #61783.
There was a problem hiding this comment.
Went back to synctest and got everything working again with the build tag. Looks like it's not quite a 1:1 in terms of behavior between the experimental version in 1.24 and 1.25. I had to initialize the uploader outside of the bubble in order to get synctest.Wait() not to hang 🤔 Also had to move some things shared between fileasync_test.go and fileasync_chaos_test.go into a helpers_test.go to prevent the enablesynctest tag from breaking untagged tests.
eriktate
force-pushed
the
bot/backport-61579-branch/v18
branch
3 times, most recently
from
cc6e077 to
b89f679
Compare
eriktate
force-pushed
the
bot/backport-61579-branch/v18
branch
from
b89f679 to
6400c6d
Compare
…g semaphore, and preventing early termination of the uploader on failed uploads
eriktate
force-pushed
the
bot/backport-61579-branch/v18
branch
from
6400c6d to
5635bb3
Compare
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Backport #61579 to branch/v18
changelog: Fixed an issue that caused a failed upload of an encrypted session recording to block other recordings from uploading