Refactor tracking of active disk indexes.

11 files changed, 241 insertions, 6 deletions

diff --git a/searchcorespi/CMakeLists.txt b/searchcorespi/CMakeLists.txt
index 66ff3fff0f2..1029619150a 100644
--- a/searchcorespi/CMakeLists.txt
+++ b/searchcorespi/CMakeLists.txt
@@ -18,4 +18,8 @@ vespa_define_module(
     src/vespa/searchcorespi
     src/vespa/searchcorespi/flush
     src/vespa/searchcorespi/index
+
+    TESTS
+    src/tests/index/active_disk_indexes
+    src/tests/index/index_disk_layout
 )
diff --git a/searchcorespi/src/tests/index/active_disk_indexes/CMakeLists.txt b/searchcorespi/src/tests/index/active_disk_indexes/CMakeLists.txt
new file mode 100644
index 00000000000..e10ada381bf
--- /dev/null
+++ b/searchcorespi/src/tests/index/active_disk_indexes/CMakeLists.txt
@@ -0,0 +1,9 @@
+# Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+vespa_add_executable(searchcorespi_active_disk_indexes_test_app
+    SOURCES
+    active_disk_indexes_test.cpp
+    DEPENDS
+    searchcorespi
+    GTest::GTest
+)
+vespa_add_test(NAME searchcorespi_active_disk_indexes_test_app COMMAND searchcorespi_active_disk_indexes_test_app)
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
new file mode 100644
index 00000000000..a6d412817d9
--- /dev/null
+++ b/searchcorespi/src/tests/index/active_disk_indexes/active_disk_indexes_test.cpp
@@ -0,0 +1,53 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#include <vespa/searchcorespi/index/activediskindexes.h>
+#include <vespa/searchcorespi/index/index_disk_dir.h>
+#include <vespa/vespalib/gtest/gtest.h>
+
+namespace searchcorespi::index {
+
+class ActiveDiskIndexesTest : public ::testing::Test,
+                              public ActiveDiskIndexes
+{
+protected:
+    ActiveDiskIndexesTest();
+    ~ActiveDiskIndexesTest();
+};
+
+ActiveDiskIndexesTest::ActiveDiskIndexesTest()
+    : ::testing::Test(),
+      ActiveDiskIndexes()
+{
+}
+
+ActiveDiskIndexesTest::~ActiveDiskIndexesTest() = default;
+
+TEST_F(ActiveDiskIndexesTest, simple_set_active_works)
+{
+    EXPECT_FALSE(isActive("index.flush.1"));
+    setActive("index.flush.1");
+    EXPECT_TRUE(isActive("index.flush.1"));
+    notActive("index.flush.1");
+    EXPECT_FALSE(isActive("index.flush.1"));
+}
+
+TEST_F(ActiveDiskIndexesTest, nested_set_active_works)
+{
+    setActive("index.flush.1");
+    setActive("index.flush.1");
+    EXPECT_TRUE(isActive("index.flush.1"));
+    notActive("index.flush.1");
+    EXPECT_TRUE(isActive("index.flush.1"));
+    notActive("index.flush.1");
+    EXPECT_FALSE(isActive("index.flush.1"));
+}
+
+TEST_F(ActiveDiskIndexesTest, is_active_returns_false_for_bad_name)
+{
+    EXPECT_FALSE(isActive("foo/bar/baz"));
+    EXPECT_FALSE(isActive("index.flush.0"));
+}
+
+}
+
+GTEST_MAIN_RUN_ALL_TESTS()
diff --git a/searchcorespi/src/tests/index/index_disk_layout/CMakeLists.txt b/searchcorespi/src/tests/index/index_disk_layout/CMakeLists.txt
new file mode 100644
index 00000000000..4e82cf1b9d2
--- /dev/null
+++ b/searchcorespi/src/tests/index/index_disk_layout/CMakeLists.txt
@@ -0,0 +1,9 @@
+# Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+vespa_add_executable(searchcorespi_index_disk_layout_test_app
+    SOURCES
+    index_disk_layout_test.cpp
+    DEPENDS
+    searchcorespi
+    GTest::GTest
+)
+vespa_add_test(NAME searchcorespi_index_disk_layout_test_app COMMAND searchcorespi_index_disk_layout_test_app)
diff --git a/searchcorespi/src/tests/index/index_disk_layout/index_disk_layout_test.cpp b/searchcorespi/src/tests/index/index_disk_layout/index_disk_layout_test.cpp
new file mode 100644
index 00000000000..e35225b2745
--- /dev/null
+++ b/searchcorespi/src/tests/index/index_disk_layout/index_disk_layout_test.cpp
@@ -0,0 +1,60 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#include <vespa/searchcorespi/index/indexdisklayout.h>
+#include <vespa/searchcorespi/index/index_disk_dir.h>
+#include <vespa/vespalib/gtest/gtest.h>
+
+namespace searchcorespi::index {
+
+namespace {
+
+void expect_index_disk_dir(IndexDiskDir exp, const vespalib::string& dir)
+{
+    auto act = IndexDiskLayout::get_index_disk_dir(dir);
+    ASSERT_TRUE(act.valid());
+    ASSERT_EQ(exp, act);
+}
+
+void expect_bad_index_disk_dir(const vespalib::string& dir)
+{
+    auto act = IndexDiskLayout::get_index_disk_dir(dir);
+    ASSERT_FALSE(act.valid());
+}
+
+}
+
+TEST(IndexDiskLayoutTest, get_index_disk_dir_works)
+{
+    {
+        SCOPED_TRACE("index.fusion.1");
+        expect_index_disk_dir(IndexDiskDir(1, true), "index.fusion.1");
+    }
+    {
+        SCOPED_TRACE("index.flush.2");
+        expect_index_disk_dir(IndexDiskDir(2, false), "index.flush.2");
+    }
+    {
+        SCOPED_TRACE("index.flush.3");
+        expect_index_disk_dir(IndexDiskDir(3, false), "index.flush.3");
+    }
+    {
+        SCOPED_TRACE("foo/bar/index.flush.4");
+        expect_index_disk_dir(IndexDiskDir(4, false), "foo/bar/index.flush.4");
+    }
+    {
+        SCOPED_TRACE("index.flush.");
+        expect_bad_index_disk_dir("index.flush.");
+    }
+    {
+        SCOPED_TRACE("index.flush.0");
+        expect_bad_index_disk_dir("index.flush.0");
+    }
+    {
+        SCOPED_TRACE("asdf");
+        expect_bad_index_disk_dir("asdf");
+    }
+}
+
+}
+
+GTEST_MAIN_RUN_ALL_TESTS()
diff --git a/searchcorespi/src/vespa/searchcorespi/index/activediskindexes.cpp b/searchcorespi/src/vespa/searchcorespi/index/activediskindexes.cpp
index c7891709801..fb9585fb58e 100644
--- a/searchcorespi/src/vespa/searchcorespi/index/activediskindexes.cpp
+++ b/searchcorespi/src/vespa/searchcorespi/index/activediskindexes.cpp
@@ -1,6 +1,9 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "activediskindexes.h"
+#include "indexdisklayout.h"
+#include "index_disk_dir.h"
+#include "index_disk_dir_active_state.h"
 #include <cassert>
 
 using vespalib::string;
@@ -11,20 +14,34 @@ ActiveDiskIndexes::ActiveDiskIndexes() = default;
 ActiveDiskIndexes::~ActiveDiskIndexes() = default;
 
 void ActiveDiskIndexes::setActive(const string &index) {
+    auto index_disk_dir = IndexDiskLayout::get_index_disk_dir(index);
+    assert(index_disk_dir.valid());
     std::lock_guard lock(_lock);
-    _active.insert(index);
+    auto insres = _active.insert(std::make_pair(index_disk_dir, IndexDiskDirActiveState()));
+    insres.first->second.activate();
 }
 
 void ActiveDiskIndexes::notActive(const string & index) {
+    auto index_disk_dir = IndexDiskLayout::get_index_disk_dir(index);
+    assert(index_disk_dir.valid());
     std::lock_guard lock(_lock);
-    auto it = _active.find(index);
+    auto it = _active.find(index_disk_dir);
     assert(it != _active.end());
-    _active.erase(it);
+    assert(it->second.is_active());
+    it->second.deactivate();
+    if (!it->second.is_active()) {
+        _active.erase(it);
+    }
 }
 
 bool ActiveDiskIndexes::isActive(const string &index) const {
+    auto index_disk_dir = IndexDiskLayout::get_index_disk_dir(index);
+    if (!index_disk_dir.valid()) {
+        return false;
+    }
     std::lock_guard lock(_lock);
-    return _active.find(index) != _active.end();
+    auto it = _active.find(index_disk_dir);
+    return (it != _active.end()) && it->second.is_active();
 }
 
 }
diff --git a/searchcorespi/src/vespa/searchcorespi/index/activediskindexes.h b/searchcorespi/src/vespa/searchcorespi/index/activediskindexes.h
index a63dbb8b2a7..365025e7450 100644
--- a/searchcorespi/src/vespa/searchcorespi/index/activediskindexes.h
+++ b/searchcorespi/src/vespa/searchcorespi/index/activediskindexes.h
@@ -3,18 +3,21 @@
 #pragma once
 
 #include <vespa/vespalib/stllike/string.h>
-#include <set>
+#include <map>
 #include <mutex>
 #include <memory>
 
 namespace searchcorespi::index {
 
+class IndexDiskDir;
+class IndexDiskDirActiveState;
+
 /**
  * Class used to keep track of the set of active disk indexes in an index maintainer.
  * The index directories are used as identifiers.
  */
 class ActiveDiskIndexes {
-    std::multiset<vespalib::string> _active;
+    std::map<IndexDiskDir, IndexDiskDirActiveState> _active;
     mutable std::mutex _lock;
 
 public:
diff --git a/searchcorespi/src/vespa/searchcorespi/index/index_disk_dir.h b/searchcorespi/src/vespa/searchcorespi/index/index_disk_dir.h
new file mode 100644
index 00000000000..278fd73e555
--- /dev/null
+++ b/searchcorespi/src/vespa/searchcorespi/index/index_disk_dir.h
@@ -0,0 +1,34 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+#pragma once
+
+namespace searchcorespi::index {
+
+/*
+ * Class naming a disk index for a document type.
+ */
+class IndexDiskDir {
+    uint32_t _id;
+    bool     _fusion;
+public:
+    IndexDiskDir(uint32_t id, bool fusion) noexcept
+        : _id(id),
+          _fusion(fusion)
+    {
+    }
+    IndexDiskDir() noexcept
+        : IndexDiskDir(0, false)
+    {
+    }
+    bool operator<(const IndexDiskDir& rhs) const noexcept {
+        if (_id != rhs._id) {
+            return _id < rhs._id;
+        }
+        return !_fusion && rhs._fusion;
+    }
+    bool operator==(const IndexDiskDir& rhs) const noexcept {
+        return (_id == rhs._id) && (_fusion == rhs._fusion);
+    }
+    bool valid() const noexcept { return _id != 0u; }
+};
+
+}
diff --git a/searchcorespi/src/vespa/searchcorespi/index/index_disk_dir_active_state.h b/searchcorespi/src/vespa/searchcorespi/index/index_disk_dir_active_state.h
new file mode 100644
index 00000000000..94363b0a113
--- /dev/null
+++ b/searchcorespi/src/vespa/searchcorespi/index/index_disk_dir_active_state.h
@@ -0,0 +1,22 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+#pragma once
+
+namespace searchcorespi::index {
+
+/*
+ * Class describing active state for a disk index directory.
+ */
+class IndexDiskDirActiveState {
+    uint32_t _active_count;
+public:
+    IndexDiskDirActiveState()
+        : _active_count(0)
+    {
+    }
+
+    void activate() noexcept { ++_active_count; }
+    void deactivate() noexcept { --_active_count; }
+    bool is_active() const noexcept { return _active_count != 0; }
+};
+
+}
diff --git a/searchcorespi/src/vespa/searchcorespi/index/indexdisklayout.cpp b/searchcorespi/src/vespa/searchcorespi/index/indexdisklayout.cpp
index a13633751bb..c701d1dfb1d 100644
--- a/searchcorespi/src/vespa/searchcorespi/index/indexdisklayout.cpp
+++ b/searchcorespi/src/vespa/searchcorespi/index/indexdisklayout.cpp
@@ -1,6 +1,7 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "indexdisklayout.h"
+#include "index_disk_dir.h"
 #include <sstream>
 
 namespace searchcorespi::index {
@@ -53,4 +54,24 @@ IndexDiskLayout::getSelectorFileName(const vespalib::string &dir)
     return dir + "/selector";
 }
 
+IndexDiskDir
+IndexDiskLayout::get_index_disk_dir(const vespalib::string& dir)
+{
+    auto name = dir.substr(dir.rfind('/') + 1);
+    const vespalib::string* prefix = nullptr;
+    bool fusion = false;
+    if (name.find(FlushDirPrefix) == 0) {
+        prefix = &FlushDirPrefix;
+    } else if (name.find(FusionDirPrefix) == 0) {
+        prefix = &FusionDirPrefix;
+        fusion = true;
+    } else {
+        return IndexDiskDir(); // invalid
+    }
+    std::istringstream ist(name.substr(prefix->size()));
+    uint32_t id = 0;
+    ist >> id;
+    return IndexDiskDir(id, fusion); // invalid if id == 0u
+}
+
 }
diff --git a/searchcorespi/src/vespa/searchcorespi/index/indexdisklayout.h b/searchcorespi/src/vespa/searchcorespi/index/indexdisklayout.h
index 0598ad17c8a..94b35936cc7 100644
--- a/searchcorespi/src/vespa/searchcorespi/index/indexdisklayout.h
+++ b/searchcorespi/src/vespa/searchcorespi/index/indexdisklayout.h
@@ -6,6 +6,8 @@
 namespace searchcorespi {
 namespace index {
 
+class IndexDiskDir;
+
 /**
  * Utility class used to get static aspects of the disk layout (i.e directory and file names)
  * needed by the index maintainer.
@@ -23,6 +25,7 @@ public:
     IndexDiskLayout(const vespalib::string &baseDir);
     vespalib::string getFlushDir(uint32_t sourceId) const;
     vespalib::string getFusionDir(uint32_t sourceId) const;
+    static IndexDiskDir get_index_disk_dir(const vespalib::string& dir);
 
     static vespalib::string getSerialNumFileName(const vespalib::string &dir);
     static vespalib::string getSchemaFileName(const vespalib::string &dir);