Merge pull request #15104 from vespa-engine/balder/gc-visibility-delay-1

Balder/gc visibility delay 1

8 files changed, 3 insertions, 94 deletions

diff --git a/searchcore/src/tests/proton/document_iterator/document_iterator_test.cpp b/searchcore/src/tests/proton/document_iterator/document_iterator_test.cpp
index 483e6719257..53f590189fa 100644
--- a/searchcore/src/tests/proton/document_iterator/document_iterator_test.cpp
+++ b/searchcore/src/tests/proton/document_iterator/document_iterator_test.cpp
@@ -275,10 +275,8 @@ struct PairDR : DocumentRetrieverBaseForTest {
 };
 
 struct Committer : public ICommitable {
-    size_t _commitCount;
     size_t _commitAndWaitCount;
-    Committer() : _commitCount(0), _commitAndWaitCount(0) { }
-    void commit() override { _commitCount++; }
+    Committer() : _commitAndWaitCount(0) { }
     void commitAndWait(ILidCommitState &) override { _commitAndWaitCount++; }
     void commitAndWait(ILidCommitState & tracker, uint32_t ) override {
         commitAndWait(tracker);
@@ -512,7 +510,6 @@ void verifyReadConsistency(DocumentIterator & itr, Committer & committer) {
     EXPECT_TRUE(res.isCompleted());
     EXPECT_EQUAL(1u, res.getEntries().size());
     TEST_DO(checkEntry(res, 0, Document(*DataType::DOCUMENT, DocumentId("id:ns:document::1")), Timestamp(2)));
-    EXPECT_EQUAL(0u, committer._commitCount);
 }
 
 void verifyStrongReadConsistency(DocumentIterator & itr) {
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 754cf4ea15d..53050db5a00 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
@@ -254,10 +254,8 @@ struct TwoAttrSchema : public OneAttrSchema
 };
 
 struct Committer : public ICommitable {
-    size_t _commitCount;
     size_t _commitAndWaitCount;
-    Committer() : _commitCount(0), _commitAndWaitCount(0) { }
-    void commit() override { _commitCount++; }
+    Committer() : _commitAndWaitCount(0) { }
     void commitAndWait(ILidCommitState & ) override { _commitAndWaitCount++; }
     void commitAndWait(ILidCommitState & tracker, uint32_t ) override {
         commitAndWait(tracker);
diff --git a/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp b/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp
index 09c4c17220e..d309c7f8b1a 100644
--- a/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp
+++ b/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp
@@ -388,7 +388,6 @@ public:
     ~MaintenanceControllerFixture() override;
 
     void syncSubDBs();
-    void commit() override { }
     void commitAndWait(ILidCommitState & ) override { }
     void commitAndWait(ILidCommitState & tracker, uint32_t ) override {
         commitAndWait(tracker);
diff --git a/searchcore/src/tests/proton/server/visibility_handler/visibility_handler_test.cpp b/searchcore/src/tests/proton/server/visibility_handler/visibility_handler_test.cpp
index 2048ecb5458..2351d5ed0a9 100644
--- a/searchcore/src/tests/proton/server/visibility_handler/visibility_handler_test.cpp
+++ b/searchcore/src/tests/proton/server/visibility_handler/visibility_handler_test.cpp
@@ -103,28 +103,6 @@ public:
                      _writeService.masterObserver().getExecuteCnt());
     }
 
-    void
-    testCommit(vespalib::duration visibilityDelay, bool internal,
-               uint32_t expForceCommitCount, SerialNum expCommittedSerialNum,
-               uint32_t expMasterExecuteCnt,
-               SerialNum currSerialNum = 10u)
-    {
-        _feedViewReal->setTracker(visibilityDelay);
-        _getSerialNum.setSerialNum(currSerialNum);
-        _visibilityHandler.setVisibilityDelay(visibilityDelay);
-        if (internal) {
-            VisibilityHandler *visibilityHandler = &_visibilityHandler;
-            auto task = makeLambdaTask([=]() { visibilityHandler->commit(); });
-            _writeService.master().execute(std::move(task));
-        } else {
-            _visibilityHandler.commit();
-        }
-        _writeService.master().sync();
-        checkCommitPostCondition(expForceCommitCount,
-                                 expCommittedSerialNum,
-                                 expMasterExecuteCnt);
-    }
-
     proton::PendingLidTracker::Token
     createToken(proton::PendingLidTrackerBase & tracker, SerialNum serialNum, uint32_t lid) {
         if (serialNum == 0) {
@@ -143,7 +121,6 @@ public:
     {
         _feedViewReal->setTracker(visibilityDelay);
         _getSerialNum.setSerialNum(currSerialNum);
-        _visibilityHandler.setVisibilityDelay(visibilityDelay);
         constexpr uint32_t MY_LID=13;
         proton::PendingLidTrackerBase * lidTracker = _feedViewReal->_tracker.get();
         {
@@ -165,36 +142,6 @@ public:
 
 }
 
-TEST_F("Check external commit with zero visibility delay", Fixture)
-{
-    f.testCommit(0s, false, 0u, 0u, 0u);
-}
-
-TEST_F("Check external commit with nonzero visibility delay", Fixture)
-{
-    f.testCommit(1s, false, 1u, 10u, 1u);
-}
-
-TEST_F("Check external commit with nonzero visibility delay and no new feed operation", Fixture)
-{
-    f.testCommit(1s, false, 1u, 0u, 1u, 0u);
-}
-
-TEST_F("Check internal commit with zero visibility delay", Fixture)
-{
-    f.testCommit(0s, true, 0u, 0u, 1u);
-}
-
-TEST_F("Check internal commit with nonzero visibility delay", Fixture)
-{
-    f.testCommit(1s, true, 1u, 10u, 1u);
-}
-
-TEST_F("Check internal commit with nonzero visibility delay and no new feed operation", Fixture)
-{
-    f.testCommit(1s, true, 1u, 0u, 1u, 0u);
-}
-
 TEST_F("Check external commitAndWait with zero visibility delay", Fixture)
 {
     f.testCommitAndWait(0s, false, 0u, 0u, 0u);
diff --git a/searchcore/src/vespa/searchcore/proton/common/icommitable.h b/searchcore/src/vespa/searchcore/proton/common/icommitable.h
index 55762a69862..85aea3fc486 100644
--- a/searchcore/src/vespa/searchcore/proton/common/icommitable.h
+++ b/searchcore/src/vespa/searchcore/proton/common/icommitable.h
@@ -12,7 +12,6 @@ class ILidCommitState;
  **/
 class ICommitable {
 public:
-    virtual void commit() = 0;
     virtual void commitAndWait(ILidCommitState & unCommittedLidTracker) = 0;
     virtual void commitAndWait(ILidCommitState &uncommittedLidTracker, uint32_t lid) = 0;
     virtual void commitAndWait(ILidCommitState &uncommittedLidTracker, const std::vector<uint32_t> & lid) = 0;
diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp
index b35e1a88495..ad500d4dac8 100644
--- a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp
@@ -208,8 +208,6 @@ DocumentDB::DocumentDB(const vespalib::string &baseDir,
     _lidSpaceCompactionHandlers.push_back(std::make_unique<LidSpaceCompactionHandler>(_maintenanceController.getNotReadySubDB(), _docTypeName.getName()));
 
     _writeFilter.setConfig(loaded_config->getMaintenanceConfigSP()->getAttributeUsageFilterConfig());
-    vespalib::duration visibilityDelay = loaded_config->getMaintenanceConfigSP()->getVisibilityDelay();
-    _visibility.setVisibilityDelay(visibilityDelay);
 }
 
 void DocumentDB::registerReference()
@@ -441,21 +439,17 @@ DocumentDB::applyConfig(DocumentDBConfig::SP configSnapshot, SerialNum serialNum
         commit_result = _feedHandler->storeOperationSync(op);
         sync(op.getSerialNum());
     }
-    bool hasVisibilityDelayChanged = false;
     {
         bool elidedConfigSave = equalReplayConfig && tlsReplayDone;
         // Flush changes to attributes and memory index, cf. visibilityDelay
         _feedView.get()->forceCommit(elidedConfigSave ? serialNum : serialNum - 1, std::make_shared<search::KeepAlive<FeedHandler::CommitResult>>(std::move(commit_result)));
         _writeService.sync();
-        vespalib::duration visibilityDelay = configSnapshot->getMaintenanceConfigSP()->getVisibilityDelay();
-        hasVisibilityDelayChanged = (visibilityDelay != _visibility.getVisibilityDelay());
-        _visibility.setVisibilityDelay(visibilityDelay);
     }
     if (_state.getState() >= DDBState::State::APPLY_LIVE_CONFIG) {
         _writeServiceConfig.update(configSnapshot->get_threading_service_config());
     }
     _writeService.setTaskLimit(_writeServiceConfig.defaultTaskLimit(), _writeServiceConfig.defaultTaskLimit());
-    if (params.shouldSubDbsChange() || hasVisibilityDelayChanged) {
+    if (params.shouldSubDbsChange()) {
         applySubDBConfig(*configSnapshot, serialNum, params);
         if (serialNum < _feedHandler->getSerialNum()) {
             // Not last entry in tls.  Reprocessing should already be done.
@@ -562,7 +556,6 @@ DocumentDB::close()
     // Abort any ongoing maintenance
     stopMaintenance();
 
-    _visibility.commit();
     _writeService.sync();
 
     // The attributes in the ready sub db is also the total set of attributes.
@@ -908,17 +901,6 @@ DocumentDB::syncFeedView()
     IFeedView::SP newFeedView(_subDBs.getFeedView());
 
     _writeService.sync();
-    /*
-     * Don't call commit() on visibility handler during transaction
-     * log replay since the serial number used for the commit will be
-     * too high until the replay is complete. This check can be
-     * removed again when feed handler has improved tracking of serial
-     * numbers during replay.
-     */
-    if (_state.getAllowReconfig()) {
-        _visibility.commit();
-    }
-    _writeService.sync();
 
     _feedView.set(newFeedView);
     _feedHandler->setActiveFeedView(newFeedView.get());
diff --git a/searchcore/src/vespa/searchcore/proton/server/visibilityhandler.cpp b/searchcore/src/vespa/searchcore/proton/server/visibilityhandler.cpp
index 3a44af517ee..67deee74e88 100644
--- a/searchcore/src/vespa/searchcore/proton/server/visibilityhandler.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/visibilityhandler.cpp
@@ -14,7 +14,6 @@ VisibilityHandler::VisibilityHandler(const IGetSerialNum & serial,
     : _serial(serial),
       _writeService(writeService),
       _feedView(feedView),
-      _visibilityDelay(vespalib::duration::zero()),
       _lastCommitSerialNum(0),
       _lock()
 {
@@ -33,13 +32,6 @@ VisibilityHandler::internalCommit(bool force)
         (void) wasCommitTaskSpawned;
     }
 }
-void
-VisibilityHandler::commit()
-{
-    if (hasVisibilityDelay()) {
-        internalCommit(true);
-    }
-}
 
 void
 VisibilityHandler::commitAndWait(ILidCommitState & unCommittedLidTracker)
diff --git a/searchcore/src/vespa/searchcore/proton/server/visibilityhandler.h b/searchcore/src/vespa/searchcore/proton/server/visibilityhandler.h
index c22128685c1..7286a532d43 100644
--- a/searchcore/src/vespa/searchcore/proton/server/visibilityhandler.h
+++ b/searchcore/src/vespa/searchcore/proton/server/visibilityhandler.h
@@ -26,10 +26,6 @@ public:
                       IThreadingService &threadingService,
                       const FeedViewHolder &feedView);
     ~VisibilityHandler() override;
-    void setVisibilityDelay(vespalib::duration visibilityDelay) { _visibilityDelay = visibilityDelay; }
-    vespalib::duration getVisibilityDelay() const { return _visibilityDelay; }
-    bool hasVisibilityDelay() const { return _visibilityDelay != vespalib::duration::zero(); }
-    void commit() override;
     void commitAndWait(ILidCommitState & unCommittedLidTracker) override;
     void commitAndWait(ILidCommitState &, uint32_t ) override;
     void commitAndWait(ILidCommitState &, const std::vector<uint32_t> & ) override;
@@ -40,7 +36,6 @@ private:
     const IGetSerialNum  & _serial;
     IThreadingService    & _writeService;
     const FeedViewHolder & _feedView;
-    vespalib::duration     _visibilityDelay;
     SerialNum              _lastCommitSerialNum;
     std::mutex             _lock;
 };