Disable secondary test with sst truncation deletion; API clarification

[hx235]

Context/Summary:
Secondary DB relies on open file descriptor of the shared SST file in primary DB to continue being able to read the file even if that file is deleted in the primary DB. However, this won't work if the file is truncated instead of deleted, which triggers an "truncated block read" corruption in stress test on secondary db reads. Truncation can happen if RocksDB implementation of SSTFileManager and bytes_max_delete_chunk>0 are used. This PR is to disable such testing combination in stress test and clarify the related API.

Test:

• Manually repro-ed with below UT. I'm in favor of not including this UT in the codebase as it should be self-evident from the API comment now about the incompatiblity. Secondary DB is in a direction of being replaced by Follower so we should minimize edge-case tests for code with no functional change for a to-be-replaced functionality.

TEST_F(DBSecondaryTest, IncompatibleWithPrimarySSTTruncation) {
  Options options;
  options.env = env_;
  options.disable_auto_compactions = true;
  options.sst_file_manager.reset(NewSstFileManager(
      env_, nullptr /*fs*/, "" /*trash_dir*/, 2024000 /*rate_bytes_per_sec*/,
      true /*delete_existing_trash*/, nullptr /*status*/,
      0.25 /*max_trash_db_ratio*/, 1129 /*bytes_max_delete_chunk*/));
  Reopen(options);

  ASSERT_OK(Put("key1", "old_value"));
  ASSERT_OK(Put("key2", "old_value"));
  ASSERT_OK(Flush());
  ASSERT_OK(Put("key1", "new_value"));
  ASSERT_OK(Put("key3", "new_value"));
  ASSERT_OK(Flush());

  Options options1;
  options1.env = env_;
  options1.max_open_files = -1;
  Reopen(options);
  OpenSecondary(options1);
  ASSERT_OK(db_secondary_->TryCatchUpWithPrimary());

  ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->SetCallBack(
      "DeleteScheduler::DeleteTrashFile:Fsync", [&](void*) {
        std::string value;
        Status s = db_secondary_->Get(ReadOptions(), "key2", &value);
        assert(s.IsCorruption());
        assert(s.ToString().find("truncated block read") !=
            std::string::npos);
      });
  ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing();

  ASSERT_OK(db_->CompactRange(CompactRangeOptions(), nullptr, nullptr));

  ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->DisableProcessing();
  ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->ClearAllCallBacks();
}

• Monitor future stress test

[facebook-github-bot]

@hx235 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[jowlyzhang]

LGTM! Thanks for the quick fix.

[jowlyzhang]

nit: rate_bytes_per_sec is another setting that enables slow deletion:

rocksdb/file/delete_scheduler.cc

Line 64 in e697219

if (rate_bytes_per_sec_.load() <= 0 ||

These two both need to be > 0 in order for ftruncate to be used for file deletion.

[hx235]

Fixed

[facebook-github-bot]

@hx235 has updated the pull request. You must reimport the pull request before landing.

[facebook-github-bot]

@hx235 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@hx235 merged this pull request in f6b2cdd.

[archang19]

thanks for making this change! hopefully the secondary crash tests are more quiet