Separate spread and bloat

8 files changed, 121 insertions, 41 deletions

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 7f28ccd0737..c16dc349d83 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
@@ -702,29 +702,31 @@ assertTarget(const vespalib::string &name,
 TEST_F("require that flush targets can be retrieved", FastAccessFixture)
 {
     IFlushTarget::List targets = getFlushTargets(f);
-    EXPECT_EQUAL(7u, targets.size());
+    EXPECT_EQUAL(8u, targets.size());
     EXPECT_EQUAL("subdb.attribute.flush.attr1", targets[0]->getName());
     EXPECT_EQUAL("subdb.attribute.shrink.attr1", targets[1]->getName());
     EXPECT_EQUAL("subdb.documentmetastore.flush", targets[2]->getName());
     EXPECT_EQUAL("subdb.documentmetastore.shrink", targets[3]->getName());
-    EXPECT_EQUAL("subdb.summary.compact", targets[4]->getName());
-    EXPECT_EQUAL("subdb.summary.flush", targets[5]->getName());
-    EXPECT_EQUAL("subdb.summary.shrink", targets[6]->getName());
+    EXPECT_EQUAL("subdb.summary.compact_bloat", targets[4]->getName());
+    EXPECT_EQUAL("subdb.summary.compact_spread", targets[5]->getName());
+    EXPECT_EQUAL("subdb.summary.flush", targets[6]->getName());
+    EXPECT_EQUAL("subdb.summary.shrink", targets[7]->getName());
 }
 
 TEST_F("require that flush targets can be retrieved", SearchableFixture)
 {
     IFlushTarget::List targets = getFlushTargets(f);
-    EXPECT_EQUAL(9u, targets.size());
+    EXPECT_EQUAL(10u, targets.size());
     EXPECT_TRUE(assertTarget("subdb.attribute.flush.attr1", FType::SYNC, FComponent::ATTRIBUTE, *targets[0]));
     EXPECT_TRUE(assertTarget("subdb.attribute.shrink.attr1", FType::GC, FComponent::ATTRIBUTE, *targets[1]));
     EXPECT_TRUE(assertTarget("subdb.documentmetastore.flush", FType::SYNC, FComponent::ATTRIBUTE, *targets[2]));
     EXPECT_TRUE(assertTarget("subdb.documentmetastore.shrink", FType::GC, FComponent::ATTRIBUTE, *targets[3]));
     EXPECT_TRUE(assertTarget("subdb.memoryindex.flush", FType::FLUSH, FComponent::INDEX, *targets[4]));
     EXPECT_TRUE(assertTarget("subdb.memoryindex.fusion", FType::GC, FComponent::INDEX, *targets[5]));
-    EXPECT_TRUE(assertTarget("subdb.summary.compact", FType::GC, FComponent::DOCUMENT_STORE, *targets[6]));
-    EXPECT_TRUE(assertTarget("subdb.summary.flush", FType::SYNC, FComponent::DOCUMENT_STORE, *targets[7]));
-    EXPECT_TRUE(assertTarget("subdb.summary.shrink", FType::GC, FComponent::DOCUMENT_STORE, *targets[8]));
+    EXPECT_TRUE(assertTarget("subdb.summary.compact_bloat", FType::GC, FComponent::DOCUMENT_STORE, *targets[6]));
+    EXPECT_TRUE(assertTarget("subdb.summary.compact_spread", FType::GC, FComponent::DOCUMENT_STORE, *targets[7]));
+    EXPECT_TRUE(assertTarget("subdb.summary.flush", FType::SYNC, FComponent::DOCUMENT_STORE, *targets[8]));
+    EXPECT_TRUE(assertTarget("subdb.summary.shrink", FType::GC, FComponent::DOCUMENT_STORE, *targets[9]));
 }
 
 TEST_F("require that only fast-access attributes are instantiated", FastAccessOnlyFixture)
diff --git a/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_common.h b/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_common.h
index 42976104836..8e1b23eba67 100644
--- a/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_common.h
+++ b/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_common.h
@@ -15,6 +15,7 @@
 #include <vespa/searchcore/proton/test/clusterstatehandler.h>
 #include <vespa/searchcore/proton/test/disk_mem_usage_notifier.h>
 #include <vespa/searchcore/proton/test/test.h>
+#include <vespa/searchcore/proton/test/dummy_document_store.h>
 #include <vespa/vespalib/util/idestructorcallback.h>
 #include <vespa/searchlib/index/docbuilder.h>
 
diff --git a/searchcore/src/vespa/searchcore/proton/docsummary/summarycompacttarget.cpp b/searchcore/src/vespa/searchcore/proton/docsummary/summarycompacttarget.cpp
index 4e0cf3f9059..06bf8d0a8a6 100644
--- a/searchcore/src/vespa/searchcore/proton/docsummary/summarycompacttarget.cpp
+++ b/searchcore/src/vespa/searchcore/proton/docsummary/summarycompacttarget.cpp
@@ -9,21 +9,26 @@ using search::SerialNum;
 using vespalib::makeLambdaTask;
 using searchcorespi::FlushStats;
 using searchcorespi::IFlushTarget;
+using searchcorespi::FlushTask;
 
 namespace proton {
 
 namespace {
 
-class Compacter : public searchcorespi::FlushTask {
+class Compacter : public FlushTask {
 private:
     IDocumentStore & _docStore;
     FlushStats     & _stats;
     SerialNum        _currSerial;
+    virtual void compact(IDocumentStore & docStore, SerialNum currSerial) const = 0;
 public:
-    Compacter(IDocumentStore & docStore, FlushStats & stats, SerialNum currSerial) :
-        _docStore(docStore), _stats(stats), _currSerial(currSerial) {}
+    Compacter(IDocumentStore & docStore, FlushStats & stats, SerialNum currSerial)
+        : _docStore(docStore),
+          _stats(stats),
+          _currSerial(currSerial)
+    {}
     void run() override {
-        _docStore.compact(_currSerial);
+        compact(_docStore, _currSerial);
         updateStats();
     }
     void updateStats() {
@@ -36,10 +41,32 @@ public:
     }
 };
 
+class CompactBloat : public Compacter {
+public:
+    CompactBloat(IDocumentStore & docStore, FlushStats & stats, SerialNum currSerial)
+        : Compacter(docStore, stats, currSerial)
+    {}
+private:
+    void compact(IDocumentStore & docStore, SerialNum currSerial) const override {
+        docStore.compactBloat(currSerial);
+    }
+};
+
+class CompactSpread : public Compacter {
+public:
+    CompactSpread(IDocumentStore & docStore, FlushStats & stats, SerialNum currSerial)
+        : Compacter(docStore, stats, currSerial)
+    {}
+private:
+    void compact(IDocumentStore & docStore, SerialNum currSerial) const override {
+        docStore.compactSpread(currSerial);
+    }
+};
+
 }
 
-SummaryCompactTarget::SummaryCompactTarget(vespalib::Executor & summaryService, IDocumentStore & docStore)
-    : IFlushTarget("summary.compact", Type::GC, Component::DOCUMENT_STORE),
+SummaryGCTarget::SummaryGCTarget(const vespalib::string & name, vespalib::Executor & summaryService, IDocumentStore & docStore)
+    : IFlushTarget(name, Type::GC, Component::DOCUMENT_STORE),
       _summaryService(summaryService),
       _docStore(docStore),
       _lastStats()
@@ -48,37 +75,69 @@ SummaryCompactTarget::SummaryCompactTarget(vespalib::Executor & summaryService,
 }
 
 IFlushTarget::MemoryGain
-SummaryCompactTarget::getApproxMemoryGain() const
+SummaryGCTarget::getApproxMemoryGain() const
 {
     return MemoryGain::noGain(_docStore.memoryUsed());
 }
 
 IFlushTarget::DiskGain
-SummaryCompactTarget::getApproxDiskGain() const
+SummaryGCTarget::getApproxDiskGain() const
 {
     size_t total(_docStore.getDiskFootprint());
-    return DiskGain(total, total - std::min(total, _docStore.getMaxCompactGain()));
+    return DiskGain(total, total - std::min(total, getBloat(_docStore)));
 }
 
 IFlushTarget::Time
-SummaryCompactTarget::getLastFlushTime() const
+SummaryGCTarget::getLastFlushTime() const
 {
     return vespalib::system_clock::now();
 }
 
 SerialNum
-SummaryCompactTarget::getFlushedSerialNum() const
+SummaryGCTarget::getFlushedSerialNum() const
 {
     return _docStore.tentativeLastSyncToken();
 }
 
 IFlushTarget::Task::UP
-SummaryCompactTarget::initFlush(SerialNum currentSerial, std::shared_ptr<search::IFlushToken>)
+SummaryGCTarget::initFlush(SerialNum currentSerial, std::shared_ptr<search::IFlushToken>)
 {
     std::promise<Task::UP> promise;
     std::future<Task::UP> future = promise.get_future();
-    _summaryService.execute(makeLambdaTask([&]() { promise.set_value(std::make_unique<Compacter>(_docStore, _lastStats, currentSerial)); }));
+    _summaryService.execute(makeLambdaTask([this, &promise,currentSerial]() {
+        promise.set_value(create(_docStore, _lastStats, currentSerial));
+    }));
     return future.get();
 }
 
+SummaryCompactBloatTarget::SummaryCompactBloatTarget(vespalib::Executor & summaryService, IDocumentStore & docStore)
+    : SummaryGCTarget("summary.compact_bloat", summaryService, docStore)
+{
+}
+
+size_t
+SummaryCompactBloatTarget::getBloat(const search::IDocumentStore & docStore) const {
+    return docStore.getDiskBloat();
+}
+
+FlushTask::UP
+SummaryCompactBloatTarget::create(IDocumentStore & docStore, FlushStats & stats, SerialNum currSerial) {
+    return std::make_unique<CompactBloat>(docStore, stats, currSerial);
+}
+
+SummaryCompactSpreadTarget::SummaryCompactSpreadTarget(vespalib::Executor & summaryService, IDocumentStore & docStore)
+    : SummaryGCTarget("summary.compact_spread", summaryService, docStore)
+{
+}
+
+size_t
+SummaryCompactSpreadTarget::getBloat(const search::IDocumentStore & docStore) const {
+    return docStore.getMaxSpreadAsBloat();
+}
+
+FlushTask::UP
+SummaryCompactSpreadTarget::create(IDocumentStore & docStore, FlushStats & stats, SerialNum currSerial) {
+    return std::make_unique<CompactSpread>(docStore, stats, currSerial);
+}
+
 } // namespace proton
diff --git a/searchcore/src/vespa/searchcore/proton/docsummary/summarycompacttarget.h b/searchcore/src/vespa/searchcore/proton/docsummary/summarycompacttarget.h
index c8035a544f2..529aa3b816e 100644
--- a/searchcore/src/vespa/searchcore/proton/docsummary/summarycompacttarget.h
+++ b/searchcore/src/vespa/searchcore/proton/docsummary/summarycompacttarget.h
@@ -12,16 +12,10 @@ namespace proton {
 /**
  * This class implements the IFlushTarget interface to proxy a summary manager.
  */
-class SummaryCompactTarget : public searchcorespi::IFlushTarget {
-private:
-    using FlushStats = searchcorespi::FlushStats;
-    vespalib::Executor &_summaryService;
-    search::IDocumentStore & _docStore;
-    FlushStats _lastStats;
-
+class SummaryGCTarget : public searchcorespi::IFlushTarget {
 public:
-    SummaryCompactTarget(vespalib::Executor & summaryService, search::IDocumentStore & docStore);
-
+    using FlushStats = searchcorespi::FlushStats;
+    using IDocumentStore = search::IDocumentStore;
     MemoryGain getApproxMemoryGain() const override;
     DiskGain getApproxDiskGain() const override;
     SerialNum getFlushedSerialNum() const override;
@@ -31,6 +25,32 @@ public:
 
     FlushStats getLastFlushStats() const override { return _lastStats; }
     uint64_t getApproxBytesToWriteToDisk() const override { return 0; }
+protected:
+    SummaryGCTarget(const vespalib::string &, vespalib::Executor & summaryService, IDocumentStore & docStore);
+private:
+
+    virtual size_t getBloat(const IDocumentStore & docStore) const = 0;
+    virtual Task::UP create(IDocumentStore & docStore, FlushStats & stats, SerialNum currSerial) = 0;
+
+    vespalib::Executor      &_summaryService;
+    IDocumentStore & _docStore;
+    FlushStats               _lastStats;
+};
+
+class SummaryCompactBloatTarget : public SummaryGCTarget {
+private:
+    size_t getBloat(const search::IDocumentStore & docStore) const override;
+    Task::UP create(IDocumentStore & docStore, FlushStats & stats, SerialNum currSerial) override;
+public:
+    SummaryCompactBloatTarget(vespalib::Executor & summaryService, IDocumentStore & docStore);
+};
+
+class SummaryCompactSpreadTarget : public SummaryGCTarget {
+private:
+    size_t getBloat(const search::IDocumentStore & docStore) const override;
+    Task::UP create(IDocumentStore & docStore, FlushStats & stats, SerialNum currSerial) override;
+public:
+    SummaryCompactSpreadTarget(vespalib::Executor & summaryService, IDocumentStore & docStore);
 };
 
 } // namespace proton
diff --git a/searchcore/src/vespa/searchcore/proton/docsummary/summarymanager.cpp b/searchcore/src/vespa/searchcore/proton/docsummary/summarymanager.cpp
index eaf5a907808..28a91e1444d 100644
--- a/searchcore/src/vespa/searchcore/proton/docsummary/summarymanager.cpp
+++ b/searchcore/src/vespa/searchcore/proton/docsummary/summarymanager.cpp
@@ -200,7 +200,8 @@ SummaryManager::getFlushTargets(vespalib::Executor & summaryService)
     IFlushTarget::List ret;
     ret.push_back(std::make_shared<SummaryFlushTarget>(getBackingStore(), summaryService));
     if (dynamic_cast<LogDocumentStore *>(_docStore.get()) != nullptr) {
-        ret.push_back(std::make_shared<SummaryCompactTarget>(summaryService, getBackingStore()));
+        ret.push_back(std::make_shared<SummaryCompactBloatTarget>(summaryService, getBackingStore()));
+        ret.push_back(std::make_shared<SummaryCompactSpreadTarget>(summaryService, getBackingStore()));
     }
     ret.push_back(createShrinkLidSpaceFlushTarget(summaryService, _docStore));
     return ret;
diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdbconfigmanager.cpp b/searchcore/src/vespa/searchcore/proton/server/documentdbconfigmanager.cpp
index a9873a80d0e..a77ff93c002 100644
--- a/searchcore/src/vespa/searchcore/proton/server/documentdbconfigmanager.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/documentdbconfigmanager.cpp
@@ -197,7 +197,7 @@ getStoreConfig(const ProtonConfig::Summary::Cache & cache, const HwInfo & hwInfo
 }
 
 LogDocumentStore::Config
-deriveConfig(const ProtonConfig::Summary & summary, const ProtonConfig::Flush::Memory & flush, const HwInfo & hwInfo) {
+deriveConfig(const ProtonConfig::Summary & summary, const HwInfo & hwInfo) {
     DocumentStore::Config config(getStoreConfig(summary.cache, hwInfo));
     const ProtonConfig::Summary::Log & log(summary.log);
     const ProtonConfig::Summary::Log::Chunk & chunk(log.chunk);
@@ -205,7 +205,6 @@ deriveConfig(const ProtonConfig::Summary & summary, const ProtonConfig::Flush::M
     LogDataStore::Config logConfig;
     logConfig.setMaxFileSize(log.maxfilesize)
             .setMaxNumLids(log.maxnumlids)
-            .setMaxDiskBloatFactor(std::min(flush.diskbloatfactor, flush.each.diskbloatfactor))
             .setMaxBucketSpread(log.maxbucketspread).setMinFileSizeFactor(log.minfilesizefactor)
             .compactCompression(deriveCompression(log.compact.compression))
             .setFileConfig(fileConfig).disableCrcOnRead(chunk.skipcrconread);
@@ -213,7 +212,7 @@ deriveConfig(const ProtonConfig::Summary & summary, const ProtonConfig::Flush::M
 }
 
 search::LogDocumentStore::Config buildStoreConfig(const ProtonConfig & proton, const HwInfo & hwInfo) {
-    return deriveConfig(proton.summary, proton.flush.memory, hwInfo);
+    return deriveConfig(proton.summary, hwInfo);
 }
 
 using AttributesConfigSP = DocumentDBConfig::AttributesConfigSP;
diff --git a/searchcore/src/vespa/searchcore/proton/test/dummy_document_store.h b/searchcore/src/vespa/searchcore/proton/test/dummy_document_store.h
index d9b83bfc3a8..7194cc4d403 100644
--- a/searchcore/src/vespa/searchcore/proton/test/dummy_document_store.h
+++ b/searchcore/src/vespa/searchcore/proton/test/dummy_document_store.h
@@ -10,13 +10,11 @@ struct DummyDocumentStore : public search::IDocumentStore
 {
     vespalib::string _baseDir;
 
-    DummyDocumentStore()
-        : _baseDir("")
-    {}
+    DummyDocumentStore() = default;
     DummyDocumentStore(const vespalib::string &baseDir)
         : _baseDir(baseDir)
     {}
-    ~DummyDocumentStore() {}
+    ~DummyDocumentStore() = default;
     DocumentUP read(search::DocumentIdT, const document::DocumentTypeRepo &) const override {
         return DocumentUP();
     }
@@ -25,7 +23,8 @@ struct DummyDocumentStore : public search::IDocumentStore
     void remove(uint64_t, search::DocumentIdT) override {}
     void flush(uint64_t) override {}
     uint64_t initFlush(uint64_t) override { return 0; }
-    void compact(uint64_t) override {}
+    void compactBloat(uint64_t) override {}
+    void compactSpread(uint64_t) override {}
     uint64_t lastSyncToken() const override { return 0; }
     uint64_t tentativeLastSyncToken() const override { return 0; }
     vespalib::system_time getLastFlushTime() const override { return vespalib::system_time(); }
@@ -34,7 +33,7 @@ struct DummyDocumentStore : public search::IDocumentStore
     size_t memoryMeta() const override { return 0; }
     size_t getDiskFootprint() const override { return 0; }
     size_t getDiskBloat() const override { return 0; }
-    size_t getMaxCompactGain() const override { return getDiskBloat(); }
+    size_t getMaxSpreadAsBloat() const override { return getDiskBloat(); }
     search::CacheStats getCacheStats() const override { return search::CacheStats(); }
     const vespalib::string &getBaseDir() const override { return _baseDir; }
     void accept(search::IDocumentStoreReadVisitor &,
diff --git a/searchcore/src/vespa/searchcore/proton/test/test.h b/searchcore/src/vespa/searchcore/proton/test/test.h
index 1494823e899..4231d5e7717 100644
--- a/searchcore/src/vespa/searchcore/proton/test/test.h
+++ b/searchcore/src/vespa/searchcore/proton/test/test.h
@@ -5,7 +5,6 @@
 #include "bucketdocuments.h"
 #include "bucketstatecalculator.h"
 #include "document.h"
-#include "dummy_document_store.h"
 #include "dummy_feed_view.h"
 #include "dummy_summary_manager.h"
 #include "resulthandler.h"