diff options
author | Tor Brede Vekterli <vekterli@yahooinc.com> | 2023-06-05 11:41:32 +0000 |
---|---|---|
committer | Tor Brede Vekterli <vekterli@yahooinc.com> | 2023-06-05 11:41:32 +0000 |
commit | 12ad5de35e58497abb8940940499a13a2ca97d6f (patch) | |
tree | 0d6b4e51489d26c79c7d75bff2e5c823266eb104 /storage | |
parent | b9585a91ac153ba1111579430f66e5acf0a37281 (diff) |
Ensure executor is synced after shutdown
Also clarify/update some comments.
Diffstat (limited to 'storage')
-rw-r--r-- | storage/src/tests/storageserver/changedbucketownershiphandlertest.cpp | 2 | ||||
-rw-r--r-- | storage/src/vespa/storage/storageserver/changedbucketownershiphandler.cpp | 12 |
2 files changed, 8 insertions, 6 deletions
diff --git a/storage/src/tests/storageserver/changedbucketownershiphandlertest.cpp b/storage/src/tests/storageserver/changedbucketownershiphandlertest.cpp index 0d60f50a779..ae2385a36d8 100644 --- a/storage/src/tests/storageserver/changedbucketownershiphandlertest.cpp +++ b/storage/src/tests/storageserver/changedbucketownershiphandlertest.cpp @@ -69,7 +69,7 @@ struct ChangedBucketOwnershipHandlerTest : Test { std::shared_ptr<AbortBucketOperationsCommand> fetch_dispatched_abort_operations_command() { _bottom->waitForMessages(2, 60); // abort cmd + set cluster state cmd - EXPECT_EQ(2, _bottom->getNumCommands()); // always _at least_ 2 + EXPECT_EQ(2, _bottom->getNumCommands()); return std::dynamic_pointer_cast<AbortBucketOperationsCommand>(_bottom->getCommand(0)); } diff --git a/storage/src/vespa/storage/storageserver/changedbucketownershiphandler.cpp b/storage/src/vespa/storage/storageserver/changedbucketownershiphandler.cpp index 84d12c4e85f..45e991f9ebc 100644 --- a/storage/src/vespa/storage/storageserver/changedbucketownershiphandler.cpp +++ b/storage/src/vespa/storage/storageserver/changedbucketownershiphandler.cpp @@ -263,10 +263,12 @@ public: void run() override { OwnershipState::CSP old_ownership; OwnershipState::CSP new_ownership; - // Get old state and update own current cluster state _before_ it is - // applied to the rest of the system. This helps ensure that no message - // can get through in the off-case that the lower level storage links - // don't apply the state immediately for some reason. + // Update the ownership state inspected by all bucket-mutating operations passing through + // this component so that messages from outdated distributors will be rejected. Note that + // this is best-effort; with our current multitude of RPC threads directly dispatching + // operations into the persistence provider, it's possible for a thread carrying an outdated + // operation to have already passed the barrier, but be preempted so that it will apply the + // op _after_ the abort step has completed. { std::lock_guard guard(_owner._stateLock); old_ownership = _owner._currentOwnership; @@ -477,7 +479,7 @@ ChangedBucketOwnershipHandler::onInternalReply( void ChangedBucketOwnershipHandler::onClose() { - _state_sync_executor.shutdown(); + _state_sync_executor.shutdown().sync(); } } |