loop shutdown should not call CancelRequest for inflight requests because it causes a race with the code that's executing those requests, we should simply wait until those requests complete by themselves #1193
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
qkrorlqr
added
large-tests
librarian
added
rebase
qkrorlqr
added
the
rebase
qkrorlqr
force-pushed
the
users/qkrorlqr/filestore-proper-graceful-shutdown
branch
from
d3384ce
to
642be6e
Compare
|
Note This is an automated comment that will be appended during run. 🔴 linux-x86_64-release-tsan: some tests FAILED for commit 642be6e.
🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 642be6e.
|
qkrorlqr
added
asan
…ause it causes a race with the code that's executing those requests, we should simply wait until those requests complete by themselves
…ug in TBootstrap in fs_ut.cpp (Apply on a callback which returns TFuture<void> discards the future returned by the callback!)
qkrorlqr
force-pushed
the
users/qkrorlqr/filestore-proper-graceful-shutdown
branch
from
519a647
to
e3d4cd0
Compare
librarian
removed
the
rebase
qkrorlqr
added
tsan
|
Note This is an automated comment that will be appended during run. 🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit e3d4cd0.
🔴 linux-x86_64-release-tsan: some tests FAILED for commit e3d4cd0.
|
qkrorlqr
removed
the
tsan
|
Note This is an automated comment that will be appended during run. 🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 98c1958.
🔴 linux-x86_64-release-tsan: some tests FAILED for commit 98c1958.
|
…f only when ShouldStop flag is set
qkrorlqr
added
tsan
|
Note This is an automated comment that will be appended during run. 🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit e074cae.
🔴 linux-x86_64-release-tsan: some tests FAILED for commit e074cae.
|
qkrorlqr
added
tsan
|
Note This is an automated comment that will be appended during run. 🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit e074cae.
🟢 linux-x86_64-release-tsan: all tests PASSED for commit e074cae.
|
…together atomically
|
Note This is an automated comment that will be appended during run. 🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 0ad0e2c.
🟢 linux-x86_64-release-tsan: all tests PASSED for commit 0ad0e2c.
|
yegorskii
approved these changes
May 21, 2024
debnatkh
approved these changes
May 21, 2024
qkrorlqr
added a commit
that referenced
this pull request
May 23, 2024
…ause it causes a race with the code that's executing those requests, we should simply wait until those requests complete by themselves (#1193) * loop shutdown should not call CancelRequest for inflight requests because it causes a race with the code that's executing those requests, we should simply wait until those requests complete by themselves * CompletionQueue shutdown should actually be async + fixed StopAsync bug in TBootstrap in fs_ut.cpp (Apply on a callback which returns TFuture<void> discards the future returned by the callback!) * loop shutdown: SuspendAsync ut + some tmp dbg output * loop shutdown: more tmp dbg output * loop shutdown: more tmp dbg output * loop shutdown: outputting dbg stuff under a lock, outputting dbg stuff only when ShouldStop flag is set * loop shutdown: fixed CompletingCount vs Requests.empty() checks race * loop shutdown: inflight count and completing count should be checked together atomically
qkrorlqr
added a commit
that referenced
this pull request
May 24, 2024
…, filestore CompactionMap empty entry cleanup (#1266) * loop shutdown should not call CancelRequest for inflight requests because it causes a race with the code that's executing those requests, we should simply wait until those requests complete by themselves (#1193) * loop shutdown should not call CancelRequest for inflight requests because it causes a race with the code that's executing those requests, we should simply wait until those requests complete by themselves * CompletionQueue shutdown should actually be async + fixed StopAsync bug in TBootstrap in fs_ut.cpp (Apply on a callback which returns TFuture<void> discards the future returned by the callback!) * loop shutdown: SuspendAsync ut + some tmp dbg output * loop shutdown: more tmp dbg output * loop shutdown: more tmp dbg output * loop shutdown: outputting dbg stuff under a lock, outputting dbg stuff only when ShouldStop flag is set * loop shutdown: fixed CompletingCount vs Requests.empty() checks race * loop shutdown: inflight count and completing count should be checked together atomically * issue-1128: entries in the CompactionMap table should be deleted instead of updating of both BlobCount and DeletionCount are 0 (#1260) * filestore-vhost loop shutdown logging (#1265) * filestore-vhost loop shutdown logging * filestore-vhost loop shutdown logging - fix
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.
Data race example:
Current code calls CancelRequest for each inflight request upon loop shutdown/suspend. But inflight requests are flying all over the system and are being accessed by other threads. We have to wait till those requests complete, we can't cancel them with our current code architecture.
#1307