Ignore GC flush targets when calculating oldest flushed serial number.

The oldest flushed serial number is used to prune the transaction log up to this point in time.
As GC flush targets are not dependent on data stored in the transaction log they should not affect pruning.

2 files changed, 53 insertions, 17 deletions

diff --git a/searchcore/src/tests/proton/flushengine/flushengine_test.cpp b/searchcore/src/tests/proton/flushengine/flushengine_test.cpp
index 9471efb5566..f668072b9fd 100644
--- a/searchcore/src/tests/proton/flushengine/flushengine_test.cpp
+++ b/searchcore/src/tests/proton/flushengine/flushengine_test.cpp
@@ -245,39 +245,45 @@ public:
     vespalib::Gate    _taskDone;
     Task::UP          _task;
 
-public:
-    typedef std::shared_ptr<SimpleTarget> SP;
-
-    SimpleTarget(Task::UP task, const std::string &name) :
-        test::DummyFlushTarget(name),
-        _flushedSerial(0),
-        _currentSerial(0),
+protected:
+    SimpleTarget(const std::string &name, const Type &type, search::SerialNum flushedSerial = 0, bool proceedImmediately = true) :
+        test::DummyFlushTarget(name, type, Component::OTHER),
+        _flushedSerial(flushedSerial),
         _proceed(),
         _initDone(),
         _taskStart(),
         _taskDone(),
-        _task(std::move(task))
+        _task(std::make_unique<SimpleTask>(_taskStart, _taskDone, &_proceed,
+                                           _flushedSerial, _currentSerial))
     {
+        if (proceedImmediately) {
+            _proceed.countDown();
+        }
     }
 
-    SimpleTarget(const std::string &name, search::SerialNum flushedSerial = 0, bool proceedImmediately = true) :
+public:
+    using SP = std::shared_ptr<SimpleTarget>;
+
+    SimpleTarget(Task::UP task, const std::string &name) :
         test::DummyFlushTarget(name),
-        _flushedSerial(flushedSerial),
+        _flushedSerial(0),
+        _currentSerial(0),
         _proceed(),
         _initDone(),
         _taskStart(),
         _taskDone(),
-        _task(std::make_unique<SimpleTask>(_taskStart, _taskDone, &_proceed,
-                             _flushedSerial, _currentSerial))
+        _task(std::move(task))
     {
-        if (proceedImmediately) {
-            _proceed.countDown();
-        }
     }
+
     SimpleTarget(search::SerialNum flushedSerial = 0, bool proceedImmediately = true)
         : SimpleTarget("anon", flushedSerial, proceedImmediately)
     { }
 
+    SimpleTarget(const std::string &name, search::SerialNum flushedSerial = 0, bool proceedImmediately = true)
+            : SimpleTarget(name, Type::OTHER, flushedSerial, proceedImmediately)
+    { }
+
     virtual Time
     getLastFlushTime() const override { return fastos::ClockSystem::now(); }
 
@@ -301,6 +307,13 @@ public:
 
 };
 
+class GCTarget : public SimpleTarget {
+public:
+    GCTarget(const vespalib::string &name, search::SerialNum flushedSerial)
+            : SimpleTarget(name, Type::GC, flushedSerial)
+    {}
+};
+
 class AssertedTarget : public SimpleTarget {
 public:
     mutable bool _mgain;
@@ -473,7 +486,6 @@ struct Fixture
     }
 };
 
-
 TEST_F("require that strategy controls flush target", Fixture(1, IINTERVAL))
 {
     vespalib::Gate fooG, barG;
@@ -523,6 +535,28 @@ TEST_F("require that oldest serial is found", Fixture(1, IINTERVAL))
     EXPECT_EQUAL(FlushDoneHistory({ 10, 20, 25 }), handlerFlushDoneHistory);
 }
 
+TEST_F("require that GC targets are not considered when oldest serial is found", Fixture(1, IINTERVAL))
+{
+    auto foo = std::make_shared<SimpleTarget>("foo", 5);
+    auto bar = std::make_shared<GCTarget>("bar", 10);
+    auto baz = std::make_shared<SimpleTarget>("baz", 20);
+    f.addTargetToStrategy(foo);
+    f.addTargetToStrategy(bar);
+    f.addTargetToStrategy(baz);
+
+    auto handler = std::make_shared<SimpleHandler>(Targets({foo, bar, baz}), "handler", 25);
+    f.putFlushHandler("handler", handler);
+    f.engine.start();
+
+    // The targets are flushed in sequence: 'foo', 'bar', 'baz'
+    EXPECT_TRUE(handler->_done.await(LONG_TIMEOUT));
+    EXPECT_EQUAL(25ul, handler->_oldestSerial);
+
+    // Before anything is flushed the oldest serial is 5.
+    // After 'foo' has been flushed the oldest serial is 20 as GC target 'bar' is not considered.
+    EXPECT_EQUAL(FlushDoneHistory({ 5, 20, 20, 25 }), handler->getFlushDoneHistory());
+}
+
 TEST_F("require that oldest serial is found in group", Fixture(2, IINTERVAL))
 {
     auto fooT1 = std::make_shared<SimpleTarget>("fooT1", 10);
diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp b/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp
index 0d2c556b4d6..4866ec0d710 100644
--- a/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp
+++ b/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp
@@ -27,7 +27,9 @@ findOldestFlushedSerial(const IFlushTarget::List &lst, const IFlushHandler &hand
 {
     search::SerialNum ret(handler.getCurrentSerialNumber());
     for (const IFlushTarget::SP & target : lst) {
-        ret = std::min(ret, target->getFlushedSerialNum());
+        if (target->getType() != IFlushTarget::Type::GC) {
+            ret = std::min(ret, target->getFlushedSerialNum());
+        }
     }
     LOG(debug, "Oldest flushed serial for '%s' is %" PRIu64 ".", handler.getName().c_str(), ret);
     return ret;