Merge pull request #20352 from vespa-engine/balder/disallow-old-style-single-uncompressed

Disallow the oldstyle compression with single uncopressed entries.

7 files changed, 52 insertions, 44 deletions

diff --git a/searchlib/src/tests/transactionlog/translogclient_test.cpp b/searchlib/src/tests/transactionlog/translogclient_test.cpp
index ab5d432ddfb..de8f9e5c462 100644
--- a/searchlib/src/tests/transactionlog/translogclient_test.cpp
+++ b/searchlib/src/tests/transactionlog/translogclient_test.cpp
@@ -5,6 +5,7 @@
 #include <vespa/vespalib/objects/identifiable.h>
 #include <vespa/searchlib/index/dummyfileheadercontext.h>
 #include <vespa/document/util/bytebuffer.h>
+#include <vespa/vespalib/util/exceptions.h>
 #include <vespa/fastos/file.h>
 #include <thread>
 
@@ -50,6 +51,12 @@ myhex(const void * b, size_t sz)
     return s;
 }
 
+DomainConfig
+createDomainConfig(uint32_t partSizeLimit) {
+    return DomainConfig().setPartSizeLimit(partSizeLimit)
+                         .setEncoding(Encoding(Encoding::xxh64, Encoding::none_multi));
+}
+
 class CallBackTest : public Callback
 {
 private:
@@ -461,7 +468,7 @@ void createAndFillDomain(const vespalib::string & name, Encoding encoding, size_
 {
     DummyFileHeaderContext fileHeaderContext;
     TransLogServer tlss("test13", 18377, ".", fileHeaderContext,
-                        DomainConfig().setPartSizeLimit(0x1000000).setEncoding(encoding), 4);
+                        createDomainConfig(0x1000000).setEncoding(encoding), 4);
     TransLogClient tls("tcp/localhost:18377");
 
     createDomainTest(tls, name, preExistingDomains);
@@ -471,7 +478,7 @@ void createAndFillDomain(const vespalib::string & name, Encoding encoding, size_
 
 void verifyDomain(const vespalib::string & name) {
     DummyFileHeaderContext fileHeaderContext;
-    TransLogServer tlss("test13", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x1000000));
+    TransLogServer tlss("test13", 18377, ".", fileHeaderContext, createDomainConfig(0x1000000));
     TransLogClient tls("tcp/localhost:18377");
     auto s1 = openDomainTest(tls, name);
     visitDomainTest(tls, s1.get(), name);
@@ -481,7 +488,7 @@ void verifyDomain(const vespalib::string & name) {
 
 TEST("testVisitOverGeneratedDomain") {
     DummyFileHeaderContext fileHeaderContext;
-    TransLogServer tlss("test7", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x10000));
+    TransLogServer tlss("test7", 18377, ".", fileHeaderContext, createDomainConfig(0x10000));
     TransLogClient tls("tcp/localhost:18377");
 
     vespalib::string name("test1");
@@ -498,7 +505,7 @@ TEST("testVisitOverGeneratedDomain") {
 TEST("testVisitOverPreExistingDomain") {
     // Depends on Test::testVisitOverGeneratedDomain()
     DummyFileHeaderContext fileHeaderContext;
-    TransLogServer tlss("test7", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x10000));
+    TransLogServer tlss("test7", 18377, ".", fileHeaderContext, createDomainConfig(0x10000));
     TransLogClient tls("tcp/localhost:18377");
 
     vespalib::string name("test1");
@@ -508,7 +515,7 @@ TEST("testVisitOverPreExistingDomain") {
 
 TEST("partialUpdateTest") {
     DummyFileHeaderContext fileHeaderContext;
-    TransLogServer tlss("test7", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x10000));
+    TransLogServer tlss("test7", 18377, ".", fileHeaderContext, createDomainConfig(0x10000));
     TransLogClient tls("tcp/localhost:18377");
 
     auto s1 = openDomainTest(tls, "test1");
@@ -562,16 +569,20 @@ TEST("partialUpdateTest") {
 }
 
 TEST("testCrcVersions") {
-    createAndFillDomain("ccitt_crc32", Encoding(Encoding::Crc::ccitt_crc32, Encoding::Compression::none), 0);
-    createAndFillDomain("xxh64", Encoding(Encoding::Crc::xxh64, Encoding::Compression::none), 1);
+    try {
+        createAndFillDomain("ccitt_crc32", Encoding(Encoding::Crc::ccitt_crc32, Encoding::Compression::none), 0);
+        ASSERT_TRUE(false);
+    } catch (vespalib::IllegalArgumentException & e) {
+        EXPECT_TRUE(e.getMessage().find("Compression:none is not allowed for the tls") != vespalib::string::npos);
+    }
+    createAndFillDomain("xxh64", Encoding(Encoding::Crc::xxh64, Encoding::Compression::zstd), 0);
 
-    verifyDomain("ccitt_crc32");
     verifyDomain("xxh64");
 }
 
 TEST("testRemove") {
     DummyFileHeaderContext fileHeaderContext;
-    TransLogServer tlss("testremove", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x10000));
+    TransLogServer tlss("testremove", 18377, ".", fileHeaderContext, createDomainConfig(0x10000));
     TransLogClient tls("tcp/localhost:18377");
 
     vespalib::string name("test-delete");
@@ -625,7 +636,7 @@ TEST("test sending a lot of data") {
     const vespalib::string MANY("many");
     {
         DummyFileHeaderContext fileHeaderContext;
-        TransLogServer tlss("test8", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x80000));
+        TransLogServer tlss("test8", 18377, ".", fileHeaderContext, createDomainConfig(0x80000));
         TransLogClient tls("tcp/localhost:18377");
 
         createDomainTest(tls, MANY, 0);
@@ -648,7 +659,7 @@ TEST("test sending a lot of data") {
     }
     {
         DummyFileHeaderContext fileHeaderContext;
-        TransLogServer tlss("test8", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x1000000));
+        TransLogServer tlss("test8", 18377, ".", fileHeaderContext, createDomainConfig(0x1000000));
         TransLogClient tls("tcp/localhost:18377");
 
         auto s1 = openDomainTest(tls, "many");
@@ -669,7 +680,7 @@ TEST("test sending a lot of data") {
     }
     {
         DummyFileHeaderContext fileHeaderContext;
-        TransLogServer tlss("test8", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x1000000));
+        TransLogServer tlss("test8", 18377, ".", fileHeaderContext, createDomainConfig(0x1000000));
         TransLogClient tls("tcp/localhost:18377");
 
         auto s1 = openDomainTest(tls, MANY);
@@ -697,7 +708,7 @@ TEST("test sending a lot of data async") {
     const vespalib::string MANY("many-async");
     {
         DummyFileHeaderContext fileHeaderContext;
-        TransLogServer tlss("test8", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x80000));
+        TransLogServer tlss("test8", 18377, ".", fileHeaderContext, createDomainConfig(0x80000));
         TransLogClient tls("tcp/localhost:18377");
         createDomainTest(tls, MANY, 1);
         auto s1 = openDomainTest(tls, MANY);
@@ -719,7 +730,7 @@ TEST("test sending a lot of data async") {
     }
     {
         DummyFileHeaderContext fileHeaderContext;
-        TransLogServer tlss("test8", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x1000000));
+        TransLogServer tlss("test8", 18377, ".", fileHeaderContext, createDomainConfig(0x1000000));
         TransLogClient tls("tcp/localhost:18377");
 
         auto s1 = openDomainTest(tls, MANY);
@@ -749,7 +760,7 @@ TEST("testErase") {
     const unsigned int TOTAL_NUM_ENTRIES = NUM_PACKETS * NUM_ENTRIES;
     {
         DummyFileHeaderContext fileHeaderContext;
-        TransLogServer tlss("test12", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x80000));
+        TransLogServer tlss("test12", 18377, ".", fileHeaderContext, createDomainConfig(0x80000));
         TransLogClient tls("tcp/localhost:18377");
 
         createDomainTest(tls, "erase", 0);
@@ -758,7 +769,7 @@ TEST("testErase") {
     }
     {
         DummyFileHeaderContext fileHeaderContext;
-        TransLogServer tlss("test12", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x1000000));
+        TransLogServer tlss("test12", 18377, ".", fileHeaderContext, createDomainConfig(0x1000000));
         TransLogClient tls("tcp/localhost:18377");
 
         auto s1 = openDomainTest(tls, "erase");
@@ -845,7 +856,7 @@ TEST("testSync") {
     const unsigned int TOTAL_NUM_ENTRIES = NUM_PACKETS * NUM_ENTRIES;
 
     DummyFileHeaderContext fileHeaderContext;
-    TransLogServer tlss("test9", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x1000000));
+    TransLogServer tlss("test9", 18377, ".", fileHeaderContext, createDomainConfig(0x1000000));
     TransLogClient tls("tcp/localhost:18377");
 
     createDomainTest(tls, "sync", 0);
@@ -867,7 +878,7 @@ TEST("test truncate on version mismatch") {
     size_t countOld(0);
     DummyFileHeaderContext fileHeaderContext;
     {
-        TransLogServer tlss("test11", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x1000000));
+        TransLogServer tlss("test11", 18377, ".", fileHeaderContext, createDomainConfig(0x1000000));
         TransLogClient tls("tcp/localhost:18377");
 
         createDomainTest(tls, "sync", 0);
@@ -888,7 +899,7 @@ TEST("test truncate on version mismatch") {
     EXPECT_EQUAL(static_cast<ssize_t>(sizeof(tmp)), f.Write2(tmp, sizeof(tmp)));
     EXPECT_TRUE(f.Close());
     {
-        TransLogServer tlss("test11", 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x10000));
+        TransLogServer tlss("test11", 18377, ".", fileHeaderContext, createDomainConfig(0x10000));
         TransLogClient tls("tcp/localhost:18377");
         auto s1 = openDomainTest(tls, "sync");
         uint64_t from(0), to(0);
@@ -910,9 +921,10 @@ TEST("test truncation after short read") {
     vespalib::string dir(topdir + "/" + domain);
     vespalib::string tlsspec("tcp/localhost:18377");
 
+    DomainConfig domainConfig = createDomainConfig(0x10000);
     DummyFileHeaderContext fileHeaderContext;
     {
-        TransLogServer tlss(topdir, 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x10000));
+        TransLogServer tlss(topdir, 18377, ".", fileHeaderContext, domainConfig);
         TransLogClient tls(tlsspec);
         
         createDomainTest(tls, domain, 0);
@@ -924,18 +936,14 @@ TEST("test truncation after short read") {
         EXPECT_TRUE(s1->sync(TOTAL_NUM_ENTRIES, syncedTo));
         EXPECT_EQUAL(syncedTo, TOTAL_NUM_ENTRIES);
     }
+    EXPECT_EQUAL(2u, countFiles(dir));
     {
-        EXPECT_EQUAL(2u, countFiles(dir));
-    }
-    {
-        TransLogServer tlss(topdir, 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x10000));
+        TransLogServer tlss(topdir, 18377, ".", fileHeaderContext, domainConfig);
         TransLogClient tls(tlsspec);
         auto s1 = openDomainTest(tls, domain);
         checkFilledDomainTest(*s1, TOTAL_NUM_ENTRIES);
     }
-    {
-        EXPECT_EQUAL(2u, countFiles(dir));
-    }
+    EXPECT_EQUAL(2u, countFiles(dir));
     {
         vespalib::string filename(dir + "/truncate-0000000000000017");
         FastOS_File trfile(filename.c_str());
@@ -944,14 +952,12 @@ TEST("test truncation after short read") {
         trfile.Close();
     }
     {
-        TransLogServer tlss(topdir, 18377, ".", fileHeaderContext, DomainConfig().setPartSizeLimit(0x10000));
+        TransLogServer tlss(topdir, 18377, ".", fileHeaderContext, domainConfig);
         TransLogClient tls(tlsspec);
         auto s1 = openDomainTest(tls, domain);
         checkFilledDomainTest(*s1, TOTAL_NUM_ENTRIES - 1);
     }
-    {
-        EXPECT_EQUAL(2u, countFiles(dir));
-    }
+    EXPECT_EQUAL(2u, countFiles(dir));
 }
 
 TEST_MAIN() { TEST_RUN_ALL(); }
diff --git a/searchlib/src/vespa/searchlib/transactionlog/chunks.cpp b/searchlib/src/vespa/searchlib/transactionlog/chunks.cpp
index ac17c47ef38..cdd58adf005 100644
--- a/searchlib/src/vespa/searchlib/transactionlog/chunks.cpp
+++ b/searchlib/src/vespa/searchlib/transactionlog/chunks.cpp
@@ -43,9 +43,8 @@ toCompression(CompressionConfig::Type type) {
         case CompressionConfig::LZ4:
             return Encoding::Compression::lz4;
         case CompressionConfig::NONE_MULTI:
-            return Encoding::Compression::none_multi;
         case CompressionConfig::NONE:
-            return Encoding::Compression::none;
+            return Encoding::Compression::none_multi;
         default:
             abort();
     }
@@ -114,9 +113,6 @@ XXH64CompressedChunk::compress(nbostream & os, Encoding::Crc crc) const {
     CompressionConfig cfg(_type, _level, 80, 200);
     ConstBufferRef uncompressed(org.data(), org.size());
     Encoding::Compression actual = toCompression(::compress(cfg, uncompressed, compressed, false));
-    if (actual == Encoding::Compression::none) {
-        actual = Encoding::Compression::none_multi;
-    }
     os << uint32_t(uncompressed.size());
     size_t start = os.wp();
     os.write(compressed.getData(), compressed.getDataLen());
diff --git a/searchlib/src/vespa/searchlib/transactionlog/domainconfig.cpp b/searchlib/src/vespa/searchlib/transactionlog/domainconfig.cpp
index b4ab8a1c791..1f414eaa6d3 100644
--- a/searchlib/src/vespa/searchlib/transactionlog/domainconfig.cpp
+++ b/searchlib/src/vespa/searchlib/transactionlog/domainconfig.cpp
@@ -1,15 +1,25 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "domainconfig.h"
+#include <vespa/vespalib/util/exceptions.h>
 
 namespace search::transactionlog {
 
 DomainConfig::DomainConfig()
-    : _encoding(Encoding::Crc::xxh64, Encoding::Compression::none),
+    : _encoding(Encoding::Crc::xxh64, Encoding::Compression::zstd),
       _compressionLevel(9),
       _fSyncOnCommit(false),
       _partSizeLimit(0x10000000), // 256M
       _chunkSizeLimit(0x40000)   // 256k
 { }
 
+DomainConfig &
+DomainConfig::setEncoding(Encoding v) {
+    if (v.getCompression() == Encoding::none) {
+        throw vespalib::IllegalArgumentException("Compression:none is not allowed for the tls", VESPA_STRLOC);
+    }
+    _encoding = v;
+    return *this;
+}
+
 }
diff --git a/searchlib/src/vespa/searchlib/transactionlog/domainconfig.h b/searchlib/src/vespa/searchlib/transactionlog/domainconfig.h
index 186227ae958..7701896fa92 100644
--- a/searchlib/src/vespa/searchlib/transactionlog/domainconfig.h
+++ b/searchlib/src/vespa/searchlib/transactionlog/domainconfig.h
@@ -11,7 +11,7 @@ class DomainConfig {
 public:
     using duration = vespalib::duration;
     DomainConfig();
-    DomainConfig & setEncoding(Encoding v)          { _encoding = v; return *this; }
+    DomainConfig & setEncoding(Encoding v);
     DomainConfig & setPartSizeLimit(size_t v)       { _partSizeLimit = v; return *this; }
     DomainConfig & setChunkSizeLimit(size_t v)      { _chunkSizeLimit = v; return *this; }
     DomainConfig & setCompressionLevel(uint8_t v)   { _compressionLevel = v; return *this; }
diff --git a/searchlib/src/vespa/searchlib/transactionlog/domainpart.cpp b/searchlib/src/vespa/searchlib/transactionlog/domainpart.cpp
index ee575820cce..3dad67df177 100644
--- a/searchlib/src/vespa/searchlib/transactionlog/domainpart.cpp
+++ b/searchlib/src/vespa/searchlib/transactionlog/domainpart.cpp
@@ -395,10 +395,7 @@ DomainPart::commit(SerialNum firstSerial, const Packet &packet)
         entry.deserialize(h);
         if (_range.to() < entry.serial()) {
             chunk->add(entry);
-            if (_encoding.getCompression() == Encoding::Compression::none) {
-                write(*_transLog, *chunk);
-                chunk = IChunk::create(_encoding, _compressionLevel);
-            }
+            assert(_encoding.getCompression() != Encoding::Compression::none);
             _sz++;
             _range.to(entry.serial());
         } else {
diff --git a/searchlib/src/vespa/searchlib/transactionlog/translogserver.cpp b/searchlib/src/vespa/searchlib/transactionlog/translogserver.cpp
index 08ee944e749..ce190d2c093 100644
--- a/searchlib/src/vespa/searchlib/transactionlog/translogserver.cpp
+++ b/searchlib/src/vespa/searchlib/transactionlog/translogserver.cpp
@@ -82,7 +82,7 @@ VESPA_THREAD_STACK_TAG(tls_executor);
 TransLogServer::TransLogServer(const vespalib::string &name, int listenPort, const vespalib::string &baseDir,
                                const FileHeaderContext &fileHeaderContext)
     : TransLogServer(name, listenPort, baseDir, fileHeaderContext,
-                     DomainConfig().setEncoding(Encoding(Encoding::xxh64, Encoding::Compression::none))
+                     DomainConfig().setEncoding(Encoding(Encoding::xxh64, Encoding::Compression::zstd))
                                         .setPartSizeLimit(0x10000000).setChunkSizeLimit(0x40000))
 {}
 
diff --git a/searchlib/src/vespa/searchlib/transactionlog/translogserverapp.cpp b/searchlib/src/vespa/searchlib/transactionlog/translogserverapp.cpp
index 2d2863af874..9ca3b678054 100644
--- a/searchlib/src/vespa/searchlib/transactionlog/translogserverapp.cpp
+++ b/searchlib/src/vespa/searchlib/transactionlog/translogserverapp.cpp
@@ -42,7 +42,6 @@ getCompression(searchlib::TranslogserverConfig::Compression::Type type)
 {
     switch (type) {
         case searchlib::TranslogserverConfig::Compression::Type::NONE:
-            return Encoding::Compression::none;
         case searchlib::TranslogserverConfig::Compression::Type::NONE_MULTI:
             return Encoding::Compression::none_multi;
         case searchlib::TranslogserverConfig::Compression::Type::LZ4: