Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate db delete file public API #13284

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions db/c.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1858,8 +1858,8 @@ extern ROCKSDB_LIBRARY_API void rocksdb_approximate_sizes_cf_with_flags(
delete[] ranges;
}

void rocksdb_delete_file(rocksdb_t* db, const char* name) {
db->rep->DeleteFile(name);
void DEPRECATED_rocksdb_delete_file(rocksdb_t* db, const char* name) {
db->rep->DEPRECATED_DeleteFile(name);
}

const rocksdb_livefiles_t* rocksdb_livefiles(rocksdb_t* db) {
Expand Down
2 changes: 1 addition & 1 deletion db/db_impl/db_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4881,7 +4881,7 @@ Status DBImpl::GetUpdatesSince(
return wal_manager_.GetUpdatesSince(seq, iter, read_options, versions_.get());
}

Status DBImpl::DeleteFile(std::string name) {
Status DBImpl::DEPRECATED_DeleteFile(std::string name) {
// TODO: plumb Env::IOActivity, Env::IOPriority
const ReadOptions read_options;
const WriteOptions write_options;
Expand Down
2 changes: 1 addition & 1 deletion db/db_impl/db_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ class DBImpl : public DB {
SequenceNumber seq_number, std::unique_ptr<TransactionLogIterator>* iter,
const TransactionLogIterator::ReadOptions& read_options =
TransactionLogIterator::ReadOptions()) override;
Status DeleteFile(std::string name) override;
Status DEPRECATED_DeleteFile(std::string name) override;
Status DeleteFilesInRanges(ColumnFamilyHandle* column_family,
const RangePtr* ranges, size_t n,
bool include_end = true);
Expand Down
6 changes: 4 additions & 2 deletions db/db_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3409,7 +3409,9 @@ class ModelDB : public DB {
return Status::NotSupported();
}

Status DeleteFile(std::string /*name*/) override { return Status::OK(); }
Status DEPRECATED_DeleteFile(std::string /*name*/) override {
return Status::OK();
}

Status GetUpdatesSince(
ROCKSDB_NAMESPACE::SequenceNumber,
Expand Down Expand Up @@ -5238,7 +5240,7 @@ TEST_F(DBTest, DynamicLevelCompressionPerLevel) {
db_->GetColumnFamilyMetaData(&cf_meta);
for (const auto& file : cf_meta.levels[4].files) {
listener->SetExpectedFileName(dbname_ + file.name);
ASSERT_OK(dbfull()->DeleteFile(file.name));
ASSERT_OK(dbfull()->DEPRECATED_DeleteFile(file.name));
}
listener->VerifyMatchedCount(cf_meta.levels[4].files.size());

Expand Down
16 changes: 8 additions & 8 deletions db/deletefile_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -174,15 +174,15 @@ TEST_F(DeleteFileTest, AddKeysAndQueryLevels) {
ASSERT_EQ(level1keycount, 50000);
ASSERT_EQ(level2keycount, 50000);

Status status = db_->DeleteFile("0.sst");
Status status = db_->DEPRECATED_DeleteFile("0.sst");
ASSERT_TRUE(status.IsInvalidArgument());

// intermediate level files cannot be deleted.
status = db_->DeleteFile(level1file);
status = db_->DEPRECATED_DeleteFile(level1file);
ASSERT_TRUE(status.IsInvalidArgument());

// Lowest level file deletion should succeed.
status = db_->DeleteFile(level2file);
status = db_->DEPRECATED_DeleteFile(level2file);
ASSERT_OK(status);
}

Expand Down Expand Up @@ -519,7 +519,7 @@ TEST_F(DeleteFileTest, DeleteFileWithIterator) {
level2file = metadata[0].name;
}

Status status = db_->DeleteFile(level2file);
Status status = db_->DEPRECATED_DeleteFile(level2file);
fprintf(stdout, "Deletion status %s: %s\n", level2file.c_str(),
status.ToString().c_str());
ASSERT_OK(status);
Expand Down Expand Up @@ -551,7 +551,7 @@ TEST_F(DeleteFileTest, DeleteLogFiles) {
ASSERT_OK(env_->FileExists(wal_dir_ + "/" + alive_log->PathName()));
fprintf(stdout, "Deleting alive log file %s\n",
alive_log->PathName().c_str());
ASSERT_NOK(db_->DeleteFile(alive_log->PathName()));
ASSERT_NOK(db_->DEPRECATED_DeleteFile(alive_log->PathName()));
ASSERT_OK(env_->FileExists(wal_dir_ + "/" + alive_log->PathName()));
logfiles.clear();

Expand All @@ -569,7 +569,7 @@ TEST_F(DeleteFileTest, DeleteLogFiles) {
ASSERT_OK(env_->FileExists(wal_dir_ + "/" + archived_log->PathName()));
fprintf(stdout, "Deleting archived log file %s\n",
archived_log->PathName().c_str());
ASSERT_OK(db_->DeleteFile(archived_log->PathName()));
ASSERT_OK(db_->DEPRECATED_DeleteFile(archived_log->PathName()));
ASSERT_TRUE(
env_->FileExists(wal_dir_ + "/" + archived_log->PathName()).IsNotFound());
}
Expand Down Expand Up @@ -605,8 +605,8 @@ TEST_F(DeleteFileTest, DeleteNonDefaultColumnFamily) {
auto new_file = metadata[0].smallest_seqno > metadata[1].smallest_seqno
? metadata[0].name
: metadata[1].name;
ASSERT_TRUE(db_->DeleteFile(new_file).IsInvalidArgument());
ASSERT_OK(db_->DeleteFile(old_file));
ASSERT_TRUE(db_->DEPRECATED_DeleteFile(new_file).IsInvalidArgument());
ASSERT_OK(db_->DEPRECATED_DeleteFile(old_file));

{
std::unique_ptr<Iterator> itr(db_->NewIterator(ReadOptions(), handles_[1]));
Expand Down
4 changes: 2 additions & 2 deletions include/rocksdb/c.h
Original file line number Diff line number Diff line change
Expand Up @@ -692,8 +692,8 @@ extern ROCKSDB_LIBRARY_API void rocksdb_compact_range_cf_opt(
rocksdb_compactoptions_t* opt, const char* start_key, size_t start_key_len,
const char* limit_key, size_t limit_key_len);

extern ROCKSDB_LIBRARY_API void rocksdb_delete_file(rocksdb_t* db,
const char* name);
extern ROCKSDB_LIBRARY_API void DEPRECATED_rocksdb_delete_file(
rocksdb_t* db, const char* name);

extern ROCKSDB_LIBRARY_API const rocksdb_livefiles_t* rocksdb_livefiles(
rocksdb_t* db);
Expand Down
9 changes: 1 addition & 8 deletions include/rocksdb/db.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,6 @@
#include "rocksdb/version.h"
#include "rocksdb/wide_columns.h"

#ifdef _WIN32
// Windows API macro interference
#undef DeleteFile
#endif

#if defined(__GNUC__) || defined(__clang__)
#define ROCKSDB_DEPRECATED_FUNC __attribute__((__deprecated__))
#elif _WIN32
Expand Down Expand Up @@ -1768,8 +1763,6 @@ class DB {
const TransactionLogIterator::ReadOptions& read_options =
TransactionLogIterator::ReadOptions()) = 0;

// Windows API macro interference
#undef DeleteFile
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hooray! Though there's another above in db.h that should be removed.

(Can't remove the #undef DeleteFile in other files because of other symbols. ☹️)

// WARNING: This API is planned for removal in RocksDB 7.0 since it does not
// operate at the proper level of abstraction for a key-value store, and its
// contract/restrictions are poorly documented. For example, it returns non-OK
Expand All @@ -1781,7 +1774,7 @@ class DB {
// Delete the file name from the db directory and update the internal state to
// reflect that. Supports deletion of sst and log files only. 'name' must be
// path relative to the db directory. eg. 000001.sst, /archive/000003.log
virtual Status DeleteFile(std::string name) = 0;
virtual Status DEPRECATED_DeleteFile(std::string name) = 0;

// Obtains a list of all live table (SST) files and how they fit into the
// LSM-trees, such as column family, level, key range, etc.
Expand Down
4 changes: 3 additions & 1 deletion include/rocksdb/utilities/stackable_db.h
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,9 @@ class StackableDB : public DB {
// do not plan to maintain it, the contract will likely remain underspecified
// until its removal. Any user is encouraged to read the implementation
// carefully and migrate away from it when possible.
Status DeleteFile(std::string name) override { return db_->DeleteFile(name); }
Status DEPRECATED_DeleteFile(std::string name) override {
return db_->DEPRECATED_DeleteFile(name);
}

Status GetDbIdentity(std::string& identity) const override {
return db_->GetDbIdentity(identity);
Expand Down
9 changes: 5 additions & 4 deletions java/rocksjni/rocksjni.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3242,11 +3242,12 @@ jlong Java_org_rocksdb_RocksDB_getUpdatesSince(JNIEnv* env, jclass,

/*
* Class: org_rocksdb_RocksDB
* Method: deleteFile
* Method: deprecated_deleteFile
* Signature: (JLjava/lang/String;)V
*/
void Java_org_rocksdb_RocksDB_deleteFile(JNIEnv* env, jclass, jlong jdb_handle,
jstring jname) {
void Java_org_rocksdb_RocksDB_deprecated_1deleteFile(JNIEnv* env, jclass,
jlong jdb_handle,
jstring jname) {
auto* db = reinterpret_cast<ROCKSDB_NAMESPACE::DB*>(jdb_handle);
jboolean has_exception = JNI_FALSE;
std::string name =
Expand All @@ -3255,7 +3256,7 @@ void Java_org_rocksdb_RocksDB_deleteFile(JNIEnv* env, jclass, jlong jdb_handle,
// exception occurred
return;
}
db->DeleteFile(name);
db->DEPRECATED_DeleteFile(name);
}

/*
Expand Down
8 changes: 5 additions & 3 deletions java/src/main/java/org/rocksdb/RocksDB.java
Original file line number Diff line number Diff line change
Expand Up @@ -4428,8 +4428,9 @@ public TransactionLogIterator getUpdatesSince(final long sequenceNumber)
*
* @throws RocksDBException if an error occurs whilst deleting the file
*/
public void deleteFile(final String name) throws RocksDBException {
deleteFile(nativeHandle_, name);
@Deprecated
public void deprecated_deleteFile(final String name) throws RocksDBException {
deprecated_deleteFile(nativeHandle_, name);
}

/**
Expand Down Expand Up @@ -5054,7 +5055,8 @@ private static native String[] getLiveFiles(final long handle, final boolean flu
private static native LogFile[] getSortedWalFiles(final long handle) throws RocksDBException;
private static native long getUpdatesSince(final long handle, final long sequenceNumber)
throws RocksDBException;
private static native void deleteFile(final long handle, final String name)
@Deprecated
private static native void deprecated_deleteFile(final long handle, final String name)
throws RocksDBException;
private static native LiveFileMetaData[] getLiveFilesMetaData(final long handle);
private static native ColumnFamilyMetaData getColumnFamilyMetaData(
Expand Down
2 changes: 1 addition & 1 deletion java/src/test/java/org/rocksdb/EventListenerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ void deleteTableFile(final AbstractEventListener el, final AtomicBoolean wasCbCa
assertThat(liveFiles).isNotNull();
assertThat(liveFiles.files).isNotNull();
assertThat(liveFiles.files.isEmpty()).isFalse();
db.deleteFile(liveFiles.files.get(0));
db.deprecated_deleteFile(liveFiles.files.get(0));
assertThat(wasCbCalled.get()).isTrue();
}
}
Expand Down
4 changes: 2 additions & 2 deletions java/src/test/java/org/rocksdb/RocksDBTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1666,11 +1666,11 @@ public void getSortedWalFiles() throws RocksDBException {
}

@Test
public void deleteFile() throws RocksDBException {
public void deprecated_deleteFile() throws RocksDBException {
try (final Options options = new Options().setCreateIfMissing(true)) {
final String dbPath = dbFolder.getRoot().getAbsolutePath();
try (final RocksDB db = RocksDB.open(options, dbPath)) {
db.deleteFile("unknown");
db.deprecated_deleteFile("unknown");
}
}
}
Expand Down
Loading