Add comments.

2 files changed, 58 insertions, 6 deletions

diff --git a/searchcorespi/src/tests/index/active_disk_indexes/active_disk_indexes_test.cpp b/searchcorespi/src/tests/index/active_disk_indexes/active_disk_indexes_test.cpp
index fb8498abce2..8e8e35ed194 100644
--- a/searchcorespi/src/tests/index/active_disk_indexes/active_disk_indexes_test.cpp
+++ b/searchcorespi/src/tests/index/active_disk_indexes/active_disk_indexes_test.cpp
@@ -85,6 +85,12 @@ TEST_F(ActiveDiskIndexesTest, remove_works)
 
 TEST_F(ActiveDiskIndexesTest, basic_get_transient_size_works)
 {
+    /*
+     * When starting to use a new fusion index, we have a transient
+     * period with two ISearchableIndexCollection instances:
+     * - old, containing index.fusion.1 and index.flush.2
+     * - new, containing index.fusion.2
+     */
     setActive("index.fusion.1", 1000000);
     setActive("index.flush.2", 500000);
     setActive("index.fusion.2", 1200000);
@@ -92,6 +98,10 @@ TEST_F(ActiveDiskIndexesTest, basic_get_transient_size_works)
     auto flush2 = get_index_disk_dir("index.flush.2");
     auto fusion2 = get_index_disk_dir("index.fusion.2");
     {
+        /*
+         * When using the old index collection, disk space used by
+         * index.fusion.2 is considered transient.
+         */
         SCOPED_TRACE("index.fusion.1");
         assert_transient_size(1200000, fusion1);
     }
@@ -100,29 +110,50 @@ TEST_F(ActiveDiskIndexesTest, basic_get_transient_size_works)
         assert_transient_size(0, flush2);
     }
     {
+        /*
+         * When using the new index collection, disk space used by
+         * index.fusion.1 and index.flush.2 is considered transient.
+         */
         SCOPED_TRACE("index.fusion.2");
         assert_transient_size(1500000, fusion2);
     }
-    notActive("index.fusion.2");
+    notActive("index.fusion.1");
+    notActive("index.flush.2");
     {
-        SCOPED_TRACE("index.fusion.1 after remove of index.fusion.2");
-        assert_transient_size(0, fusion1);
+        /*
+         * old index collection removed.
+         */
+        SCOPED_TRACE("index.fusion.2 after remove of index.fusion.1 and index.flush.1");
+        assert_transient_size(0, fusion2);
     }
 }
 
-TEST_F(ActiveDiskIndexesTest, dynamic_get_transient_size_works)
+TEST_F(ActiveDiskIndexesTest, get_transient_size_during_ongoing_fusion)
 {
+    /*
+     * During ongoing fusion, we have one ISearchableIndexCollection instance:
+     * - old, containing index.fusion.1 and index.flush.2
+     *
+     * Fusion output directory is index.fusion.2
+     */
     setActive("index.fusion.1", 1000000);
+    setActive("index.flush.2", 500000);
     auto fusion1 = get_index_disk_dir("index.fusion.1");
     auto fusion2 = get_index_disk_dir("index.fusion.2");
-    add_not_active(fusion2);
+    add_not_active(fusion2); // start tracking disk space for fusion output
     {
+        /*
+         * Fusion not yet started.
+         */
         SCOPED_TRACE("dir missing");
         assert_transient_size(0, fusion1);
     }
     auto dir = base_dir + "/index.fusion.2";
     vespalib::mkdir(dir, true);
     {
+        /*
+         * Fusion started, but no files written yet.
+         */
         SCOPED_TRACE("empty dir");
         assert_transient_size(0, fusion1);
     }
@@ -136,11 +167,17 @@ TEST_F(ActiveDiskIndexesTest, dynamic_get_transient_size_works)
         ostr.close();
     }
     {
+        /*
+         * Fusion started, one file written.
+         */
         SCOPED_TRACE("single file");
         assert_transient_size((seek_pos + block_size) / block_size * block_size, fusion1);
     }
-    EXPECT_TRUE(remove(fusion2));
+    EXPECT_TRUE(remove(fusion2)); // stop tracking disk space for fusion output
     {
+        /*
+         * Fusion aborted.
+         */
         SCOPED_TRACE("removed");
         assert_transient_size(0, fusion1);
     }
diff --git a/searchcorespi/src/vespa/searchcorespi/index/activediskindexes.cpp b/searchcorespi/src/vespa/searchcorespi/index/activediskindexes.cpp
index 368df1e6da3..b2623d04b08 100644
--- a/searchcorespi/src/vespa/searchcorespi/index/activediskindexes.cpp
+++ b/searchcorespi/src/vespa/searchcorespi/index/activediskindexes.cpp
@@ -80,6 +80,10 @@ ActiveDiskIndexes::remove(IndexDiskDir index_disk_dir)
 uint64_t
 ActiveDiskIndexes::get_transient_size(IndexDiskLayout& layout, IndexDiskDir index_disk_dir) const
 {
+    /*
+     * Only report transient size related to a valid fusion index. This ensures
+     * that transient size is reported once per index collection.
+     */
     if (!index_disk_dir.valid() || !index_disk_dir.is_fusion_index()) {
         return 0u;
     }
@@ -89,11 +93,22 @@ ActiveDiskIndexes::get_transient_size(IndexDiskLayout& layout, IndexDiskDir inde
         std::lock_guard lock(_lock);
         for (auto &entry : _active) {
             if (entry.first < index_disk_dir) {
+                /*
+                 * Indexes before current fusion index are on the way out and
+                 * will be removed when all older index collections
+                 * referencing them are destroyed. Disk space used by these
+                 * indexes is considered transient.
+                 */
                 if (entry.second.get_size_on_disk().has_value()) {
                     transient_size += entry.second.get_size_on_disk().value();
                 }
             }
             if (index_disk_dir < entry.first && entry.first.is_fusion_index()) {
+                /*
+                 * Fusion indexes after current fusion index can be partially
+                 * complete and might be removed if fusion is aborted. Disk
+                 * space used by these indexes is consider transient.
+                 */
                 if (entry.second.get_size_on_disk().has_value()) {
                     transient_size += entry.second.get_size_on_disk().value();
                 } else {