kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a...@apache.org
Subject [kudu] 02/04: compression: fix handling of NO_COMPRESSION
Date Tue, 21 Jan 2020 22:41:10 GMT
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 <adar@cloudera.com>
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 <awong@cloudera.com>
    Reviewed-by: Alexey Serbin <aserbin@cloudera.com>
---
 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


Mime
View raw message