Ensure executor is synced after shutdown

Also clarify/update some comments.

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();
 }
 
 }