Merge pull request #2227 from yahoo/geirst/only-update-maintenance-controller-if-config-has-changed

Geirst/only update maintenance controller if config has changed

7 files changed, 123 insertions, 54 deletions

diff --git a/searchcore/src/tests/proton/documentdb/configurer/configurer_test.cpp b/searchcore/src/tests/proton/documentdb/configurer/configurer_test.cpp
index 0cd11a9f9c4..7d7c3604feb 100644
--- a/searchcore/src/tests/proton/documentdb/configurer/configurer_test.cpp
+++ b/searchcore/src/tests/proton/documentdb/configurer/configurer_test.cpp
@@ -48,11 +48,11 @@ using searchcorespi::index::IThreadingService;
 using proton::test::MockGidToLidChangeHandler;
 
 
-typedef DocumentDBConfig::ComparisonResult ConfigComparisonResult;
-typedef SearchableDocSubDBConfigurer Configurer;
-typedef std::unique_ptr<SearchableDocSubDBConfigurer> ConfigurerUP;
-typedef SummaryManager::SummarySetup SummarySetup;
-typedef proton::DocumentDBConfig::DocumenttypesConfigSP DocumenttypesConfigSP;
+using CCR = DocumentDBConfig::ComparisonResult;
+using Configurer = SearchableDocSubDBConfigurer;
+using ConfigurerUP = std::unique_ptr<SearchableDocSubDBConfigurer>;
+using SummarySetup = SummaryManager::SummarySetup;
+using DocumenttypesConfigSP = proton::DocumentDBConfig::DocumenttypesConfigSP;
 
 const vespalib::string BASE_DIR("baseDir");
 const vespalib::string DOC_TYPE("invalid");
@@ -504,12 +504,9 @@ asAttributeManager(const proton::IAttributeManager::SP &attrMgr)
 TEST_F("require that we can reconfigure attribute manager", Fixture)
 {
     ViewPtrs o = f._views.getViewPtrs();
-    ConfigComparisonResult cmpres;
-    cmpres.attributesChanged = true;
-    cmpres._schemaChanged = true;
     AttributeCollectionSpec::AttributeList specList;
     AttributeCollectionSpec spec(specList, 1, 0);
-    ReconfigParams params(cmpres);
+    ReconfigParams params(CCR().setAttributesChanged(true).setSchemaChanged(true));
     // Use new config snapshot == old config snapshot (only relevant for reprocessing)
     f._configurer->reconfigure(*createConfig(), *createConfig(), spec, params, f._resolver);
 
@@ -538,12 +535,9 @@ TEST_F("require that we can reconfigure attribute manager", Fixture)
 
 TEST_F("require that reconfigure returns reprocessing initializer when changing attributes", Fixture)
 {
-    ConfigComparisonResult cmpres;
-    cmpres.attributesChanged = true;
-    cmpres._schemaChanged = true;
     AttributeCollectionSpec::AttributeList specList;
     AttributeCollectionSpec spec(specList, 1, 0);
-    ReconfigParams params(cmpres);
+    ReconfigParams params(CCR().setAttributesChanged(true).setSchemaChanged(true));
     IReprocessingInitializer::UP init =
             f._configurer->reconfigure(*createConfig(), *createConfig(), spec, params, f._resolver);
 
@@ -582,9 +576,7 @@ TEST_F("require that reconfigure returns reprocessing initializer", FastAccessFi
 TEST_F("require that we can reconfigure summary manager", Fixture)
 {
     ViewPtrs o = f._views.getViewPtrs();
-    ConfigComparisonResult cmpres;
-    cmpres.summarymapChanged = true;
-    ReconfigParams params(cmpres);
+    ReconfigParams params(CCR().setSummarymapChanged(true));
     // Use new config snapshot == old config snapshot (only relevant for reprocessing)
     f._configurer->reconfigure(*createConfig(), *createConfig(), params, f._resolver);
 
@@ -604,11 +596,9 @@ TEST_F("require that we can reconfigure summary manager", Fixture)
 TEST_F("require that we can reconfigure matchers", Fixture)
 {
     ViewPtrs o = f._views.getViewPtrs();
-    ConfigComparisonResult cmpres;
-    cmpres.rankProfilesChanged = true;
     // Use new config snapshot == old config snapshot (only relevant for reprocessing)
     f._configurer->reconfigure(*createConfig(o.fv->getSchema()), *createConfig(o.fv->getSchema()),
-            ReconfigParams(cmpres), f._resolver);
+            ReconfigParams(CCR().setRankProfilesChanged(true)), f._resolver);
 
     ViewPtrs n = f._views.getViewPtrs();
     { // verify search view
@@ -634,12 +624,45 @@ TEST_F("require that we can reconfigure matchers", Fixture)
 
 TEST("require that attribute manager should change when imported fields has changed")
 {
-    DocumentDBConfig::ComparisonResult result;
-    result._importedFieldsChanged = true;
-    ReconfigParams params(result);
+    ReconfigParams params(CCR().setImportedFieldsChanged(true));
     EXPECT_TRUE(params.shouldAttributeManagerChange());
 }
 
+void
+assertMaintenanceControllerShouldNotChange(DocumentDBConfig::ComparisonResult result)
+{
+    ReconfigParams params(result);
+    EXPECT_FALSE(params.configHasChanged());
+    EXPECT_FALSE(params.shouldMaintenanceControllerChange());
+}
+
+void
+assertMaintenanceControllerShouldChange(DocumentDBConfig::ComparisonResult result)
+{
+    ReconfigParams params(result);
+    EXPECT_TRUE(params.configHasChanged());
+    EXPECT_TRUE(params.shouldMaintenanceControllerChange());
+}
+
+TEST("require that maintenance controller should change if some config has changed")
+{
+    TEST_DO(assertMaintenanceControllerShouldNotChange(CCR()));
+
+    TEST_DO(assertMaintenanceControllerShouldChange(CCR().setRankProfilesChanged(true)));
+    TEST_DO(assertMaintenanceControllerShouldChange(CCR().setRankingConstantsChanged(true)));
+    TEST_DO(assertMaintenanceControllerShouldChange(CCR().setIndexschemaChanged(true)));
+    TEST_DO(assertMaintenanceControllerShouldChange(CCR().setAttributesChanged(true)));
+    TEST_DO(assertMaintenanceControllerShouldChange(CCR().setSummaryChanged(true)));
+    TEST_DO(assertMaintenanceControllerShouldChange(CCR().setSummarymapChanged(true)));
+    TEST_DO(assertMaintenanceControllerShouldChange(CCR().setJuniperrcChanged(true)));
+    TEST_DO(assertMaintenanceControllerShouldChange(CCR().setDocumenttypesChanged(true)));
+    TEST_DO(assertMaintenanceControllerShouldChange(CCR().setDocumentTypeRepoChanged(true)));
+    TEST_DO(assertMaintenanceControllerShouldChange(CCR().setImportedFieldsChanged(true)));
+    TEST_DO(assertMaintenanceControllerShouldChange(CCR().setTuneFileDocumentDBChanged(true)));
+    TEST_DO(assertMaintenanceControllerShouldChange(CCR().setSchemaChanged(true)));
+    TEST_DO(assertMaintenanceControllerShouldChange(CCR().setMaintenanceChanged(true)));
+}
+
 TEST_MAIN()
 {
     TEST_RUN_ALL();
diff --git a/searchcore/src/tests/proton/documentdb/document_subdbs/document_subdbs_test.cpp b/searchcore/src/tests/proton/documentdb/document_subdbs/document_subdbs_test.cpp
index ea201dfd251..abfd80c9582 100644
--- a/searchcore/src/tests/proton/documentdb/document_subdbs/document_subdbs_test.cpp
+++ b/searchcore/src/tests/proton/documentdb/document_subdbs/document_subdbs_test.cpp
@@ -349,8 +349,8 @@ struct FixtureBase
 	    MyConfigSnapshot::UP newCfg(new MyConfigSnapshot(reconfigSchema, reconfigConfigDir));
 	    DocumentDBConfig::ComparisonResult cmpResult;
 	    cmpResult.attributesChanged = true;
-	    cmpResult._documenttypesChanged = true;
-	    cmpResult._documentTypeRepoChanged = true;
+	    cmpResult.documenttypesChanged = true;
+	    cmpResult.documentTypeRepoChanged = true;
 		MyDocumentDBReferenceResolver resolver;
 	    IReprocessingTask::List tasks =
 	            _subDb.applyConfig(*newCfg->_cfg,
diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp
index 0a02d00980e..4b6ced83e16 100644
--- a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp
@@ -494,7 +494,9 @@ DocumentDB::applyConfig(DocumentDBConfig::SP configSnapshot,
         _state.clearRejectedConfig();
     }
     setActiveConfig(configSnapshot, serialNum, generation);
-    forwardMaintenanceConfig();
+    if (params.shouldMaintenanceControllerChange()) {
+        forwardMaintenanceConfig();
+    }
     _writeFilter.setConfig(configSnapshot->getMaintenanceConfigSP()->
                            getAttributeUsageFilterConfig());
 }
diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdbconfig.cpp b/searchcore/src/vespa/searchcore/proton/server/documentdbconfig.cpp
index 820190d62cb..c766318a294 100644
--- a/searchcore/src/vespa/searchcore/proton/server/documentdbconfig.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/documentdbconfig.cpp
@@ -34,12 +34,12 @@ DocumentDBConfig::ComparisonResult::ComparisonResult()
       summaryChanged(false),
       summarymapChanged(false),
       juniperrcChanged(false),
-      _documenttypesChanged(false),
-      _documentTypeRepoChanged(false),
-      _importedFieldsChanged(false),
-      _tuneFileDocumentDBChanged(false),
-      _schemaChanged(false),
-      _maintenanceChanged(false)
+      documenttypesChanged(false),
+      documentTypeRepoChanged(false),
+      importedFieldsChanged(false),
+      tuneFileDocumentDBChanged(false),
+      schemaChanged(false),
+      maintenanceChanged(false)
 { }
 
 DocumentDBConfig::DocumentDBConfig(
@@ -156,18 +156,18 @@ DocumentDBConfig::compare(const DocumentDBConfig &rhs) const
         !equals<SummarymapConfig>(_summarymap.get(), rhs._summarymap.get());
     retval.juniperrcChanged =
         !equals<JuniperrcConfig>(_juniperrc.get(), rhs._juniperrc.get());
-    retval._documenttypesChanged =
+    retval.documenttypesChanged =
         !equals<DocumenttypesConfig>(_documenttypes.get(),
                 rhs._documenttypes.get());
-    retval._documentTypeRepoChanged = _repo.get() != rhs._repo.get();
-    retval._importedFieldsChanged =
+    retval.documentTypeRepoChanged = _repo.get() != rhs._repo.get();
+    retval.importedFieldsChanged =
             !equals<ImportedFieldsConfig >(_importedFields.get(), rhs._importedFields.get());
-    retval._tuneFileDocumentDBChanged =
+    retval.tuneFileDocumentDBChanged =
         !equals<TuneFileDocumentDB>(_tuneFileDocumentDB.get(),
                 rhs._tuneFileDocumentDB.get());
-    retval._schemaChanged =
+    retval.schemaChanged =
         !equals<Schema>(_schema.get(), rhs._schema.get());
-    retval._maintenanceChanged =
+    retval.maintenanceChanged =
         !equals<DocumentDBMaintenanceConfig>(_maintenance.get(),
                 rhs._maintenance.get());
     return retval;
diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdbconfig.h b/searchcore/src/vespa/searchcore/proton/server/documentdbconfig.h
index bdbce138fd9..46a6a8e76db 100644
--- a/searchcore/src/vespa/searchcore/proton/server/documentdbconfig.h
+++ b/searchcore/src/vespa/searchcore/proton/server/documentdbconfig.h
@@ -44,14 +44,27 @@ public:
         bool summaryChanged;
         bool summarymapChanged;
         bool juniperrcChanged;
-        bool _documenttypesChanged;
-        bool _documentTypeRepoChanged;
-        bool _importedFieldsChanged;
-        bool _tuneFileDocumentDBChanged;
-        bool _schemaChanged;
-        bool _maintenanceChanged;
+        bool documenttypesChanged;
+        bool documentTypeRepoChanged;
+        bool importedFieldsChanged;
+        bool tuneFileDocumentDBChanged;
+        bool schemaChanged;
+        bool maintenanceChanged;
 
         ComparisonResult();
+        ComparisonResult &setRankProfilesChanged(bool val) { rankProfilesChanged = val; return *this; }
+        ComparisonResult &setRankingConstantsChanged(bool val) { rankingConstantsChanged = val; return *this; }
+        ComparisonResult &setIndexschemaChanged(bool val) { indexschemaChanged = val; return *this; }
+        ComparisonResult &setAttributesChanged(bool val) { attributesChanged = val; return *this; }
+        ComparisonResult &setSummaryChanged(bool val) { summaryChanged = val; return *this; }
+        ComparisonResult &setSummarymapChanged(bool val) { summarymapChanged = val; return *this; }
+        ComparisonResult &setJuniperrcChanged(bool val) { juniperrcChanged = val; return *this; }
+        ComparisonResult &setDocumenttypesChanged(bool val) { documenttypesChanged = val; return *this; }
+        ComparisonResult &setDocumentTypeRepoChanged(bool val) { documentTypeRepoChanged = val; return *this; }
+        ComparisonResult &setImportedFieldsChanged(bool val) { importedFieldsChanged = val; return *this; }
+        ComparisonResult &setTuneFileDocumentDBChanged(bool val) { tuneFileDocumentDBChanged = val; return *this; }
+        ComparisonResult &setSchemaChanged(bool val) { schemaChanged = val; return *this; }
+        ComparisonResult &setMaintenanceChanged(bool val) { maintenanceChanged = val; return *this; }
     };
 
     using SP = std::shared_ptr<DocumentDBConfig>;
diff --git a/searchcore/src/vespa/searchcore/proton/server/reconfig_params.cpp b/searchcore/src/vespa/searchcore/proton/server/reconfig_params.cpp
index 34709a7764d..646dc66e3cb 100644
--- a/searchcore/src/vespa/searchcore/proton/server/reconfig_params.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/reconfig_params.cpp
@@ -14,9 +14,27 @@ ReconfigParams(const DocumentDBConfig::ComparisonResult &res)
 }
 
 bool
+ReconfigParams::configHasChanged() const
+{
+    return _res.rankProfilesChanged ||
+            _res.rankingConstantsChanged ||
+            _res.indexschemaChanged ||
+            _res.attributesChanged ||
+            _res.summaryChanged ||
+            _res.summarymapChanged ||
+            _res.juniperrcChanged ||
+            _res.documenttypesChanged ||
+            _res.documentTypeRepoChanged ||
+            _res.importedFieldsChanged ||
+            _res.tuneFileDocumentDBChanged ||
+            _res.schemaChanged ||
+            _res.maintenanceChanged;
+}
+
+bool
 ReconfigParams::shouldSchemaChange() const
 {
-    return _res._schemaChanged;
+    return _res.schemaChanged;
 }
 
 bool
@@ -34,7 +52,7 @@ ReconfigParams::shouldIndexManagerChange() const
 bool
 ReconfigParams::shouldAttributeManagerChange() const
 {
-    return _res.attributesChanged || _res._importedFieldsChanged;
+    return _res.attributesChanged || _res.importedFieldsChanged;
 }
 
 bool
@@ -43,4 +61,18 @@ ReconfigParams::shouldSummaryManagerChange() const
     return _res.summaryChanged || _res.summarymapChanged || _res.juniperrcChanged;
 }
 
+bool
+ReconfigParams::shouldSubDbsChange() const
+{
+    return shouldMatchersChange()
+           || shouldAttributeManagerChange()
+           || shouldSummaryManagerChange();
+}
+
+bool
+ReconfigParams::shouldMaintenanceControllerChange() const
+{
+    return configHasChanged();
+}
+
 } // namespace proton
diff --git a/searchcore/src/vespa/searchcore/proton/server/reconfig_params.h b/searchcore/src/vespa/searchcore/proton/server/reconfig_params.h
index 0a9f73cfd26..d5c5e53fcca 100644
--- a/searchcore/src/vespa/searchcore/proton/server/reconfig_params.h
+++ b/searchcore/src/vespa/searchcore/proton/server/reconfig_params.h
@@ -6,24 +6,23 @@
 
 namespace proton {
 
-class ReconfigParams
-{
+/**
+ * Class specifying which components that should change after a reconfig.
+ */
+class ReconfigParams {
 private:
     const DocumentDBConfig::ComparisonResult _res;
 
 public:
     ReconfigParams(const DocumentDBConfig::ComparisonResult &res);
+    bool configHasChanged() const;
     bool shouldSchemaChange() const;
     bool shouldMatchersChange() const;
     bool shouldIndexManagerChange() const;
     bool shouldAttributeManagerChange() const;
     bool shouldSummaryManagerChange() const;
-    bool shouldSubDbsChange() const {
-        return shouldMatchersChange()
-                || shouldAttributeManagerChange()
-                || shouldSummaryManagerChange();
-    }
+    bool shouldSubDbsChange() const;
+    bool shouldMaintenanceControllerChange() const;
 };
 
-} // namespace proton
-
+}