From e1f22931a69218dea32db5d5f9a4e86586e2ebca Mon Sep 17 00:00:00 2001
From: Adam Kupczyk <akupczyk@redhat.com>
Date: Mon, 30 Mar 2020 15:31:59 +0200
Subject: [PATCH] os/bluestore: Fixed default sharding definition

Modified recommendation for column families.

Signed-off-by: Adam Kupczyk <akupczyk@redhat.com>
---
 src/common/options.cc           |  6 ++++--
 src/kv/RocksDBStore.cc          |  8 +++----
 src/test/objectstore/test_kv.cc | 37 +++++++++++++++++++++++++++++++++
 3 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/src/common/options.cc b/src/common/options.cc
index 380dd16402766..9f66f0663f8d7 100644
--- a/src/common/options.cc
+++ b/src/common/options.cc
@@ -4417,12 +4417,14 @@ std::vector<Option> get_global_options() {
     .set_description("Enable use of rocksdb column families for bluestore metadata"),
 
     Option("bluestore_rocksdb_cfs", Option::TYPE_STR, Option::LEVEL_DEV)
-    .set_default("m(5,0-8) O(7,0-13) L")
+    .set_default("m(5) O(7,0-13) L")
     .set_description("Definition of column families and their sharding")
     .set_long_description("Space separated list of elements: column_def [ '=' rocksdb_options ]. "
 			  "column_def := column_name [ '(' shard_count [ ',' hash_begin '-' [ hash_end ] ] ')' ]. "
 			  "Example: 'I=write_buffer_size=1048576 O(6) m(7,10-)'. "
-			  "Interval [hash_begin..hash_end) defines characters to use for hash calculation."),
+			  "Interval [hash_begin..hash_end) defines characters to use for hash calculation. "
+			  "Recommended hash ranges: O(0-13) P(0-8) m(0-16). "
+			  "Sharding of S,T,C,M,B prefixes is inadvised"),
 
     Option("bluestore_fsck_on_mount", Option::TYPE_BOOL, Option::LEVEL_DEV)
     .set_default(false)
diff --git a/src/kv/RocksDBStore.cc b/src/kv/RocksDBStore.cc
index 54134de245b6f..fa16bdc18ca84 100644
--- a/src/kv/RocksDBStore.cc
+++ b/src/kv/RocksDBStore.cc
@@ -524,9 +524,9 @@ bool RocksDBStore::is_column_family(const std::string& prefix) {
 
 rocksdb::ColumnFamilyHandle *RocksDBStore::get_cf_handle(const std::string& prefix, const std::string& key) {
   auto iter = cf_handles.find(prefix);
-  if (iter == cf_handles.end())
+  if (iter == cf_handles.end()) {
     return nullptr;
-  else {
+  } else {
     if (iter->second.handles.size() == 1) {
       return iter->second.handles[0];
     } else {
@@ -540,9 +540,9 @@ rocksdb::ColumnFamilyHandle *RocksDBStore::get_cf_handle(const std::string& pref
 
 rocksdb::ColumnFamilyHandle *RocksDBStore::get_cf_handle(const std::string& prefix, const char* key, size_t keylen) {
   auto iter = cf_handles.find(prefix);
-  if (iter == cf_handles.end())
+  if (iter == cf_handles.end()) {
     return nullptr;
-  else {
+  } else {
     if (iter->second.handles.size() == 1) {
       return iter->second.handles[0];
     } else {
diff --git a/src/test/objectstore/test_kv.cc b/src/test/objectstore/test_kv.cc
index 7cb73b5b46684..b011c0efce458 100644
--- a/src/test/objectstore/test_kv.cc
+++ b/src/test/objectstore/test_kv.cc
@@ -312,6 +312,43 @@ TEST_P(KVTest, RMRange) {
   fini();
 }
 
+TEST_P(KVTest, ShardingRMRange) {
+  if(string(GetParam()) != "rocksdb")
+    return;
+  std::string cfs("O(7)=");
+  ASSERT_EQ(0, db->create_and_open(cout, cfs));
+  {
+    KeyValueDB::Transaction t = db->get_transaction();
+    for (size_t i = 0; i < 1000; i++) {
+      bufferlist value;
+      char* a;
+      ASSERT_EQ(asprintf(&a, "key%3.3ld", i), 6);
+      value.append(a);
+      t->set("O", a, value);
+      free(a);
+    }
+    db->submit_transaction_sync(t);
+  }
+
+  {
+    KeyValueDB::Transaction t = db->get_transaction();
+    t->rm_range_keys("O", "key277", "key467");
+    db->submit_transaction_sync(t);
+  }
+
+  for (size_t i = 0; i < 1000; i++) {
+    char* key;
+    ASSERT_EQ(asprintf(&key, "key%3.3ld", i), 6);
+    bufferlist value;
+    int r = db->get("O", key, &value);
+    ASSERT_EQ(r, (i >= 277 && i < 467 ? -ENOENT : 0));
+    free(key);
+  }
+
+  fini();
+}
+
+
 TEST_P(KVTest, RocksDBColumnFamilyTest) {
   if(string(GetParam()) != "rocksdb")
     return;
-- 
2.39.5