Make iterator invalid on Merge error

Summary:
Since #1665, on merge error, iterator will be set to corrupted status, but it doesn't invalidate the iterator. Fixing it.
Closes https://github.com/facebook/rocksdb/pull/3226

Differential Revision: D6499094

Pulled By: yiwu-arbug

fbshipit-source-id: 80222930f949e31f90a6feaa37ddc3529b510d2c
This commit is contained in:
Yi Wu 2017-12-06 11:54:45 -08:00 committed by Facebook Github Bot
parent 36911f55dd
commit 20995c5729
3 changed files with 36 additions and 10 deletions

View File

@ -4,6 +4,7 @@
### New Features
### Bug Fixes
* Fix IOError on WAL write doesn't propagate to write group follower
* Make iterator invalid on merge error.
## 5.9.0 (11/1/2017)
### Public API Change

View File

@ -640,6 +640,7 @@ void DBIter::MergeValuesNewToOld() {
merge_operator_, ikey.user_key, &val, merge_context_.GetOperands(),
&saved_value_, logger_, statistics_, env_, &pinned_value_, true);
if (!s.ok()) {
valid_ = false;
status_ = s;
}
// iter_ is positioned after put
@ -677,6 +678,7 @@ void DBIter::MergeValuesNewToOld() {
&saved_value_, logger_, statistics_, env_,
&pinned_value_, true);
if (!s.ok()) {
valid_ = false;
status_ = s;
}
}
@ -946,8 +948,10 @@ bool DBIter::FindValueForCurrentKey() {
assert(false);
break;
}
valid_ = true;
if (!s.ok()) {
if (s.ok()) {
valid_ = true;
} else {
valid_ = false;
status_ = s;
}
return true;
@ -1023,8 +1027,10 @@ bool DBIter::FindValueForCurrentKeyUsingSeek() {
iter_->Seek(last_key);
RecordTick(statistics_, NUMBER_OF_RESEEKS_IN_ITERATION);
}
valid_ = true;
if (!s.ok()) {
if (s.ok()) {
valid_ = true;
} else {
valid_ = false;
status_ = s;
}
return true;
@ -1035,8 +1041,10 @@ bool DBIter::FindValueForCurrentKeyUsingSeek() {
&val, merge_context_.GetOperands(),
&saved_value_, logger_, statistics_, env_,
&pinned_value_, true);
valid_ = true;
if (!s.ok()) {
if (s.ok()) {
valid_ = true;
} else {
valid_ = false;
status_ = s;
}
return true;

View File

@ -133,16 +133,33 @@ TEST_F(DBMergeOperatorTest, MergeErrorOnIteration) {
ASSERT_OK(Merge("k1", "v1"));
ASSERT_OK(Merge("k1", "corrupted"));
ASSERT_OK(Put("k2", "v2"));
VerifyDBFromMap({{"k1", ""}, {"k2", "v2"}}, nullptr, false,
{{"k1", Status::Corruption()}});
auto* iter = db_->NewIterator(ReadOptions());
iter->Seek("k1");
ASSERT_FALSE(iter->Valid());
ASSERT_TRUE(iter->status().IsCorruption());
delete iter;
iter = db_->NewIterator(ReadOptions());
iter->Seek("k2");
ASSERT_TRUE(iter->Valid());
ASSERT_OK(iter->status());
iter->Prev();
ASSERT_FALSE(iter->Valid());
ASSERT_TRUE(iter->status().IsCorruption());
delete iter;
VerifyDBInternal({{"k1", "corrupted"}, {"k1", "v1"}, {"k2", "v2"}});
DestroyAndReopen(options);
ASSERT_OK(Merge("k1", "v1"));
ASSERT_OK(Put("k2", "v2"));
ASSERT_OK(Merge("k2", "corrupted"));
VerifyDBFromMap({{"k1", "v1"}, {"k2", ""}}, nullptr, false,
{{"k2", Status::Corruption()}});
iter = db_->NewIterator(ReadOptions());
iter->Seek("k1");
ASSERT_TRUE(iter->Valid());
ASSERT_OK(iter->status());
iter->Next();
ASSERT_FALSE(iter->Valid());
ASSERT_TRUE(iter->status().IsCorruption());
delete iter;
VerifyDBInternal({{"k1", "v1"}, {"k2", "corrupted"}, {"k2", "v2"}});
}