Add comments about per-reply masks.

3 files changed, 16 insertions, 1 deletions

diff --git a/storage/src/tests/persistence/mergehandlertest.cpp b/storage/src/tests/persistence/mergehandlertest.cpp
index a7f1ab5a58d..b3ec8816f1f 100644
--- a/storage/src/tests/persistence/mergehandlertest.cpp
+++ b/storage/src/tests/persistence/mergehandlertest.cpp
@@ -1303,8 +1303,12 @@ TEST_F(MergeHandlerTest, partially_filled_apply_bucket_diff_reply)
         EXPECT_EQ(2u, diff.size());
         EXPECT_EQ(EntryCheck(20000u, 4u), diff[0]._entry);
         EXPECT_EQ(EntryCheck(20100u, 4u), diff[1]._entry);
+        /*
+         * Only fill first diff entry to simulate max chunk size being exceeded
+         * when filling diff entries on source node (node 3).
+         */
         fill_entry(diff[0], *doc1, getEnv().getDocumentTypeRepo());
-        diff[0]._entry._hasMask |= 2u;
+        diff[0]._entry._hasMask |= 2u; // Simulate diff entry having been applied on node 1.
         handler.handleApplyBucketDiffReply(*reply, messageKeeper());
         LOG(debug, "handled first ApplyBucketDiffReply");
     }
diff --git a/storage/src/vespa/storage/persistence/filestorage/mergestatus.cpp b/storage/src/vespa/storage/persistence/filestorage/mergestatus.cpp
index 9b4372c73fc..97d3da28abe 100644
--- a/storage/src/vespa/storage/persistence/filestorage/mergestatus.cpp
+++ b/storage/src/vespa/storage/persistence/filestorage/mergestatus.cpp
@@ -77,6 +77,10 @@ MergeStatus::MergeStatus(const framework::Clock& clock, const metrics::LoadType&
 
 MergeStatus::~MergeStatus() = default;
 
+/*
+ * Note: hasMask parameter and _entry._hasMask in part vector are per-reply masks,
+ *       based on the nodes returned in the ApplyBucketDiffReply.
+ */
 bool
 MergeStatus::removeFromDiff(
         const std::vector<api::ApplyBucketDiffCommand::Entry>& part,
@@ -124,6 +128,11 @@ MergeStatus::removeFromDiff(
                 it = diff.erase(it);
                 altered = true;
             } else {
+                /* 
+                 * Remap from per-reply mask for the ApplyBucketDiffReply to a
+                 * per-merge-operation mask with same bit assignment as _hasMask in
+                 * the diff vector.
+                 */
                 uint16_t mask = remap_mask(it2->_entry._hasMask);
                 if (mask != it->_hasMask) {
                     // Hasmasks have changed, meaning bucket contents changed on
diff --git a/storage/src/vespa/storage/persistence/filestorage/mergestatus.h b/storage/src/vespa/storage/persistence/filestorage/mergestatus.h
index e122199c986..18ced81c280 100644
--- a/storage/src/vespa/storage/persistence/filestorage/mergestatus.h
+++ b/storage/src/vespa/storage/persistence/filestorage/mergestatus.h
@@ -32,6 +32,8 @@ public:
     ~MergeStatus();
 
     /**
+     * Note: hasMask parameter and _entry._hasMask in part vector are per-reply masks,
+     *       based on the nodes returned in ApplyBucketDiffReply.
      * @return true if any entries were removed from the internal diff
      *   or the two diffs had entries with mismatching hasmasks, which
      *   indicates that bucket contents have changed during the merge.