Remove compression support (facebook#14266)

Summary:
Pull Request resolved: facebook#14266

Compression is unused in production, making this dead code that adds unnecessary complexity.

Core changes:
- Remove `compression` field from `BlobDBOptions`
- Remove compression/decompression methods (`GetCompressedSlice`,
  `DecompressSlice`, `BlobDecompressor`)
- Simplify `ReadBlobFromOldFile` and `GetBlobValue` to handle only
  uncompressed blobs
- Update compaction filter to skip compression/decompression

CLI tool cleanup:
- Remove `--blob_db_compression_type` flag from db_bench (Stacked BlobDB)
- Remove `--show_uncompressed_blob` from blob_dump tool
- Remove `--dump_uncompressed_blobs` from ldb dump/file_dump commands
- blob_dump_tool now fails fast with NotSupported for compressed files

Tests:
- Remove compression-related tests (`Compression`, `DecompressAfterReopen`,
  `EnableDisableCompressionGC`, `ChangeCompressionGC`)

Reviewed By: xingbowang

Differential Revision: D91088957

fbshipit-source-id: 496ee41dcbd0023b794aa8a6d7dcc9c2451b7470

Author: mszeszko-meta

include/rocksdb/utilities/ldb_cmd.h

@@ -71,7 +71,6 @@ class LDBCommand {
   static const std::string ARG_BLOB_FILE_STARTING_LEVEL;
   static const std::string ARG_PREPOPULATE_BLOB_CACHE;
   static const std::string ARG_DECODE_BLOB_INDEX;
-  static const std::string ARG_DUMP_UNCOMPRESSED_BLOBS;
   static const std::string ARG_READ_TIMESTAMP;
   static const std::string ARG_GET_WRITE_UNIX_TIME;
 

tools/blob_dump.cc

@@ -27,12 +27,10 @@ int main(int argc, char** argv) {
       {"file", required_argument, nullptr, 'f'},
       {"show_key", optional_argument, nullptr, 'k'},
       {"show_blob", optional_argument, nullptr, 'b'},
-      {"show_uncompressed_blob", optional_argument, nullptr, 'r'},
       {"show_summary", optional_argument, nullptr, 's'},
   };
   DisplayType show_key = DisplayType::kRaw;
   DisplayType show_blob = DisplayType::kNone;
-  DisplayType show_uncompressed_blob = DisplayType::kNone;
   bool show_summary = false;
   std::string file;
   while (true) {
@@ -47,7 +45,6 @@ int main(int argc, char** argv) {
                 "Usage: blob_dump --file=filename "
                 "[--show_key[=none|raw|hex|detail]] "
                 "[--show_blob[=none|raw|hex|detail]] "
-                "[--show_uncompressed_blob[=none|raw|hex|detail]] "
                 "[--show_summary]\n");
         return 0;
       case 'f':
@@ -73,17 +70,6 @@ int main(int argc, char** argv) {
           show_blob = DisplayType::kHex;
         }
         break;
-      case 'r':
-        if (optarg) {
-          if (display_types.count(arg_str) == 0) {
-            fprintf(stderr, "Unrecognized blob display type.\n");
-            return -1;
-          }
-          show_uncompressed_blob = display_types.at(arg_str);
-        } else {
-          show_uncompressed_blob = DisplayType::kHex;
-        }
-        break;
       case 's':
         show_summary = true;
         break;
@@ -93,8 +79,7 @@ int main(int argc, char** argv) {
     }
   }
   BlobDumpTool tool;
-  Status s =
-      tool.Run(file, show_key, show_blob, show_uncompressed_blob, show_summary);
+  Status s = tool.Run(file, show_key, show_blob, show_summary);
   if (!s.ok()) {
     fprintf(stderr, "Failed: %s\n", s.ToString().c_str());
     return -1;

tools/db_bench_tool.cc

@@ -1087,12 +1087,6 @@ DEFINE_uint64(blob_db_file_size,
               ROCKSDB_NAMESPACE::blob_db::BlobDBOptions().blob_file_size,
               "[Stacked BlobDB] Target size of each blob file.");
 
-DEFINE_string(
-    blob_db_compression_type, "snappy",
-    "[Stacked BlobDB] Algorithm to use to compress blobs in blob files.");
-static enum ROCKSDB_NAMESPACE::CompressionType
-    FLAGS_blob_db_compression_type_e = ROCKSDB_NAMESPACE::kSnappyCompression;
-
 // Integrated BlobDB options
 DEFINE_bool(
     enable_blob_files,
@@ -5204,7 +5198,6 @@ class Benchmark {
       blob_db_options.min_blob_size = FLAGS_blob_db_min_blob_size;
       blob_db_options.bytes_per_sync = FLAGS_blob_db_bytes_per_sync;
       blob_db_options.blob_file_size = FLAGS_blob_db_file_size;
-      blob_db_options.compression = FLAGS_blob_db_compression_type_e;
       blob_db::BlobDB* ptr = nullptr;
       s = hooks.Open(options, blob_db_options, db_name, &ptr);
       if (s.ok()) {
@@ -9191,10 +9184,6 @@ int db_bench_tool(int argc, char** argv, ToolHooks& hooks) {
   FLAGS_compressed_secondary_cache_compression_type_e = StringToCompressionType(
       FLAGS_compressed_secondary_cache_compression_type.c_str());
 
-  // Stacked BlobDB
-  FLAGS_blob_db_compression_type_e =
-      StringToCompressionType(FLAGS_blob_db_compression_type.c_str());
-
   int env_opts = !FLAGS_env_uri.empty() + !FLAGS_fs_uri.empty();
   if (env_opts > 1) {
     fprintf(stderr, "Error: --env_uri and --fs_uri are mutually exclusive\n");

tools/ldb_cmd.cc

@@ -110,8 +110,6 @@ const std::string LDBCommand::ARG_BLOB_FILE_STARTING_LEVEL =
 const std::string LDBCommand::ARG_PREPOPULATE_BLOB_CACHE =
     "prepopulate_blob_cache";
 const std::string LDBCommand::ARG_DECODE_BLOB_INDEX = "decode_blob_index";
-const std::string LDBCommand::ARG_DUMP_UNCOMPRESSED_BLOBS =
-    "dump_uncompressed_blobs";
 const std::string LDBCommand::ARG_READ_TIMESTAMP = "read_timestamp";
 const std::string LDBCommand::ARG_GET_WRITE_UNIX_TIME = "get_write_unix_time";
 
@@ -201,7 +199,7 @@ void DumpSstFile(Options options, std::string filename, bool output_hex,
                  std::string from_key = "", std::string to_key = "");
 
 void DumpBlobFile(const std::string& filename, bool is_key_hex,
-                  bool is_value_hex, bool dump_uncompressed_blobs);
+                  bool is_value_hex);
 
 Status EncodeUserProvidedTimestamp(const std::string& user_timestamp,
                                    std::string* ts_buf);
@@ -2288,13 +2286,12 @@ DBDumperCommand::DBDumperCommand(
     const std::vector<std::string>& /*params*/,
     const std::map<std::string, std::string>& options,
     const std::vector<std::string>& flags)
-    : LDBCommand(
-          options, flags, true /* is_read_only */,
-          BuildCmdLineOptions(
-              {ARG_TTL, ARG_HEX, ARG_KEY_HEX, ARG_VALUE_HEX, ARG_FROM, ARG_TO,
-               ARG_MAX_KEYS, ARG_COUNT_ONLY, ARG_COUNT_DELIM, ARG_STATS,
-               ARG_TTL_START, ARG_TTL_END, ARG_TTL_BUCKET, ARG_TIMESTAMP,
-               ARG_PATH, ARG_DECODE_BLOB_INDEX, ARG_DUMP_UNCOMPRESSED_BLOBS})),
+    : LDBCommand(options, flags, true /* is_read_only */,
+                 BuildCmdLineOptions(
+                     {ARG_TTL, ARG_HEX, ARG_KEY_HEX, ARG_VALUE_HEX, ARG_FROM,
+                      ARG_TO, ARG_MAX_KEYS, ARG_COUNT_ONLY, ARG_COUNT_DELIM,
+                      ARG_STATS, ARG_TTL_START, ARG_TTL_END, ARG_TTL_BUCKET,
+                      ARG_TIMESTAMP, ARG_PATH, ARG_DECODE_BLOB_INDEX})),
       null_from_(true),
       null_to_(true),
       max_keys_(-1),
@@ -2342,7 +2339,6 @@ DBDumperCommand::DBDumperCommand(
   print_stats_ = IsFlagPresent(flags, ARG_STATS);
   count_only_ = IsFlagPresent(flags, ARG_COUNT_ONLY);
   decode_blob_index_ = IsFlagPresent(flags, ARG_DECODE_BLOB_INDEX);
-  dump_uncompressed_blobs_ = IsFlagPresent(flags, ARG_DUMP_UNCOMPRESSED_BLOBS);
 
   if (is_key_hex_) {
     if (!null_from_) {
@@ -2377,7 +2373,6 @@ void DBDumperCommand::Help(std::string& ret) {
   ret.append(" [--" + ARG_TTL_END + "=<N>:- is exclusive]");
   ret.append(" [--" + ARG_PATH + "=<path_to_a_file>]");
   ret.append(" [--" + ARG_DECODE_BLOB_INDEX + "]");
-  ret.append(" [--" + ARG_DUMP_UNCOMPRESSED_BLOBS + "]");
   ret.append("\n");
 }
 
@@ -2424,8 +2419,7 @@ void DBDumperCommand::DoCommand() {
                          /*  json_ */ false, column_families_);
         break;
       case kBlobFile:
-        DumpBlobFile(path_, is_key_hex_, is_value_hex_,
-                     dump_uncompressed_blobs_);
+        DumpBlobFile(path_, is_key_hex_, is_value_hex_);
         break;
       default:
         exec_state_ = LDBCommandExecuteResult::Failed(
@@ -4718,22 +4712,16 @@ void DumpSstFile(Options options, std::string filename, bool output_hex,
 }
 
 void DumpBlobFile(const std::string& filename, bool is_key_hex,
-                  bool is_value_hex, bool dump_uncompressed_blobs) {
+                  bool is_value_hex) {
   using ROCKSDB_NAMESPACE::blob_db::BlobDumpTool;
   BlobDumpTool tool;
-  BlobDumpTool::DisplayType blob_type = is_value_hex
+  BlobDumpTool::DisplayType show_blob = is_value_hex
                                             ? BlobDumpTool::DisplayType::kHex
                                             : BlobDumpTool::DisplayType::kRaw;
-  BlobDumpTool::DisplayType show_uncompressed_blob =
-      dump_uncompressed_blobs ? blob_type : BlobDumpTool::DisplayType::kNone;
-  BlobDumpTool::DisplayType show_blob =
-      dump_uncompressed_blobs ? BlobDumpTool::DisplayType::kNone : blob_type;
-
   BlobDumpTool::DisplayType show_key = is_key_hex
                                            ? BlobDumpTool::DisplayType::kHex
                                            : BlobDumpTool::DisplayType::kRaw;
-  Status s = tool.Run(filename, show_key, show_blob, show_uncompressed_blob,
-                      /* show_summary */ true);
+  Status s = tool.Run(filename, show_key, show_blob, /* show_summary */ true);
   if (!s.ok()) {
     fprintf(stderr, "Failed: %s\n", s.ToString().c_str());
   }
@@ -4757,17 +4745,13 @@ DBFileDumperCommand::DBFileDumperCommand(
     const std::map<std::string, std::string>& options,
     const std::vector<std::string>& flags)
     : LDBCommand(options, flags, true /* is_read_only */,
-                 BuildCmdLineOptions(
-                     {ARG_DECODE_BLOB_INDEX, ARG_DUMP_UNCOMPRESSED_BLOBS})),
-      decode_blob_index_(IsFlagPresent(flags, ARG_DECODE_BLOB_INDEX)),
-      dump_uncompressed_blobs_(
-          IsFlagPresent(flags, ARG_DUMP_UNCOMPRESSED_BLOBS)) {}
+                 BuildCmdLineOptions({ARG_DECODE_BLOB_INDEX})),
+      decode_blob_index_(IsFlagPresent(flags, ARG_DECODE_BLOB_INDEX)) {}
 
 void DBFileDumperCommand::Help(std::string& ret) {
   ret.append("  ");
   ret.append(DBFileDumperCommand::Name());
-  ret.append(" [--" + ARG_DECODE_BLOB_INDEX + "] ");
-  ret.append(" [--" + ARG_DUMP_UNCOMPRESSED_BLOBS + "] ");
+  ret.append(" [--" + ARG_DECODE_BLOB_INDEX + "]");
   ret.append("\n");
 }
 
@@ -4835,8 +4819,7 @@ void DBFileDumperCommand::DoCommand() {
       filename = NormalizePath(filename);
       std::cout << filename << std::endl;
       std::cout << "------------------------------" << std::endl;
-      DumpBlobFile(filename, /* is_key_hex */ false, /* is_value_hex */ false,
-                   dump_uncompressed_blobs_);
+      DumpBlobFile(filename, /* is_key_hex */ false, /* is_value_hex */ false);
       std::cout << std::endl;
     }
   }

tools/ldb_cmd_impl.h

@@ -47,7 +47,6 @@ class DBFileDumperCommand : public LDBCommand {
 
  private:
   bool decode_blob_index_;
-  bool dump_uncompressed_blobs_;
 };
 
 class DBLiveFilesMetadataDumperCommand : public LDBCommand {
@@ -109,7 +108,6 @@ class DBDumperCommand : public LDBCommand {
   bool print_stats_;
   std::string path_;
   bool decode_blob_index_;
-  bool dump_uncompressed_blobs_;
 
   static const std::string ARG_COUNT_ONLY;
   static const std::string ARG_COUNT_DELIM;

tools/ldb_test.py

@@ -613,7 +613,7 @@ def testDumpLiveFiles(self):
         # Call the dump_live_files function with the edited dbPath name.
         self.assertTrue(
             self.dumpLiveFiles(
-                "--db=%s --decode_blob_index --dump_uncompressed_blobs" % dbPath,
+                "--db=%s --decode_blob_index" % dbPath,
                 dumpFilePath,
             )
         )
@@ -881,7 +881,7 @@ def testBlobDump(self):
         expected_pattern = re.compile(regex)
         blob_files = self.getBlobFiles(dbPath)
         self.assertTrue(len(blob_files) >= 1)
-        cmd = "dump --path=%s --dump_uncompressed_blobs"
+        cmd = "dump --path=%s"
         self.assertRunOKFull(
             (cmd) % (blob_files[0]), expected_pattern, unexpected=False, isPattern=True
         )

utilities/blob_db/blob_compaction_filter.cc

@@ -94,10 +94,7 @@ CompactionFilter::Decision BlobIndexCompactionFilterBase::FilterV2(
     }
     // Read value from blob file.
     PinnableSlice blob;
-    CompressionType compression_type = kNoCompression;
-    constexpr bool need_decompress = true;
-    if (!ReadBlobFromOldFile(ikey.user_key, blob_index, &blob, need_decompress,
-                             &compression_type)) {
+    if (!ReadBlobFromOldFile(ikey.user_key, blob_index, &blob)) {
       return Decision::kIOError;
     }
     CompactionFilter::Decision decision = ucf->FilterV2(
@@ -123,15 +120,6 @@ CompactionFilter::Decision BlobIndexCompactionFilterBase::HandleValueChange(
     return Decision::kIOError;
   }
   Slice new_blob_value(*new_value);
-  GrowableBuffer compressed_output;
-  if (blob_db_impl->bdb_options_.compression != kNoCompression) {
-    Status s = blob_db_impl->CompressBlob(new_blob_value, &compressed_output);
-    if (!s.ok()) {
-      // Best approximation
-      return Decision::kIOError;
-    }
-    new_blob_value = compressed_output.AsSlice();
-  }
   uint64_t new_blob_file_number = 0;
   uint64_t new_blob_offset = 0;
   if (!WriteBlobToNewFile(key, new_blob_value, &new_blob_file_number,
@@ -142,8 +130,7 @@ CompactionFilter::Decision BlobIndexCompactionFilterBase::HandleValueChange(
     return Decision::kIOError;
   }
   BlobIndex::EncodeBlob(new_value, new_blob_file_number, new_blob_offset,
-                        new_blob_value.size(),
-                        blob_db_impl->bdb_options_.compression);
+                        new_blob_value.size(), kNoCompression);
   return Decision::kChangeBlobIndex;
 }
 
@@ -205,14 +192,13 @@ bool BlobIndexCompactionFilterBase::OpenNewBlobFileIfNeeded() const {
 }
 
 bool BlobIndexCompactionFilterBase::ReadBlobFromOldFile(
-    const Slice& key, const BlobIndex& blob_index, PinnableSlice* blob,
-    bool need_decompress, CompressionType* compression_type) const {
+    const Slice& key, const BlobIndex& blob_index, PinnableSlice* blob) const {
   BlobDBImpl* const blob_db_impl = context_.blob_db_impl;
   assert(blob_db_impl);
 
-  Status s = blob_db_impl->GetRawBlobFromFile(
-      key, blob_index.file_number(), blob_index.offset(), blob_index.size(),
-      blob, compression_type);
+  Status s = blob_db_impl->GetRawBlobFromFile(key, blob_index.file_number(),
+                                              blob_index.offset(),
+                                              blob_index.size(), blob);
 
   if (!s.ok()) {
     ROCKS_LOG_ERROR(
@@ -225,21 +211,6 @@ bool BlobIndexCompactionFilterBase::ReadBlobFromOldFile(
     return false;
   }
 
-  if (need_decompress && *compression_type != kNoCompression) {
-    s = blob_db_impl->DecompressSlice(*blob, *compression_type, blob);
-    if (!s.ok()) {
-      ROCKS_LOG_ERROR(
-          blob_db_impl->db_options_.info_log,
-          "Uncompression error during blob read from file: %" PRIu64
-          " blob_offset: %" PRIu64 " blob_size: %" PRIu64
-          " key: %s status: '%s'",
-          blob_index.file_number(), blob_index.offset(), blob_index.size(),
-          key.ToString(/* output_hex */ true).c_str(), s.ToString().c_str());
-
-      return false;
-    }
-  }
-
   return true;
 }
 
@@ -372,33 +343,11 @@ CompactionFilter::BlobDecision BlobIndexCompactionFilterGC::PrepareBlobOutput(
   }
 
   PinnableSlice blob;
-  CompressionType compression_type = kNoCompression;
-  GrowableBuffer compressed_output;
-  if (!ReadBlobFromOldFile(key, blob_index, &blob, false, &compression_type)) {
+  if (!ReadBlobFromOldFile(key, blob_index, &blob)) {
     gc_stats_.SetError();
     return BlobDecision::kIOError;
   }
 
-  // If the compression_type is changed, re-compress it with the new compression
-  // type.
-  if (compression_type != blob_db_impl->bdb_options_.compression) {
-    if (compression_type != kNoCompression) {
-      const Status status =
-          blob_db_impl->DecompressSlice(blob, compression_type, &blob);
-      if (!status.ok()) {
-        gc_stats_.SetError();
-        return BlobDecision::kCorruption;
-      }
-    }
-    if (blob_db_impl->bdb_options_.compression != kNoCompression) {
-      s = blob_db_impl->CompressBlob(blob, &compressed_output);
-      if (!s.ok()) {
-        return BlobDecision::kCorruption;
-      }
-      blob.PinSelf(compressed_output.AsSlice());
-    }
-  }
-
   uint64_t new_blob_file_number = 0;
   uint64_t new_blob_offset = 0;
   if (!WriteBlobToNewFile(key, blob, &new_blob_file_number, &new_blob_offset)) {
@@ -412,7 +361,7 @@ CompactionFilter::BlobDecision BlobIndexCompactionFilterGC::PrepareBlobOutput(
   }
 
   BlobIndex::EncodeBlob(new_value, new_blob_file_number, new_blob_offset,
-                        blob.size(), compression_type);
+                        blob.size(), kNoCompression);
 
   gc_stats_.AddRelocatedBlob(blob_index.size());
 

utilities/blob_db/blob_compaction_filter.h

@@ -59,8 +59,7 @@ class BlobIndexCompactionFilterBase : public LayeredCompactionFilterBase {
   bool IsBlobFileOpened() const;
   virtual bool OpenNewBlobFileIfNeeded() const;
   bool ReadBlobFromOldFile(const Slice& key, const BlobIndex& blob_index,
-                           PinnableSlice* blob, bool need_decompress,
-                           CompressionType* compression_type) const;
+                           PinnableSlice* blob) const;
   bool WriteBlobToNewFile(const Slice& key, const Slice& blob,
                           uint64_t* new_blob_file_number,
                           uint64_t* new_blob_offset) const;

utilities/blob_db/blob_db.cc

@@ -92,9 +92,6 @@ void BlobDBOptions::Dump(Logger* log) const {
   ROCKS_LOG_HEADER(
       log, "                            BlobDBOptions.blob_file_size: %" PRIu64,
       blob_file_size);
-  ROCKS_LOG_HEADER(
-      log, "                               BlobDBOptions.compression: %d",
-      static_cast<int>(compression));
   ROCKS_LOG_HEADER(
       log, "                 BlobDBOptions.enable_garbage_collection: %d",
       enable_garbage_collection);

utilities/blob_db/blob_db.h

@@ -66,9 +66,6 @@ struct BlobDBOptions {
   // after it exceeds that size
   uint64_t blob_file_size = 256 * 1024 * 1024;
 
-  // what compression to use for Blob's
-  CompressionType compression = kNoCompression;
-
   // If enabled, BlobDB cleans up stale blobs in non-TTL files during compaction
   // by rewriting the remaining live blobs to new files.
   bool enable_garbage_collection = false;