Skip to content

Commit

Permalink
Refresh() is not supported by vector memtable which may lead to error…
Browse files Browse the repository at this point in the history
… in db_stress
  • Loading branch information
ayulas authored and Or Friedmann committed Jan 24, 2024
1 parent 3ef9092 commit e231df1
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 3 deletions.
5 changes: 4 additions & 1 deletion db/arena_wrapped_db_iter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,10 @@ ArenaWrappedDBIter* NewArenaWrappedDbIterator(
ArenaWrappedDBIter* iter = new ArenaWrappedDBIter();
iter->Init(env, read_options, ioptions, mutable_cf_options, version, sequence,
max_sequential_skip_in_iterations, version_number, read_callback,
db_impl, cfd, expose_blob_index, allow_refresh);
db_impl, cfd, expose_blob_index,
!ioptions.memtable_factory->IsRefreshIterSupported()
? false
: allow_refresh);
if (db_impl != nullptr && cfd != nullptr && allow_refresh) {
iter->StoreRefreshInfo(db_impl, cfd, read_callback, expose_blob_index);
}
Expand Down
15 changes: 15 additions & 0 deletions db/arena_wrapped_db_iter.h
Original file line number Diff line number Diff line change
@@ -1,3 +1,17 @@
// Copyright (C) 2023 Speedb Ltd. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

// Copyright (c) 2011-present, Facebook, Inc. All rights reserved.
// This source code is licensed under both the GPLv2 (found in the
// COPYING file in the root directory) and Apache 2.0 License
Expand Down Expand Up @@ -76,6 +90,7 @@ class ArenaWrappedDBIter : public Iterator {
Status status() const override { return db_iter_->status(); }
Slice timestamp() const override { return db_iter_->timestamp(); }
bool IsBlob() const { return db_iter_->IsBlob(); }
bool IsAllowRefresh() override { return allow_refresh_; }

Status GetProperty(std::string prop_name, std::string* prop) override;

Expand Down
37 changes: 37 additions & 0 deletions db/db_iterator_test.cc
Original file line number Diff line number Diff line change
@@ -1,3 +1,17 @@
// Copyright (C) 2023 Speedb Ltd. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

// Copyright (c) 2011-present, Facebook, Inc. All rights reserved.
// This source code is licensed under both the GPLv2 (found in the
// COPYING file in the root directory) and Apache 2.0 License
Expand Down Expand Up @@ -3550,6 +3564,29 @@ TEST_F(DBIteratorTest, ErrorWhenReadFile) {
iter->Reset();
}

TEST_F(DBIteratorTest, VectorRefreshStatus) {
Options options = CurrentOptions();
options.allow_concurrent_memtable_write = false;
options.memtable_factory.reset(new VectorRepFactory());
DestroyAndReopen(options);
Iterator* iter = db_->NewIterator(ReadOptions());
Status s = iter->Refresh();
ASSERT_TRUE(s.IsNotSupported());
ASSERT_FALSE(iter->IsAllowRefresh());
delete iter;
}

TEST_F(DBIteratorTest, SkipListRefreshStatus) {
Options options = CurrentOptions();
options.memtable_factory.reset(new SkipListFactory());
DestroyAndReopen(options);
Iterator* iter = db_->NewIterator(ReadOptions());
Status s = iter->Refresh();
ASSERT_OK(s);
ASSERT_TRUE(iter->IsAllowRefresh());
delete iter;
}

} // namespace ROCKSDB_NAMESPACE

int main(int argc, char** argv) {
Expand Down
2 changes: 1 addition & 1 deletion db_stress_tool/no_batched_ops_stress.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1870,7 +1870,7 @@ class NonBatchedOpsStressTest : public StressTest {

// Write-prepared and Write-unprepared do not support Refresh() yet.
if (!(FLAGS_use_txn && FLAGS_txn_write_policy != 0) &&
thread->rand.OneIn(2)) {
thread->rand.OneIn(2) && iter->IsAllowRefresh()) {
pre_read_expected_values.clear();
post_read_expected_values.clear();
// Refresh after forward/backward scan to allow higher chance of SV
Expand Down
17 changes: 16 additions & 1 deletion include/rocksdb/iterator.h
Original file line number Diff line number Diff line change
@@ -1,3 +1,17 @@
// Copyright (C) 2023 Speedb Ltd. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

// Copyright (c) 2011-present, Facebook, Inc. All rights reserved.
// This source code is licensed under both the GPLv2 (found in the
// COPYING file in the root directory) and Apache 2.0 License
Expand Down Expand Up @@ -122,7 +136,8 @@ class Iterator : public Cleanable {
virtual Status Refresh(const class Snapshot*) {
return Status::NotSupported("Refresh() is not supported");
}

// Internally - previous check to Refresh
virtual bool IsAllowRefresh() { return true; }
// Property "rocksdb.iterator.is-key-pinned":
// If returning "1", this means that the Slice returned by key() is valid
// as long as the iterator is not deleted.
Expand Down
4 changes: 4 additions & 0 deletions include/rocksdb/memtablerep.h
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,9 @@ class MemTableRepFactory : public Customizable {
// false when if the <key,seq> already exists.
// Default: false
virtual bool CanHandleDuplicatedKey() const { return false; }

// Return true if the memtable support Refresh()
virtual bool IsRefreshIterSupported() const { return true; }
};

// This uses a skip list to store keys. It is the default.
Expand Down Expand Up @@ -378,6 +381,7 @@ class VectorRepFactory : public MemTableRepFactory {
static const char* kNickName() { return "vector"; }
const char* Name() const override { return kClassName(); }
const char* NickName() const override { return kNickName(); }
bool IsRefreshIterSupported() const override { return false; }

// Methods for MemTableRepFactory class overrides
using MemTableRepFactory::CreateMemTableRep;
Expand Down

0 comments on commit e231df1

Please sign in to comment.