Skip to content

Commit 8e0e60f

Browse files
jimwwalkerdaverigby
authored andcommitted
MB-32568: Set DeleteSource in StoredValue::setValueImpl
setValueImpl leaves the delete source member untouched meaning we can end up with a StoredValue in a state which doesn't match it's source. Change-Id: I6beb050e797a7eb080fe7d64c464a6c1f9f6e7a9 Reviewed-on: http://review.couchbase.org/103484 Reviewed-by: Chris Farman <[email protected]> Tested-by: Build Bot <[email protected]> Reviewed-by: Dave Rigby <[email protected]>
1 parent 65249ea commit 8e0e60f

File tree

3 files changed

+31
-3
lines changed

3 files changed

+31
-3
lines changed

engines/ep/src/item.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -295,8 +295,7 @@ void Item::setDeleted(DeleteSource cause) {
295295
case queue_op::system_event:
296296
if (cause == DeleteSource::TTL) {
297297
throw std::logic_error(
298-
"Item::setDeleted should not expire a system_event " +
299-
to_string(op));
298+
"Item::setDeleted should not expire a system_event");
300299
}
301300
deleted = 1; // true
302301
deletionCause = static_cast<uint8_t>(cause);

engines/ep/src/stored-value.cc

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ StoredValue::StoredValue(const Item& itm,
4646
flags(itm.getFlags()),
4747
revSeqno(itm.getRevSeqno()),
4848
datatype(itm.getDataType()),
49+
deletionSource(0),
4950
committed(static_cast<uint8_t>(CommittedState::CommittedViaMutation)) {
5051
// Initialise bit fields
5152
setDeletedPriv(itm.isDeleted());
@@ -338,10 +339,13 @@ void StoredValue::setValueImpl(const Item& itm) {
338339
}
339340

340341
setDeletedPriv(itm.isDeleted());
342+
if (itm.isDeleted()) {
343+
setDeletionSource(itm.deletionSource());
344+
}
345+
341346
flags = itm.getFlags();
342347
datatype = itm.getDataType();
343348
bySeqno = itm.getBySeqno();
344-
345349
cas = itm.getCas();
346350
lock_expiry_or_delete_time = 0;
347351
exptime = itm.getExptime();
@@ -452,6 +456,11 @@ std::ostream& operator<<(std::ostream& os, const StoredValue& sv) {
452456
const auto* osv = sv.toOrderedStoredValue();
453457
os << (osv->isStalePriv() ? 'S' : '.');
454458
}
459+
460+
if (sv.isDeleted() && sv.getDeletionSource() == DeleteSource::TTL) {
461+
os << "TTL";
462+
}
463+
455464
os << ' ';
456465

457466
// Temporary states

engines/ep/tests/module_tests/stored_value_test.cc

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,26 @@ TEST(StoredValueTest, expectedSize) {
370370
<< "Unexpected change in StoredValue storage size for key: " << key;
371371
}
372372

373+
// Validate the deletion source propagates via setValue
374+
TYPED_TEST(ValueTest, MB_32568) {
375+
Item itm(makeStoredDocKey("k"),
376+
0,
377+
0,
378+
(const value_t)TaggedPtr<Blob>{},
379+
PROTOCOL_BINARY_RAW_BYTES,
380+
0,
381+
StoredValue::state_temp_init);
382+
itm.setDeleted();
383+
this->sv->setValue(itm);
384+
EXPECT_EQ(DeleteSource::Explicit, this->sv->getDeletionSource());
385+
386+
// And now for TTL
387+
this->sv = this->factory(this->item, {});
388+
itm.setDeleted(DeleteSource::TTL);
389+
this->sv->setValue(itm);
390+
EXPECT_EQ(DeleteSource::TTL, this->sv->getDeletionSource());
391+
}
392+
373393
/**
374394
* Test fixture for OrderedStoredValue-only tests.
375395
*/

0 commit comments

Comments
 (0)