compression: fix handling of NO_COMPRESSION
The string values of CompressionType and GetCompressionCodecType() did not
agree: the former used NO_COMPRESSION and the latter NONE to indicate the
lack of compression. This led to some unnecessary warnings when a
stringified CompressionType was fed into GetCompressionCodecType(), as is
done in log-test.
This patch changes GetCompressionCodecType() to expect NO_COMPRESSION rather
than NONE. It shouldn't affect backwards compatibility: if someone really
does use NONE (i.e. in a gflag), they'll just get no compression anyway,
albeit with the ugly warning. That's not ideal, but the alternative (use
NONE in CompressionType) may break backwards compatibility in JSON encoding,
and NO_COMPRESSION is the value we use in our public APIs.
Change-Id: I900458b7c7ed4be02906479becaaf60bad379029
Reviewed-on: http://gerrit.cloudera.org:8080/15078
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong <awong@cloudera.com>
Reviewed-by: Alexey Serbin <aserbin@cloudera.com>
diff --git a/src/kudu/cfile/cfile_writer.cc b/src/kudu/cfile/cfile_writer.cc
index 5769e10..aa6f85d 100644
--- a/src/kudu/cfile/cfile_writer.cc
+++ b/src/kudu/cfile/cfile_writer.cc
@@ -52,7 +52,7 @@
DEFINE_int32(cfile_default_block_size, 256*1024, "The default block size to use in cfiles");
TAG_FLAG(cfile_default_block_size, advanced);
-DEFINE_string(cfile_default_compression_codec, "none",
+DEFINE_string(cfile_default_compression_codec, "no_compression",
"Default cfile block compression codec.");
TAG_FLAG(cfile_default_compression_codec, advanced);
@@ -80,10 +80,6 @@
static const size_t kMinBlockSize = 512;
-static CompressionType GetDefaultCompressionCodec() {
- return GetCompressionCodecType(FLAGS_cfile_default_compression_codec);
-}
-
////////////////////////////////////////////////////////////
// CFileWriter
////////////////////////////////////////////////////////////
@@ -114,7 +110,7 @@
compression_ = options_.storage_attributes.compression;
if (compression_ == DEFAULT_COMPRESSION) {
- compression_ = GetDefaultCompressionCodec();
+ compression_ = GetCompressionCodecType(FLAGS_cfile_default_compression_codec);
}
if (options_.storage_attributes.cfile_block_size <= 0) {
diff --git a/src/kudu/consensus/log-test.cc b/src/kudu/consensus/log-test.cc
index 4a68189..653ed00 100644
--- a/src/kudu/consensus/log-test.cc
+++ b/src/kudu/consensus/log-test.cc
@@ -430,7 +430,7 @@
}
TEST_F(LogTest, TestWriteAndReadToAndFromInProgressSegment) {
- FLAGS_log_compression_codec = "none";
+ FLAGS_log_compression_codec = "no_compression";
const int kNumEntries = 4;
ASSERT_OK(BuildLog());
@@ -1054,7 +1054,7 @@
// Test various situations where we expect different segments depending on what the
// min log index is.
TEST_F(LogTest, TestGetGCableDataSize) {
- FLAGS_log_compression_codec = "none";
+ FLAGS_log_compression_codec = "no_compression";
FLAGS_log_min_segments_to_retain = 2;
ASSERT_OK(BuildLog());
diff --git a/src/kudu/consensus/log.cc b/src/kudu/consensus/log.cc
index 54400d2..429036f 100644
--- a/src/kudu/consensus/log.cc
+++ b/src/kudu/consensus/log.cc
@@ -42,7 +42,6 @@
#include "kudu/gutil/strings/substitute.h"
#include "kudu/gutil/walltime.h"
#include "kudu/util/async_util.h"
-#include "kudu/util/compression/compression.pb.h"
#include "kudu/util/compression/compression_codec.h"
#include "kudu/util/debug/trace_event.h"
#include "kudu/util/env.h"
@@ -448,13 +447,9 @@
uint64_t sequence_number,
scoped_refptr<ReadableLogSegment>* new_readable_segment) {
// Init the compression codec.
- if (!FLAGS_log_compression_codec.empty()) {
- auto codec_type = GetCompressionCodecType(FLAGS_log_compression_codec);
- if (codec_type != NO_COMPRESSION) {
- RETURN_NOT_OK_PREPEND(GetCompressionCodec(codec_type, &codec_),
- "could not instantiate compression codec");
- }
- }
+ RETURN_NOT_OK_PREPEND(GetCompressionCodec(
+ GetCompressionCodecType(FLAGS_log_compression_codec), &codec_),
+ "could not instantiate compression codec");
active_segment_sequence_number_ = sequence_number;
RETURN_NOT_OK(ThreadPoolBuilder("log-alloc")
.set_max_threads(1)
diff --git a/src/kudu/integration-tests/disk_reservation-itest.cc b/src/kudu/integration-tests/disk_reservation-itest.cc
index 5f42295..90a2adc 100644
--- a/src/kudu/integration-tests/disk_reservation-itest.cc
+++ b/src/kudu/integration-tests/disk_reservation-itest.cc
@@ -131,7 +131,7 @@
"--disable_core_dumps",
// Disable compression so that our data being written doesn't end up
// compressed away.
- "--log_compression_codec=none"
+ "--log_compression_codec=no_compression"
};
NO_FATALS(StartCluster(ts_flags, {}, 1));
diff --git a/src/kudu/integration-tests/raft_consensus-itest-base.cc b/src/kudu/integration-tests/raft_consensus-itest-base.cc
index b357dcc..ef28531 100644
--- a/src/kudu/integration-tests/raft_consensus-itest-base.cc
+++ b/src/kudu/integration-tests/raft_consensus-itest-base.cc
@@ -26,7 +26,6 @@
#include <vector>
#include <gflags/gflags.h>
-#include <gflags/gflags_declare.h>
#include <glog/logging.h>
#include <gtest/gtest.h>
@@ -181,7 +180,7 @@
//
// Additionally, we disable log compression, since these tests write a lot of
// repetitive data to cause the rolls, and compression would make it all tiny.
- extra_tserver_flags->push_back("--log_compression_codec=none");
+ extra_tserver_flags->push_back("--log_compression_codec=no_compression");
extra_tserver_flags->push_back("--log_cache_size_limit_mb=1");
extra_tserver_flags->push_back("--log_segment_size_mb=1");
extra_tserver_flags->push_back("--log_async_preallocate_segments=false");
diff --git a/src/kudu/integration-tests/raft_consensus-itest.cc b/src/kudu/integration-tests/raft_consensus-itest.cc
index 9072b2d..dfe2940 100644
--- a/src/kudu/integration-tests/raft_consensus-itest.cc
+++ b/src/kudu/integration-tests/raft_consensus-itest.cc
@@ -815,7 +815,7 @@
// We write 128KB cells in this test, so bump the limit.
"--max_cell_size_bytes=1000000",
// And disable WAL compression so the 128KB cells don't get compressed away.
- "--log_compression_codec=none"
+ "--log_compression_codec=no_compression"
};
NO_FATALS(BuildAndStart(kTsFlags));
@@ -2140,7 +2140,7 @@
// We write 128KB cells in this test, so bump the limit, and disable compression.
"--max_cell_size_bytes=1000000",
"--log_segment_size_mb=1",
- "--log_compression_codec=none",
+ "--log_compression_codec=no_compression",
"--log_min_segments_to_retain=100", // disable GC of logs.
};
diff --git a/src/kudu/integration-tests/ts_recovery-itest.cc b/src/kudu/integration-tests/ts_recovery-itest.cc
index 6be498e..36b9ea8 100644
--- a/src/kudu/integration-tests/ts_recovery-itest.cc
+++ b/src/kudu/integration-tests/ts_recovery-itest.cc
@@ -153,7 +153,7 @@
vector<string> flags;
flags.emplace_back("--log_segment_size_mb=1");
flags.emplace_back("--log_min_segments_to_retain=3");
- flags.emplace_back("--log_compression_codec=''");
+ flags.emplace_back("--log_compression_codec=no_compression");
NO_FATALS(StartCluster(flags));
const int kNumTablets = 1;
diff --git a/src/kudu/util/compression/compression_codec.cc b/src/kudu/util/compression/compression_codec.cc
index b927d48..dc69311 100644
--- a/src/kudu/util/compression/compression_codec.cc
+++ b/src/kudu/util/compression/compression_codec.cc
@@ -276,7 +276,7 @@
return LZ4;
if (uname == "ZLIB")
return ZLIB;
- if (uname == "NONE")
+ if (uname == "NO_COMPRESSION")
return NO_COMPRESSION;
LOG(WARNING) << "Unable to recognize the compression codec '" << name
