Skip to content

Commit

Permalink
Add support for Delete/SingleDelete with secondary indices (#13291)
Browse files Browse the repository at this point in the history 
Summary:
The patch implements support for `Delete` and `SingleDelete` with secondary indices, leveraging the earlier pieces built for `Put` / `PutEntity`. As expected, deleting an entry using these APIs also deletes any associated secondary index entries.

Pull Request resolved: #13291

Reviewed By: jaykorean

Differential Revision: D68041422

fbshipit-source-id: c8afc9ff69dea834f89ae855a72c1d76e7db0e35
  • Loading branch information
@ltamasi @facebook-github-bot
ltamasi authored and facebook-github-bot committed Jan 15, 2025
1 parent 8bccd39 commit 5938ad2
Show file tree
Hide file tree
Showing 2 changed files with 154 additions and 49 deletions.
169 changes: 121 additions & 48 deletions utilities/secondary_index/secondary_index_mixin.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,30 +73,35 @@ class SecondaryIndexMixin : public Txn {
}

using Txn::Delete;
Status Delete(ColumnFamilyHandle* /* column_family */, const Slice& /* key */,
const bool /* assume_tracked */ = false) override {
return Status::NotSupported(
"Delete with secondary indices not yet supported");
Status Delete(ColumnFamilyHandle* column_family, const Slice& key,
const bool assume_tracked = false) override {
return PerformWithSavePoint([&]() {
const bool do_validate = !assume_tracked;
return DeleteWithSecondaryIndices(column_family, key, do_validate);
});
}
Status Delete(ColumnFamilyHandle* /* column_family */,
const SliceParts& /* key */,
const bool /* assume_tracked */ = false) override {
return Status::NotSupported(
"Delete with secondary indices not yet supported");
Status Delete(ColumnFamilyHandle* column_family, const SliceParts& key,
const bool assume_tracked = false) override {
std::string key_str;
const Slice key_slice(key, &key_str);

return Delete(column_family, key_slice, assume_tracked);
}

using Txn::SingleDelete;
Status SingleDelete(ColumnFamilyHandle* /* column_family */,
const Slice& /* key */,
const bool /* assume_tracked */ = false) override {
return Status::NotSupported(
"SingleDelete with secondary indices not yet supported");
Status SingleDelete(ColumnFamilyHandle* column_family, const Slice& key,
const bool assume_tracked = false) override {
return PerformWithSavePoint([&]() {
const bool do_validate = !assume_tracked;
return SingleDeleteWithSecondaryIndices(column_family, key, do_validate);
});
}
Status SingleDelete(ColumnFamilyHandle* /* column_family */,
const SliceParts& /* key */,
const bool /* assume_tracked */ = false) override {
return Status::NotSupported(
"SingleDelete with secondary indices not yet supported");
Status SingleDelete(ColumnFamilyHandle* column_family, const SliceParts& key,
const bool assume_tracked = false) override {
std::string key_str;
const Slice key_slice(key, &key_str);

return SingleDelete(column_family, key_slice, assume_tracked);
}

using Txn::PutUntracked;
Expand Down Expand Up @@ -136,22 +141,28 @@ class SecondaryIndexMixin : public Txn {
}

using Txn::DeleteUntracked;
Status DeleteUntracked(ColumnFamilyHandle* /* column_family */,
const Slice& /* key */) override {
return Status::NotSupported(
"DeleteUntracked with secondary indices not yet supported");
Status DeleteUntracked(ColumnFamilyHandle* column_family,
const Slice& key) override {
return PerformWithSavePoint([&]() {
constexpr bool do_validate = false;
return DeleteWithSecondaryIndices(column_family, key, do_validate);
});
}
Status DeleteUntracked(ColumnFamilyHandle* /* column_family */,
const SliceParts& /* key */) override {
return Status::NotSupported(
"DeleteUntracked with secondary indices not yet supported");
Status DeleteUntracked(ColumnFamilyHandle* column_family,
const SliceParts& key) override {
std::string key_str;
const Slice key_slice(key, &key_str);

return DeleteUntracked(column_family, key_slice);
}

using Txn::SingleDeleteUntracked;
Status SingleDeleteUntracked(ColumnFamilyHandle* /* column_family */,
const Slice& /* key */) override {
return Status::NotSupported(
"SingleDeleteUntracked with secondary indices not yet supported");
Status SingleDeleteUntracked(ColumnFamilyHandle* column_family,
const Slice& key) override {
return PerformWithSavePoint([&]() {
constexpr bool do_validate = false;
return SingleDeleteWithSecondaryIndices(column_family, key, do_validate);
});
}

private:
Expand Down Expand Up @@ -307,22 +318,10 @@ class SecondaryIndexMixin : public Txn {
}

Status RemoveSecondaryEntries(ColumnFamilyHandle* column_family,
const Slice& primary_key, bool do_validate) {
const Slice& primary_key,
const WideColumns& existing_columns) {
assert(column_family);

PinnableWideColumns existing_primary_columns;

const Status s = GetPrimaryEntryForUpdate(
column_family, primary_key, &existing_primary_columns, do_validate);
if (s.IsNotFound()) {
return Status::OK();
}
if (!s.ok()) {
return s;
}

const auto& existing_columns = existing_primary_columns.columns();

for (const auto& secondary_index : *secondary_indices_) {
assert(secondary_index);

Expand Down Expand Up @@ -459,10 +458,20 @@ class SecondaryIndexMixin : public Txn {
const Slice& primary_key = key;

{
const Status s =
RemoveSecondaryEntries(column_family, primary_key, do_validate);
PinnableWideColumns existing_primary_columns;

const Status s = GetPrimaryEntryForUpdate(
column_family, primary_key, &existing_primary_columns, do_validate);
if (!s.ok()) {
return s;
if (!s.IsNotFound()) {
return s;
}
} else {
const Status st = RemoveSecondaryEntries(
column_family, primary_key, existing_primary_columns.columns());
if (!st.ok()) {
return st;
}
}
}

Expand Down Expand Up @@ -510,6 +519,70 @@ class SecondaryIndexMixin : public Txn {
do_validate);
}

template <typename Operation>
Status DeleteWithSecondaryIndicesImpl(ColumnFamilyHandle* column_family,
const Slice& key, bool do_validate,
Operation&& operation) {
if (!column_family) {
column_family = Txn::DefaultColumnFamily();
}

{
PinnableWideColumns existing_primary_columns;

const Status s = GetPrimaryEntryForUpdate(
column_family, key, &existing_primary_columns, do_validate);
if (!s.ok()) {
if (!s.IsNotFound()) {
return s;
}

return Status::OK();
} else {
const Status st = RemoveSecondaryEntries(
column_family, key, existing_primary_columns.columns());
if (!st.ok()) {
return st;
}
}
}

{
const Status s = operation(column_family, key);
if (!s.ok()) {
return s;
}
}

return Status::OK();
}

Status DeleteWithSecondaryIndices(ColumnFamilyHandle* column_family,
const Slice& key, bool do_validate) {
return DeleteWithSecondaryIndicesImpl(
column_family, key, do_validate,
[&](ColumnFamilyHandle* cfh, const Slice& primary_key) {
assert(cfh);

constexpr bool assume_tracked = true;

return Txn::Delete(cfh, primary_key, assume_tracked);
});
}

Status SingleDeleteWithSecondaryIndices(ColumnFamilyHandle* column_family,
const Slice& key, bool do_validate) {
return DeleteWithSecondaryIndicesImpl(
column_family, key, do_validate,
[&](ColumnFamilyHandle* cfh, const Slice& primary_key) {
assert(cfh);

constexpr bool assume_tracked = true;

return Txn::SingleDelete(cfh, primary_key, assume_tracked);
});
}

const std::vector<std::shared_ptr<SecondaryIndex>>* secondary_indices_;
};

Expand Down
34 changes: 33 additions & 1 deletion utilities/transactions/transaction_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8008,7 +8008,7 @@ TEST_P(TransactionTest, AttributeGroupIteratorSanityChecks) {
}
}

TEST_P(TransactionTest, SecondaryIndexPut) {
TEST_P(TransactionTest, SecondaryIndexPutDelete) {
const TxnDBWritePolicy write_policy = std::get<2>(GetParam());
if (write_policy != TxnDBWritePolicy::WRITE_COMMITTED) {
ROCKSDB_GTEST_BYPASS("Test only WriteCommitted for now");
Expand Down Expand Up @@ -8323,6 +8323,38 @@ TEST_P(TransactionTest, SecondaryIndexPut) {
ASSERT_FALSE(it->Valid());
ASSERT_OK(it->status());
}

// Delete/SingleDelete "key1" and "key3" via an explicit transaction and
// "key2" and a non-existing "key4" via the DB interface (i.e. an implicit
// transaction)
{
std::unique_ptr<Transaction> txn(db->BeginTransaction(WriteOptions()));

ASSERT_OK(txn->Delete(cfh1, "key1"));
ASSERT_OK(txn->SingleDelete(cfh1, "key3"));

ASSERT_OK(txn->Commit());
}

ASSERT_OK(db->SingleDelete(WriteOptions(), cfh1, "key2"));
ASSERT_OK(db->Delete(WriteOptions(), cfh1, "key4"));

// cfh1 and cfh2 are expected to be empty
{
std::unique_ptr<Iterator> it(db->NewIterator(ReadOptions(), cfh1));

it->SeekToFirst();
ASSERT_FALSE(it->Valid());
ASSERT_OK(it->status());
}

{
std::unique_ptr<Iterator> it(db->NewIterator(ReadOptions(), cfh2));

it->SeekToFirst();
ASSERT_FALSE(it->Valid());
ASSERT_OK(it->status());
}
}

TEST_P(TransactionTest, SecondaryIndexPutEntity) {
Expand Down

0 comments on commit 5938ad2

Please sign in to comment.