diff --git a/HISTORY.md b/HISTORY.md index bca3080c9..61e5566c3 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -4,6 +4,7 @@ * Iterator::SeekForPrev is now a pure virtual method. This is to prevent user who implement the Iterator interface fail to implement SeekForPrev by mistake. * Add `include_end` option to make the range end exclusive when `include_end == false` in `DeleteFilesInRange()`. * Add `CompactRangeOptions::allow_write_stall`, which makes `CompactRange` start working immediately, even if it causes user writes to stall. The default value is false, meaning we add delay to `CompactRange` calls until stalling can be avoided when possible. Note this delay is not present in previous RocksDB versions. +* Creating checkpoint with empty directory now returns `Status::InvalidArgument`; previously, it returned `Status::IOError`. ### New Features * Improve the performance of iterators doing long range scans by using readahead. diff --git a/tools/ldb_cmd.cc b/tools/ldb_cmd.cc index 8da148b44..96138ffa6 100644 --- a/tools/ldb_cmd.cc +++ b/tools/ldb_cmd.cc @@ -2616,10 +2616,7 @@ CheckPointCommand::CheckPointCommand( : LDBCommand(options, flags, false /* is_read_only */, BuildCmdLineOptions({ARG_CHECKPOINT_DIR})) { auto itr = options.find(ARG_CHECKPOINT_DIR); - if (itr == options.end()) { - exec_state_ = LDBCommandExecuteResult::Failed( - "--" + ARG_CHECKPOINT_DIR + ": missing checkpoint directory"); - } else { + if (itr != options.end()) { checkpoint_dir_ = itr->second; } } diff --git a/utilities/checkpoint/checkpoint_impl.cc b/utilities/checkpoint/checkpoint_impl.cc index b116afe34..d93c7095f 100644 --- a/utilities/checkpoint/checkpoint_impl.cc +++ b/utilities/checkpoint/checkpoint_impl.cc @@ -62,9 +62,10 @@ Status CheckpointImpl::CreateCheckpoint(const std::string& checkpoint_dir, size_t final_nonslash_idx = checkpoint_dir.find_last_not_of('/'); if (final_nonslash_idx == std::string::npos) { - // npos means it's only slashes, which means it's the root directory, but it - // shouldn't be because we verified above the directory doesn't exist. - assert(false); + // npos means it's only slashes or empty. Non-empty means it's the root + // directory, but it shouldn't be because we verified above the directory + // doesn't exist. + assert(checkpoint_dir.empty()); return Status::InvalidArgument("invalid checkpoint directory name"); } diff --git a/utilities/checkpoint/checkpoint_test.cc b/utilities/checkpoint/checkpoint_test.cc index ad49fd55c..794097f2d 100644 --- a/utilities/checkpoint/checkpoint_test.cc +++ b/utilities/checkpoint/checkpoint_test.cc @@ -564,6 +564,15 @@ TEST_F(CheckpointTest, CurrentFileModifiedWhileCheckpointing2PC) { delete txdb; } +TEST_F(CheckpointTest, CheckpointInvalidDirectoryName) { + for (std::string checkpoint_dir : {"", "/", "////"}) { + Checkpoint* checkpoint; + ASSERT_OK(Checkpoint::Create(db_, &checkpoint)); + ASSERT_TRUE(checkpoint->CreateCheckpoint("").IsInvalidArgument()); + delete checkpoint; + } +} + } // namespace rocksdb int main(int argc, char** argv) {