This is an automated email from the ASF dual-hosted git repository. adar pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 37c3a960e9b0f86069876f049e9a073d4b5fd907 Author: Adar Dembo <[email protected]> AuthorDate: Mon Jan 20 00:43:07 2020 -0800 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 <[email protected]> Reviewed-by: Alexey Serbin <[email protected]> --- src/kudu/cfile/cfile_writer.cc | 8 ++------ src/kudu/consensus/log-test.cc | 4 ++-- src/kudu/consensus/log.cc | 11 +++-------- src/kudu/integration-tests/disk_reservation-itest.cc | 2 +- src/kudu/integration-tests/raft_consensus-itest-base.cc | 3 +-- src/kudu/integration-tests/raft_consensus-itest.cc | 4 ++-- src/kudu/integration-tests/ts_recovery-itest.cc | 2 +- src/kudu/util/compression/compression_codec.cc | 2 +- 8 files changed, 13 insertions(+), 23 deletions(-) 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 @@ const size_t kChecksumSize = sizeof(uint32_t); static const size_t kMinBlockSize = 512; -static CompressionType GetDefaultCompressionCodec() { - return GetCompressionCodecType(FLAGS_cfile_default_compression_codec); -} - //////////////////////////////////////////////////////////// // CFileWriter //////////////////////////////////////////////////////////// @@ -114,7 +110,7 @@ CFileWriter::CFileWriter(WriterOptions options, 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_P(LogTestOptionalCompression, TestSegmentRollover) { } 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_P(LogTestOptionalCompression, TestReadReplicatesHighIndex) { // 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 @@ Status SegmentAllocator::Init( 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 @@ TEST_F(DiskReservationITest, TestWalWriteToFullDiskAborts) { "--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 @@ void RaftConsensusITestBase::AddFlagsForLogRolls(vector<string>* extra_tserver_f // // 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 @@ TEST_F(RaftConsensusITest, TestCatchupAfterOpsEvicted) { // 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 @@ TEST_F(RaftConsensusITest, TestLargeBatches) { // 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 @@ TEST_F(TsRecoveryITest, TestTabletRecoveryAfterSegmentDelete) { 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 @@ CompressionType GetCompressionCodecType(const std::string& name) { 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
