Skip to content

Commit

Permalink
[yugabyte#10340] DocDb: Made compression_type gflag check to be case …
Browse files Browse the repository at this point in the history
…insensitive

Summary:
Previously, compression_type gflag check was case sensitive, as a result if the user provided a string that didn't match the expected input, it would crash the TServer. This change fixes the issue by making the gflag check to be case insensitive and not crash the TServer. Providing the compression type as `zlib` instead of `Zlib` should work now as expected.

Added new tests to validate the case insensitive checks.

Test Plan:
Jenkins
```
ctest -R docdb_rocksdb_util
```

Reviewers: rthallam, amitanand

Reviewed By: rthallam, amitanand

Subscribers: ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D17278
  • Loading branch information
basavaraj29 committed May 31, 2022
1 parent 9737277 commit 4111455
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 1 deletion.
20 changes: 20 additions & 0 deletions src/yb/docdb/docdb_rocksdb_util-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,26 @@ namespace docdb {

class DocDBRocksDBUtilTest : public YBTest {};

TEST_F(DocDBRocksDBUtilTest, CaseInsensitiveCompressionType) {
rocksdb::CompressionType got_compression_type =
CHECK_RESULT(TEST_GetConfiguredCompressionType("snappy"));

ASSERT_EQ(got_compression_type, rocksdb::kSnappyCompression);

got_compression_type = CHECK_RESULT(TEST_GetConfiguredCompressionType("SNappy"));
ASSERT_EQ(got_compression_type, rocksdb::kSnappyCompression);
got_compression_type = CHECK_RESULT(TEST_GetConfiguredCompressionType("snaPPy"));
ASSERT_EQ(got_compression_type, rocksdb::kSnappyCompression);

ASSERT_NOK(TEST_GetConfiguredCompressionType("snappy-"));

got_compression_type = CHECK_RESULT(TEST_GetConfiguredCompressionType("Lz4"));
ASSERT_EQ(got_compression_type, rocksdb::kLZ4Compression);

got_compression_type = CHECK_RESULT(TEST_GetConfiguredCompressionType("zLiB"));
ASSERT_EQ(got_compression_type, rocksdb::kZlibCompression);
}

TEST_F(DocDBRocksDBUtilTest, MaxBackgroundFlushesDefault) {
FLAGS_num_cpus = 16;
auto options = TEST_AutoInitFromRocksDBFlags();
Expand Down
8 changes: 7 additions & 1 deletion src/yb/docdb/docdb_rocksdb_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
#include <memory>
#include <thread>

#include <boost/algorithm/string/predicate.hpp>

#include "yb/common/transaction.h"

#include "yb/docdb/bounded_rocksdb_iterator.h"
Expand Down Expand Up @@ -161,7 +163,7 @@ Result<rocksdb::CompressionType> GetConfiguredCompressionType(const std::string&
rocksdb::kLZ4Compression
};
for (const auto& compression_type : kValidRocksDBCompressionTypes) {
if (flag_value == rocksdb::CompressionTypeToString(compression_type)) {
if (boost::iequals(flag_value, rocksdb::CompressionTypeToString(compression_type))) {
if (rocksdb::CompressionTypeSupported(compression_type)) {
return compression_type;
}
Expand Down Expand Up @@ -529,6 +531,10 @@ rocksdb::BlockBasedTableOptions TEST_AutoInitFromRocksDbTableFlags() {
return blockBasedTableOptions;
}

Result<rocksdb::CompressionType> TEST_GetConfiguredCompressionType(const std::string& flag_value) {
return yb::GetConfiguredCompressionType(flag_value);
}

int32_t GetGlobalRocksDBPriorityThreadPoolSize() {
if (FLAGS_rocksdb_disable_compactions) {
return 1;
Expand Down
2 changes: 2 additions & 0 deletions src/yb/docdb/docdb_rocksdb_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ rocksdb::Options TEST_AutoInitFromRocksDBFlags();

rocksdb::BlockBasedTableOptions TEST_AutoInitFromRocksDbTableFlags();

Result<rocksdb::CompressionType> TEST_GetConfiguredCompressionType(const std::string& flag_value);

Result<rocksdb::KeyValueEncodingFormat> GetConfiguredKeyValueEncodingFormat(
const std::string& flag_value);

Expand Down

0 comments on commit 4111455

Please sign in to comment.