From 1445ba17a1a9b9e396368b115fa1d3a229c8b7e9 Mon Sep 17 00:00:00 2001 From: Yingxin Cheng Date: Tue, 29 Jun 2021 09:39:10 +0800 Subject: [PATCH] crimson/os/seastore/cache: add absent extent to read-set before retire So that transaction can be invalidated if the retiring extent has conflict. Signed-off-by: Yingxin Cheng --- src/crimson/os/seastore/cache.cc | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc index 478d94d9404..49cb3a968ff 100644 --- a/src/crimson/os/seastore/cache.cc +++ b/src/crimson/os/seastore/cache.cc @@ -45,6 +45,7 @@ Cache::retire_extent_ret Cache::retire_extent_addr( // absent from transaction result = query_cache_for_extent(addr, &ext); if (result == Transaction::get_extent_ret::PRESENT) { + t.add_to_read_set(ext); return trans_intr::make_interruptible( ext->wait_io() ).then_interruptible([&t, ext=std::move(ext)]() mutable { @@ -52,6 +53,21 @@ Cache::retire_extent_ret Cache::retire_extent_addr( return retire_extent_iertr::now(); }); } else { // result == get_extent_ret::ABSENT + // FIXME this will cause incorrect transaction invalidation because t + // will not be notified if other transactions that modify the extent at + // this addr are committed. + // + // Say that we have transaction A and B, conflicting on extent E at laddr + // L: + // A: dec_ref(L) // cause uncached retirement + // B: read(L) -> E + // B: mutate(E) + // B: submit_transaction() // assume successful + // A: about to submit... + // + // A cannot be invalidated because E is not in A's read-set + // + // TODO: leverage RetiredExtentPlaceholder to fix the issue. t.add_to_retired_uncached(addr, length); return retire_extent_iertr::now(); } -- 2.39.5