Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fetched_items not fully cleared #4502

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 1 addition & 17 deletions .github/workflows/reusable-container-workflow.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -148,22 +148,6 @@ jobs:
# only load AMD64 image for testing
load: true

- if: ${{ hashFiles(format('{0}-{1}', matrix.dockerfile, inputs.build_type)) }}
name: Build release image for arm64
uses: docker/build-push-action@v6
with:
context: .
platforms: linux/arm64
build-args: |
QEMU_CPU=max,pauth-impdef=on
push: false
tags: |
${{ steps.metadata.outputs.tags }}
labels: ${{ steps.metadata.outputs.labels }}
file: ${{ matrix.dockerfile }}-${{ inputs.build_type }}
cache-from: type=gha
cache-to: type=gha,mode=max

- if: ${{ hashFiles(format('{0}-{1}', matrix.dockerfile, inputs.build_type)) }}
name: Test Docker run
run: |
Expand Down Expand Up @@ -193,7 +177,7 @@ jobs:
uses: docker/build-push-action@v6
with:
context: .
platforms: linux/amd64,linux/arm64
platforms: linux/amd64
push: true
build-args: |
QEMU_CPU=max,pauth-impdef=on
Expand Down
2 changes: 2 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ configure:

build:
cd $(RELEASE_DIR); \
ninja mimalloc_project; \
cat third_party/src/mimalloc_project-stamp/mimalloc_project-build-*.log; \
ninja dragonfly && ldd dragonfly

package:
Expand Down
38 changes: 22 additions & 16 deletions src/server/db_slice.cc
Original file line number Diff line number Diff line change
Expand Up @@ -218,16 +218,6 @@ unsigned PrimeEvictionPolicy::Evict(const PrimeTable::HotspotBuckets& eb, PrimeT
return 1;
}

// Deprecated and should be removed.
class FetchedItemsRestorer {
public:
template <typename U> explicit FetchedItemsRestorer(U&& u) {
}

~FetchedItemsRestorer() {
}
};

} // namespace

#define ADD(x) (x) += o.x
Expand Down Expand Up @@ -270,6 +260,25 @@ SliceEvents& SliceEvents::operator+=(const SliceEvents& o) {

#undef ADD

class DbSlice::PrimeBumpPolicy {
public:
PrimeBumpPolicy(absl::flat_hash_set<uint64_t, FpHasher>* items) : fetched_items_(items) {
}

// returns true if we can change the object location in dash table.
bool CanBump(const CompactObj& obj) const {
if (obj.IsSticky()) {
return false;
}

auto hc = obj.HashCode();
return fetched_items_->insert(hc).second;
}

private:
mutable absl::flat_hash_set<uint64_t, FpHasher>* fetched_items_;
};

DbSlice::DbSlice(uint32_t index, bool caching_mode, EngineShard* owner)
: shard_id_(index),
caching_mode_(caching_mode),
Expand Down Expand Up @@ -473,13 +482,14 @@ OpResult<DbSlice::PrimeItAndExp> DbSlice::FindInternal(const Context& cntx, std:

if (caching_mode_ && IsValid(res.it)) {
if (!change_cb_.empty()) {
FetchedItemsRestorer fetched_restorer(&fetched_items_);
auto bump_cb = [&](PrimeTable::bucket_iterator bit) {
CallChangeCallbacks(cntx.db_index, key, bit);
};
db.prime.CVCUponBump(change_cb_.back().first, res.it, bump_cb);
}

VLOG(1) << "BumpUp " << key << " " << fb2::GetStacktrace();

block_counter_.Wait(); // We must not change the bucket's internal order during serialization
auto bump_it = db.prime.BumpUp(res.it, PrimeBumpPolicy{&fetched_items_});
if (bump_it != res.it) { // the item was bumped
Expand Down Expand Up @@ -566,8 +576,6 @@ OpResult<DbSlice::AddOrFindResult> DbSlice::AddOrFindInternal(const Context& cnt
auto status = res.status();
CHECK(status == OpStatus::KEY_NOTFOUND || status == OpStatus::OUT_OF_MEMORY) << status;

FetchedItemsRestorer fetched_restorer(&fetched_items_);

// It's a new entry.
CallChangeCallbacks(cntx.db_index, key, {key});

Expand Down Expand Up @@ -788,7 +796,7 @@ void DbSlice::FlushDbIndexes(const std::vector<DbIndex>& indexes) {
std::swap(db_arr_[index]->trans_locks, flush_db_arr[index]->trans_locks);
}

CHECK(fetched_items_.empty());
CHECK_EQ(fetched_items_.size(), 0u);

auto cb = [indexes, flush_db_arr = std::move(flush_db_arr)]() mutable {
flush_db_arr.clear();
Expand Down Expand Up @@ -1057,7 +1065,6 @@ bool DbSlice::CheckLock(IntentLock::Mode mode, DbIndex dbid, uint64_t fp) const
}

void DbSlice::PreUpdate(DbIndex db_ind, Iterator it, std::string_view key) {
FetchedItemsRestorer fetched_restorer(&fetched_items_);
CallChangeCallbacks(db_ind, key, ChangeReq{it.GetInnerIt()});
it.GetInnerIt().SetVersion(NextVersion());
}
Expand Down Expand Up @@ -1176,7 +1183,6 @@ uint64_t DbSlice::RegisterOnChange(ChangeCallback cb) {
}

void DbSlice::FlushChangeToEarlierCallbacks(DbIndex db_ind, Iterator it, uint64_t upper_bound) {
FetchedItemsRestorer fetched_restorer(&fetched_items_);
std::unique_lock<LocalBlockingCounter> lk(block_counter_);

uint64_t bucket_version = it.GetVersion();
Expand Down
18 changes: 1 addition & 17 deletions src/server/db_slice.h
Original file line number Diff line number Diff line change
Expand Up @@ -647,23 +647,7 @@ class DbSlice {
absl::container_internal::hash_default_eq<std::string>, AllocatorType>
client_tracking_map_;

class PrimeBumpPolicy {
public:
PrimeBumpPolicy(absl::flat_hash_set<uint64_t, FpHasher>* items) : fetched_items_(items) {
}

// returns true if we can change the object location in dash table.
bool CanBump(const CompactObj& obj) const {
if (obj.IsSticky()) {
return false;
}
auto hc = obj.HashCode();
return fetched_items_->insert(hc).second;
}

private:
mutable absl::flat_hash_set<uint64_t, FpHasher>* fetched_items_;
};
class PrimeBumpPolicy;
};

inline bool IsValid(const DbSlice::Iterator& it) {
Expand Down
2 changes: 1 addition & 1 deletion tools/packaging/Dockerfile.ubuntu-dev
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# syntax=docker/dockerfile:1
FROM ghcr.io/romange/ubuntu-dev:20 as builder
FROM ghcr.io/romange/ubuntu-dev:20 AS builder

WORKDIR /build

Expand Down
Loading