bahman/scylla
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

atomic_cell: compare_atomic_cell_for_merge: compare ttl if expiry is equal

Browse Source 
Unlike atomic_cell_or_collection::equals, compare_atomic_cell_for_merge
currently returns std::strong_ordering::equal if two cells are equal in
every way except their ttl:s.

The problem with that is that the cells' hashes are different and this
will cause repair to keep trying to repair discrepancies caused by the
ttl being different.

This may be triggered by e.g. the spark migrator that computes the ttl
based on the expiry time by subtracting the expiry time from the current
time to produce a respective ttl.

If the cell is migrated multiple times at different times, it will generate
cells that the same expiry (by design) but have different ttl values.

Fixes #10156

Test: mutation_test.test_cell_ordering, unit(dev)

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Message-Id: <20220302154328.2400717-1-bhalevy@scylladb.com>
This commit is contained in:
Benny Halevy
2022-03-02 17:43:28 +02:00
committed by Nadav Har'El
parent d3673f2b29
commit a57c087c89
2 changed files with 8 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
atomic_cell.cc
View File

@@ -88,7 +88,11 @@ compare_atomic_cell_for_merge(atomic_cell_view left, atomic_cell_view right) {
return left.is_live_and_has_ttl() ? std::strong_ordering::greater : std::strong_ordering::less;
}
if (left.is_live_and_has_ttl()) {
return left.expiry() <=> right.expiry();
if (left.expiry() != right.expiry()) {
return left.expiry() <=> right.expiry();
} else {
return left.ttl() <=> right.ttl();
}
}
} else {
// Both are deleted

4
test/boost/mutation_test.cc
View File

@@ -690,6 +690,7 @@ SEASTAR_TEST_CASE(test_cell_ordering) {
};
auto assert_equal = [] (atomic_cell_view c1, atomic_cell_view c2) {
testlog.trace("Expected {} == {}", c1, c2);
BOOST_REQUIRE(compare_atomic_cell_for_merge(c1, c2) == 0);
BOOST_REQUIRE(compare_atomic_cell_for_merge(c2, c1) == 0);
};
@@ -711,7 +712,8 @@ SEASTAR_TEST_CASE(test_cell_ordering) {
atomic_cell::make_live(*bytes_type, 1, bytes(), expiry_2, ttl_2));
// Origin doesn't compare ttl (is it wise?)
assert_equal(
// But we do. See https://github.com/scylladb/scylla/issues/10156
assert_order(
atomic_cell::make_live(*bytes_type, 1, bytes("value"), expiry_1, ttl_1),
atomic_cell::make_live(*bytes_type, 1, bytes("value"), expiry_1, ttl_2));