Skip to content

feat: add internal BigSegmentStoreWrapper with caching and status polling#543

Open
beekld wants to merge 4 commits into
mainfrom
beeklimt/SDK-2365
Open

feat: add internal BigSegmentStoreWrapper with caching and status polling#543
beekld wants to merge 4 commits into
mainfrom
beeklimt/SDK-2365

Conversation

@beekld
Copy link
Copy Markdown
Contributor

@beekld beekld commented Jun 2, 2026

Summary

Internal layer between the evaluator and a customer-provided
IBigSegmentStore: hashes context keys, caches membership lookups, polls the
store's metadata in the background to track availability/staleness, and
broadcasts status changes. No caller yet — the evaluator and the public status
provider come later in the stack.

Design notes

  • Context keys are hashed to base64(sha256(key)) using the standard base64
    alphabet (not the existing URL-safe Base64UrlEncode), so the lookup key
    matches what the Relay Proxy writes.
  • Concurrent misses for the same key coalesce into a single store query rather
    than each hitting the store.
  • The background poll is built on async::Delay + CancellationSource.
  • A store error encountered during a membership lookup flips the store status to
    unavailable. The spec requires this; the Java and Go SDKs don't do it, so this
    is a deliberate divergence.
  • Changes StoreMetadata::last_up_to_date from a bare milliseconds to a
    system_clock::time_point, so staleness math can't mix epochs. This touches
    the Redis and DynamoDB stores (one line each) and their metadata tests.

Test plan

  • Added unit tests
  • Full server-sdk suite green (499/499)
  • DynamoDB source suite green (40/40) against DynamoDB Local
  • Redis source suite green (37/37) against a live Redis

Note

Cursor Bugbot is generating a summary for commit 2b88796. Configure here.

Base automatically changed from beeklimt/SDK-2364 to main June 2, 2026 17:05
@beekld beekld force-pushed the beeklimt/SDK-2365 branch from f03dd53 to 35fb2ae Compare June 2, 2026 17:12
@beekld beekld marked this pull request as ready for review June 2, 2026 17:38
@beekld beekld requested a review from a team as a code owner June 2, 2026 17:38

// Query the store outside any lock, then publish the result to waiters and
// drop the in-flight entry.
auto result = store_->GetMembership(HashContextKey(context_key));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does IBigSegmentStore::GetMembership need to be noexcept?

@kinyoklion
Copy link
Copy Markdown
Member

A store error encountered during a membership lookup flips the store status to
unavailable. The spec requires this; the Java and Go SDKs don't do it, so this
is a deliberate divergence.

We'll want to determine if we should update the spec here or the implementations. It does sound reasonably though.


BigSegmentStoreStatus BigSegmentStoreWrapper::GetStatus() {
{
std::lock_guard lock(status_mutex_);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not a major concern, but should this have the same type of in-flight check as membership?

(It appears implementation side this is inconsistent, go/java just let it thunder while .Net creates a task and then re-uses it for subsequent waiters.)


/**
* @brief Registers a listener invoked whenever the store status changes
* (not on every poll). The returned connection unregisters on destruction.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the connection actually un-register on destruction?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants