From bd8b40d91afed3ed566677385d7c11643825e4b2 Mon Sep 17 00:00:00 2001 From: Tor Egge Date: Wed, 12 Jan 2022 10:26:52 +0100 Subject: Refactor tracking of active disk indexes. --- searchcorespi/CMakeLists.txt | 4 ++ .../tests/index/active_disk_indexes/CMakeLists.txt | 9 ++++ .../active_disk_indexes_test.cpp | 53 +++++++++++++++++++ .../tests/index/index_disk_layout/CMakeLists.txt | 9 ++++ .../index_disk_layout/index_disk_layout_test.cpp | 60 ++++++++++++++++++++++ .../searchcorespi/index/activediskindexes.cpp | 25 +++++++-- .../vespa/searchcorespi/index/activediskindexes.h | 7 ++- .../src/vespa/searchcorespi/index/index_disk_dir.h | 34 ++++++++++++ .../index/index_disk_dir_active_state.h | 22 ++++++++ .../vespa/searchcorespi/index/indexdisklayout.cpp | 21 ++++++++ .../vespa/searchcorespi/index/indexdisklayout.h | 3 ++ 11 files changed, 241 insertions(+), 6 deletions(-) create mode 100644 searchcorespi/src/tests/index/active_disk_indexes/CMakeLists.txt create mode 100644 searchcorespi/src/tests/index/active_disk_indexes/active_disk_indexes_test.cpp create mode 100644 searchcorespi/src/tests/index/index_disk_layout/CMakeLists.txt create mode 100644 searchcorespi/src/tests/index/index_disk_layout/index_disk_layout_test.cpp create mode 100644 searchcorespi/src/vespa/searchcorespi/index/index_disk_dir.h create mode 100644 searchcorespi/src/vespa/searchcorespi/index/index_disk_dir_active_state.h (limited to 'searchcorespi') 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 +#include +#include + +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 +#include +#include + +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 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 -#include +#include #include #include 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 _active; + std::map _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 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); -- cgit v1.2.3