Test operation batch barrier with different lock modes

2 files changed, 20 insertions, 1 deletions

diff --git a/storage/src/tests/persistence/persistencequeuetest.cpp b/storage/src/tests/persistence/persistencequeuetest.cpp
index 746ae770cb3..e96ad013923 100644
--- a/storage/src/tests/persistence/persistencequeuetest.cpp
+++ b/storage/src/tests/persistence/persistencequeuetest.cpp
@@ -22,6 +22,7 @@ public:
     void exclusive_locked_operation_not_started_if_shared_op_active();
     void shared_locked_operation_not_started_if_exclusive_op_active();
     void exclusive_locked_operation_not_started_if_exclusive_op_active();
+    void operation_batching_not_allowed_across_different_lock_modes();
 
     std::shared_ptr<api::StorageMessage> createPut(uint64_t bucket, uint64_t docIdx);
     std::shared_ptr<api::StorageMessage> createGet(uint64_t bucket) const;
@@ -34,6 +35,7 @@ public:
     CPPUNIT_TEST(exclusive_locked_operation_not_started_if_shared_op_active);
     CPPUNIT_TEST(shared_locked_operation_not_started_if_exclusive_op_active);
     CPPUNIT_TEST(exclusive_locked_operation_not_started_if_exclusive_op_active);
+    CPPUNIT_TEST(operation_batching_not_allowed_across_different_lock_modes);
     CPPUNIT_TEST_SUITE_END();
 
     struct Fixture {
@@ -184,4 +186,19 @@ void PersistenceQueueTest::exclusive_locked_operation_not_started_if_exclusive_o
     CPPUNIT_ASSERT(!lock1.first.get());
 }
 
+void PersistenceQueueTest::operation_batching_not_allowed_across_different_lock_modes() {
+    Fixture f(*this);
+
+    f.filestorHandler->schedule(createPut(1234, 0), _disk);
+    f.filestorHandler->schedule(createGet(1234), _disk);
+
+    auto lock0 = f.filestorHandler->getNextMessage(_disk, f.stripeId);
+    CPPUNIT_ASSERT(lock0.first);
+    CPPUNIT_ASSERT(lock0.second);
+    CPPUNIT_ASSERT_EQUAL(api::LockingRequirements::Exclusive, lock0.first->lockingRequirements());
+
+    f.filestorHandler->getNextMessage(_disk, f.stripeId, lock0);
+    CPPUNIT_ASSERT(!lock0.second);
+}
+
 } // namespace storage
diff --git a/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.cpp b/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.cpp
index 1cae13ea95c..f9571228ef9 100644
--- a/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.cpp
+++ b/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.cpp
@@ -959,7 +959,9 @@ FileStorHandlerImpl::Stripe::getNextMessage(FileStorHandler::LockedMessage& lck)
     }
 
     api::StorageMessage & m(*range.first->_command);
-    // We don't allow batching of operations across lock requirement modes.
+    // For now, don't allow batching of operations across lock requirement modes.
+    // We might relax this requirement later once we're 100% sure it can't trigger
+    // any unfortunate edge cases.
     if (lck.first->lockingRequirements() != m.lockingRequirements()) {
         lck.second.reset();
         return lck;