Un-revert #7049, revert #7022 (#7071)

Summary:
Even though local bisection gave me a clear signal (and still does) that reverting https://github.com/facebook/rocksdb/issues/7049 would fix the failures in MultiThreadedDBTest, https://github.com/facebook/rocksdb/issues/7022 seems to be the root cause. Reverting https://github.com/facebook/rocksdb/issues/7022 and keeping https://github.com/facebook/rocksdb/issues/7049 seems to fix the issue in local reproducer also. (Had these landed in opposite order, bisection would have found the root cause.)
Pull Request resolved: https://github.com/facebook/rocksdb/pull/7071

Reviewed By: akankshamahajan15

Differential Revision: D22362857

Pulled By: pdillinger

fbshipit-source-id: ed63df3d74e9d4ce1604de8fe43b216166c7a3f0

HISTORY.md

@@ -22,7 +22,6 @@
 
 ### Bug Fixes
 * Fail recovery and report once hitting a physical log record checksum mismatch, while reading MANIFEST. RocksDB should not continue processing the MANIFEST any further.
-* Fix a bug when index_type == kTwoLevelIndexSearch in PartitionedIndexBuilder to update FlushPolicy to point to internal key partitioner when it changes from user-key mode to internal-key mode in index partition.
 
 ## 6.11 (6/12/2020)
 ### Bug Fixes

db/db_test2.cc

@@ -144,38 +144,6 @@ INSTANTIATE_TEST_CASE_P(TestReadOnlyWithCompressedCache,
                         TestReadOnlyWithCompressedCache,
                         ::testing::Combine(::testing::Values(-1, 100),
                                            ::testing::Bool()));
-
-class PartitionedIndexTestListener : public EventListener {
- public:
-  void OnFlushCompleted(DB* /*db*/, const FlushJobInfo& info) override {
-    ASSERT_GT(info.table_properties.index_partitions, 1);
-    ASSERT_EQ(info.table_properties.index_key_is_user_key, 0);
-  }
-};
-
-TEST_F(DBTest2, PartitionedIndexUserToInternalKey) {
-  BlockBasedTableOptions table_options;
-  Options options = CurrentOptions();
-  table_options.index_type = BlockBasedTableOptions::kTwoLevelIndexSearch;
-  PartitionedIndexTestListener* listener = new PartitionedIndexTestListener();
-  options.table_factory.reset(NewBlockBasedTableFactory(table_options));
-  options.listeners.emplace_back(listener);
-  std::vector<const Snapshot*> snapshots;
-  Reopen(options);
-  Random rnd(301);
-
-  for (int i = 0; i < 3000; i++) {
-    int j = i % 30;
-    std::string value = RandomString(&rnd, 10500);
-    ASSERT_OK(Put("keykey_" + std::to_string(j), value));
-    snapshots.push_back(db_->GetSnapshot());
-  }
-  Flush();
-  for (auto s : snapshots) {
-    db_->ReleaseSnapshot(s);
-  }
-}
-
 #endif  // ROCKSDB_LITE
 
 class PrefixFullBloomWithReverseComparator

table/block_based/index_builder.cc

@@ -112,7 +112,6 @@ void PartitionedIndexBuilder::MakeNewSubIndexBuilder() {
           ? sub_index_builder_->index_block_builder_
           : sub_index_builder_->index_block_builder_without_seq_));
   partition_cut_requested_ = false;
-  seperator_is_key_plus_seq_ = false;
 }
 
 void PartitionedIndexBuilder::RequestPartitionCut() {
@@ -130,15 +129,9 @@ void PartitionedIndexBuilder::AddIndexEntry(
     }
     sub_index_builder_->AddIndexEntry(last_key_in_current_block,
                                       first_key_in_next_block, block_handle);
-    if (!seperator_is_key_plus_seq_ &&
+    if (sub_index_builder_->seperator_is_key_plus_seq_) {
-        sub_index_builder_->seperator_is_key_plus_seq_) {
+      // then we need to apply it to all sub-index builders
-      // then we need to apply it to all sub-index builders and reset
-      // flush_policy to point to Block Builder of sub_index_builder_ that store
-      // internal keys.
       seperator_is_key_plus_seq_ = true;
-      flush_policy_.reset(FlushBlockBySizePolicyFactory::NewFlushBlockPolicy(
-          table_opt_.metadata_block_size, table_opt_.block_size_deviation,
-          sub_index_builder_->index_block_builder_));
     }
     sub_index_last_key_ = std::string(*last_key_in_current_block);
     entries_.push_back(
@@ -168,14 +161,8 @@ void PartitionedIndexBuilder::AddIndexEntry(
     sub_index_builder_->AddIndexEntry(last_key_in_current_block,
                                       first_key_in_next_block, block_handle);
     sub_index_last_key_ = std::string(*last_key_in_current_block);
-    if (!seperator_is_key_plus_seq_ &&
+    if (sub_index_builder_->seperator_is_key_plus_seq_) {
-        sub_index_builder_->seperator_is_key_plus_seq_) {
+      // then we need to apply it to all sub-index builders
-      // then we need to apply it to all sub-index builders and reset
-      // flush_policy to point to Block Builder of sub_index_builder_ that store
-      // internal keys.
-      flush_policy_.reset(FlushBlockBySizePolicyFactory::NewFlushBlockPolicy(
-          table_opt_.metadata_block_size, table_opt_.block_size_deviation,
-          sub_index_builder_->index_block_builder_));
       seperator_is_key_plus_seq_ = true;
     }
   }