Reapply make attribute collection spec serial num optional.

Pass reconfig serial num explicitly to various reconfigure member
functions since it might not be available from attribute collection spec.

3 files changed, 76 insertions, 22 deletions

diff --git a/searchcore/src/tests/proton/attribute/attribute_initializer/attribute_initializer_test.cpp b/searchcore/src/tests/proton/attribute/attribute_initializer/attribute_initializer_test.cpp
index 106bdf5cc98..52b7a7ce60a 100644
--- a/searchcore/src/tests/proton/attribute/attribute_initializer/attribute_initializer_test.cpp
+++ b/searchcore/src/tests/proton/attribute/attribute_initializer/attribute_initializer_test.cpp
@@ -95,7 +95,7 @@ struct Fixture
     vespalib::ThreadStackExecutor _executor;
     Fixture();
     ~Fixture();
-    std::unique_ptr<AttributeInitializer> createInitializer(AttributeSpec && spec, SerialNum serialNum);
+    std::unique_ptr<AttributeInitializer> createInitializer(AttributeSpec && spec, std::optional<SerialNum> serialNum);
 };
 
 Fixture::Fixture()
@@ -109,7 +109,7 @@ Fixture::Fixture()
 Fixture::~Fixture() = default;
 
 std::unique_ptr<AttributeInitializer>
-Fixture::createInitializer(AttributeSpec &&spec, SerialNum serialNum)
+Fixture::createInitializer(AttributeSpec &&spec, std::optional<SerialNum> serialNum)
 {
     return std::make_unique<AttributeInitializer>(_diskLayout->createAttributeDir(spec.getName()), "test.subdb", std::move(spec), serialNum, _factory, _executor);
 }
@@ -241,6 +241,15 @@ TEST("require that transient memory usage is reported for attribute load without
     EXPECT_EQUAL(0u, avi->get_transient_memory_usage());
 }
 
+TEST("require that saved attribute is ignored when serial num is not set")
+{
+    saveAttr("a", int32_sv, 10, 2);
+    Fixture f;
+    auto av = f.createInitializer({"a", int32_sv}, std::nullopt)->init().getAttribute();
+    EXPECT_EQUAL(0u, av->getCreateSerialNum());
+    EXPECT_EQUAL(1u, av->getNumDocs());
+}
+
 }
 
 TEST_MAIN()
diff --git a/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp b/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp
index 616bf9edf6a..29639448023 100644
--- a/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp
+++ b/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp
@@ -2,6 +2,7 @@
 
 #include <vespa/searchcore/proton/attribute/attribute_collection_spec_factory.h>
 #include <vespa/searchcore/proton/attribute/attribute_manager_initializer.h>
+#include <vespa/searchcore/proton/attribute/attribute_manager_reconfig.h>
 #include <vespa/searchcore/proton/attribute/attribute_writer.h>
 #include <vespa/searchcore/proton/attribute/attributemanager.h>
 #include <vespa/searchcore/proton/attribute/exclusive_attribute_read_accessor.h>
@@ -222,7 +223,7 @@ SequentialAttributeManager::SequentialAttributeManager(const AttributeManager &c
     : initializer(newSpec.getDocIdLimit()),
       mgr(currMgr, std::move(newSpec), initializer)
 {
-    mgr.addInitializedAttributes(initializer.getInitializedAttributes());
+    mgr.addInitializedAttributes(initializer.getInitializedAttributes(), std::nullopt, std::nullopt);
 }
 SequentialAttributeManager::~SequentialAttributeManager() = default;
 
@@ -877,6 +878,36 @@ TEST_F("transient resource usage is zero in steady state", Fixture)
     EXPECT_EQUAL(0u, usage.memory());
 }
 
+TEST_F("late create serial number is set on new attributes", Fixture)
+{
+    auto am1 = f.make_manager();
+    am1->addAttribute({"a1", INT32_SINGLE}, 4);
+    auto a1 = am1->getAttribute("a1")->getSP();
+    uint32_t docid = 0;
+    a1->addDoc(docid);
+    EXPECT_EQUAL(1u, docid);
+    a1->clearDoc(docid);
+    a1->commit(CommitParam(5));
+    AttrSpecList new_spec;
+    new_spec.emplace_back("a1", INT32_SINGLE);
+    new_spec.emplace_back("a2", INT32_SINGLE);
+    // late serial number
+    auto am2 = am1->prepare_create(AttrMgrSpec(std::move(new_spec), 10, std::nullopt))->create(14, 20);
+    auto am3 = std::dynamic_pointer_cast<AttributeManager>(am2);
+    EXPECT_TRUE(a1 == am3->getAttribute("a1")->getSP());
+    auto a2 = am3->getAttribute("a2")->getSP();
+    TEST_DO(assertCreateSerialNum(*am3, "a1", 4));
+    TEST_DO(assertCreateSerialNum(*am3, "a2", 20));
+    TEST_DO(assertShrinkTargetSerial(*am3, "a1", 3));
+    TEST_DO(assertShrinkTargetSerial(*am3, "a2", 19));
+    EXPECT_EQUAL(0u, am3->getFlushedSerialNum("a1"));
+    EXPECT_EQUAL(0u, am3->getFlushedSerialNum("a2"));
+    EXPECT_EQUAL(2u, a1->getNumDocs());
+    EXPECT_EQUAL(2u, a1->getCommittedDocIdLimit());
+    EXPECT_EQUAL(14u, a2->getNumDocs());
+    EXPECT_EQUAL(14u, a2->getCommittedDocIdLimit());
+}
+
 TEST_MAIN()
 {
     std::filesystem::remove_all(std::filesystem::path(test_dir));
diff --git a/searchcore/src/tests/proton/documentdb/configurer/configurer_test.cpp b/searchcore/src/tests/proton/documentdb/configurer/configurer_test.cpp
index db5fb1ae294..2877005e2c1 100644
--- a/searchcore/src/tests/proton/documentdb/configurer/configurer_test.cpp
+++ b/searchcore/src/tests/proton/documentdb/configurer/configurer_test.cpp
@@ -47,6 +47,7 @@ using namespace vespa::config::search;
 using namespace vespalib;
 
 using proton::matching::SessionManager;
+using search::SerialNum;
 using searchcorespi::IndexSearchable;
 using searchcorespi::index::IThreadingService;
 using proton::test::MockGidToLidChangeHandler;
@@ -165,12 +166,14 @@ struct Fixture
     void reconfigure(const DocumentDBConfig& new_config_snapshot,
                      const DocumentDBConfig& old_config_snapshot,
                      const ReconfigParams& reconfig_params,
-                     IDocumentDBReferenceResolver& resolver);
+                     IDocumentDBReferenceResolver& resolver,
+                     SerialNum serial_num);
     IReprocessingInitializer::UP reconfigure(const DocumentDBConfig& new_config_snapshot,
                                              const DocumentDBConfig& old_config_snapshot,
                                              AttributeCollectionSpec&& attr_spec,
                                              const ReconfigParams& reconfig_params,
-                                             IDocumentDBReferenceResolver& resolver);
+                                             IDocumentDBReferenceResolver& resolver,
+                                             SerialNum serial_num);
 };
 
 Fixture::Fixture()
@@ -240,10 +243,11 @@ void
 Fixture::reconfigure(const DocumentDBConfig& new_config_snapshot,
                      const DocumentDBConfig& old_config_snapshot,
                      const ReconfigParams& reconfig_params,
-                     IDocumentDBReferenceResolver& resolver)
+                     IDocumentDBReferenceResolver& resolver,
+                     SerialNum serial_num)
 {
     auto prepared_reconfig = _configurer->prepare_reconfig(new_config_snapshot, old_config_snapshot, reconfig_params);
-    _configurer->reconfigure(new_config_snapshot, old_config_snapshot, reconfig_params, resolver, *prepared_reconfig);
+    _configurer->reconfigure(new_config_snapshot, old_config_snapshot, reconfig_params, resolver, *prepared_reconfig, serial_num);
 }
 
 IReprocessingInitializer::UP
@@ -251,10 +255,11 @@ Fixture::reconfigure(const DocumentDBConfig& new_config_snapshot,
                      const DocumentDBConfig& old_config_snapshot,
                      AttributeCollectionSpec&& attr_spec,
                      const ReconfigParams& reconfig_params,
-                     IDocumentDBReferenceResolver& resolver)
+                     IDocumentDBReferenceResolver& resolver,
+                     SerialNum serial_num)
 {
     auto prepared_reconfig = _configurer->prepare_reconfig(new_config_snapshot, old_config_snapshot, reconfig_params);
-    return _configurer->reconfigure(new_config_snapshot, old_config_snapshot, std::move(attr_spec), reconfig_params, resolver, *prepared_reconfig);
+    return _configurer->reconfigure(new_config_snapshot, old_config_snapshot, std::move(attr_spec), reconfig_params, resolver, *prepared_reconfig, serial_num);
 }
 
 using MySummaryAdapter = test::MockSummaryAdapter;
@@ -325,17 +330,19 @@ struct FastAccessFixture
     IReprocessingInitializer::UP
     reconfigure(const DocumentDBConfig& new_config_snapshot,
                 const DocumentDBConfig& old_config_snapshot,
-                AttributeCollectionSpec&& attr_spec);
+                AttributeCollectionSpec&& attr_spec,
+                SerialNum serial_num);
 };
 
 IReprocessingInitializer::UP
 FastAccessFixture::reconfigure(const DocumentDBConfig& new_config_snapshot,
                                const DocumentDBConfig& old_config_snapshot,
-                               AttributeCollectionSpec&& attr_spec)
+                               AttributeCollectionSpec&& attr_spec,
+                               SerialNum serial_num)
 {
     ReconfigParams reconfig_params{CCR()};
     auto prepared_reconfig = _configurer.prepare_reconfig(new_config_snapshot, old_config_snapshot, reconfig_params);
-    return _configurer.reconfigure(new_config_snapshot, old_config_snapshot, std::move(attr_spec), *prepared_reconfig);
+    return _configurer.reconfigure(new_config_snapshot, old_config_snapshot, std::move(attr_spec), *prepared_reconfig, serial_num);
 }
 
 DocumentDBConfig::SP
@@ -500,9 +507,10 @@ TEST_F("require that we can reconfigure attribute manager", Fixture)
     ViewPtrs o = f._views.getViewPtrs();
     ReconfigParams params(CCR().setAttributesChanged(true).setSchemaChanged(true));
     // Use new config snapshot == old config snapshot (only relevant for reprocessing)
+    SerialNum reconfig_serial_num = 0;
     f.reconfigure(*createConfig(), *createConfig(),
-                  AttributeCollectionSpec(AttributeCollectionSpec::AttributeList(), 1, 0),
-                  params, f._resolver);
+                  AttributeCollectionSpec(AttributeCollectionSpec::AttributeList(), 1, reconfig_serial_num),
+                  params, f._resolver, reconfig_serial_num);
 
     ViewPtrs n = f._views.getViewPtrs();
     { // verify search view
@@ -538,9 +546,10 @@ checkAttributeWriterChangeOnRepoChange(Fixture &f, bool docTypeRepoChanged)
     auto oldAttributeWriter = getAttributeWriter(f);
     ReconfigParams params(CCR().setDocumentTypeRepoChanged(docTypeRepoChanged));
     // Use new config snapshot == old config snapshot (only relevant for reprocessing)
+    SerialNum reconfig_serial_num = 0;
     f.reconfigure(*createConfig(), *createConfig(),
-                  AttributeCollectionSpec(AttributeCollectionSpec::AttributeList(), 1, 0),
-                  params, f._resolver);
+                  AttributeCollectionSpec(AttributeCollectionSpec::AttributeList(), 1, reconfig_serial_num),
+                  params, f._resolver, reconfig_serial_num);
     auto newAttributeWriter = getAttributeWriter(f);
     if (docTypeRepoChanged) {
         EXPECT_NOT_EQUAL(oldAttributeWriter, newAttributeWriter);
@@ -558,10 +567,11 @@ TEST_F("require that we get new attribute writer if document type repo changes",
 TEST_F("require that reconfigure returns reprocessing initializer when changing attributes", Fixture)
 {
     ReconfigParams params(CCR().setAttributesChanged(true).setSchemaChanged(true));
+    SerialNum reconfig_serial_num = 0;
     IReprocessingInitializer::UP init =
         f.reconfigure(*createConfig(), *createConfig(),
-                      AttributeCollectionSpec(AttributeCollectionSpec::AttributeList(), 1, 0),
-                      params, f._resolver);
+                      AttributeCollectionSpec(AttributeCollectionSpec::AttributeList(), 1, reconfig_serial_num),
+                      params, f._resolver, reconfig_serial_num);
 
     EXPECT_TRUE(init.get() != nullptr);
     EXPECT_TRUE((dynamic_cast<AttributeReprocessingInitializer *>(init.get())) != nullptr);
@@ -571,8 +581,9 @@ TEST_F("require that reconfigure returns reprocessing initializer when changing
 TEST_F("require that we can reconfigure attribute writer", FastAccessFixture)
 {
     FastAccessFeedView::SP o = f._view._feedView.get();
+    SerialNum reconfig_serial_num = 0;
     f.reconfigure(*createConfig(), *createConfig(),
-                  AttributeCollectionSpec(AttributeCollectionSpec::AttributeList(), 1, 0));
+                  AttributeCollectionSpec(AttributeCollectionSpec::AttributeList(), 1, reconfig_serial_num), reconfig_serial_num);
     FastAccessFeedView::SP n = f._view._feedView.get();
 
     FastAccessFeedViewComparer cmp(o, n);
@@ -584,8 +595,9 @@ TEST_F("require that we can reconfigure attribute writer", FastAccessFixture)
 
 TEST_F("require that reconfigure returns reprocessing initializer", FastAccessFixture)
 {
+    SerialNum reconfig_serial_num = 0;
     IReprocessingInitializer::UP init = f.reconfigure(*createConfig(), *createConfig(),
-                                                      AttributeCollectionSpec(AttributeCollectionSpec::AttributeList(), 1, 0));
+                                                      AttributeCollectionSpec(AttributeCollectionSpec::AttributeList(), 1, reconfig_serial_num), reconfig_serial_num);
 
     EXPECT_TRUE(init.get() != nullptr);
     EXPECT_TRUE((dynamic_cast<AttributeReprocessingInitializer *>(init.get())) != nullptr);
@@ -597,7 +609,8 @@ TEST_F("require that we can reconfigure summary manager", Fixture)
     ViewPtrs o = f._views.getViewPtrs();
     ReconfigParams params(CCR().setSummaryChanged(true));
     // Use new config snapshot == old config snapshot (only relevant for reprocessing)
-    f.reconfigure(*createConfig(), *createConfig(), params, f._resolver);
+    SerialNum reconfig_serial_num = 0;
+    f.reconfigure(*createConfig(), *createConfig(), params, f._resolver, reconfig_serial_num);
 
     ViewPtrs n = f._views.getViewPtrs();
     { // verify search view
@@ -616,8 +629,9 @@ TEST_F("require that we can reconfigure matchers", Fixture)
 {
     ViewPtrs o = f._views.getViewPtrs();
     // Use new config snapshot == old config snapshot (only relevant for reprocessing)
+    SerialNum reconfig_serial_num = 0;
     f.reconfigure(*createConfig(o.fv->getSchema()), *createConfig(o.fv->getSchema()),
-                  ReconfigParams(CCR().setRankProfilesChanged(true)), f._resolver);
+                  ReconfigParams(CCR().setRankProfilesChanged(true)), f._resolver, reconfig_serial_num);
 
     ViewPtrs n = f._views.getViewPtrs();
     { // verify search view