Skip to content

feat: add IBigSegmentStore interface + Redis and DynamoDB stores#536

Open
beekld wants to merge 3 commits into
mainfrom
beeklimt/SDK-2363
Open

feat: add IBigSegmentStore interface + Redis and DynamoDB stores#536
beekld wants to merge 3 commits into
mainfrom
beeklimt/SDK-2363

Conversation

@beekld
Copy link
Copy Markdown
Contributor

@beekld beekld commented May 19, 2026

Ticket: SDK-2363 · Follows #534

Summary

Adds the public IBigSegmentStore interface, the Membership / StoreMetadata value types, and concrete DynamoDB + Redis implementations. Schema strings match what the Relay Proxy writes.

auto redis_store = RedisBigSegmentStore::Create("redis://localhost:6379", "prefix");
auto dynamo_store = DynamoDBBigSegmentStore::Create("my-table", "prefix", options);

Design notes

  • synchronizedOn parsing. Stored as DynamoDB N / Redis string. Both stores reject malformed values (non-numeric strings, wrong DynamoDB attribute type) rather than silently returning 0, matching the existing dynamodb_source.cpp row validation.
  • No hashing here. The interface contract says callers pass the already-hashed base64 SHA-256 context key; the SDK will hash in the wrapper, not the store implementations.

Not in scope

  • BigSegmentsBuilder config plumbing.
  • BigSegmentStoreWrapper (LRU cache + staleness polling) and the hashing path.
  • Evaluator wiring / replacing the rules.cpp big-segments TODO.

Test plan

  • 7 Membership unit tests pass.
  • 9 RedisBigSegmentStore integration tests pass against Redis 7 (docker).
  • 8 DynamoDBBigSegmentStore integration tests pass against DynamoDB Local.
  • Full server-sdk test suite (468 tests) still green.
  • Schema constants match what Relay writes (verified against the LocalStack/Redis fixture data).

Note

Medium Risk
Introduces new public Big Segments store API and two new persistence integrations (Redis/DynamoDB), which could affect consumers and runtime behavior if schema/connection handling is incorrect. Risk is moderated by being additive and covered by new unit/integration tests, but it touches external storage and parsing/validation paths.

Overview
Adds a new public Big Segments persistence API via IBigSegmentStore plus value types Membership and StoreMetadata, including semantics like inclusion wins and explicit error propagation.

Implements concrete read-only Big Segments stores for DynamoDB (DynamoDBBigSegmentStore) and Redis (RedisBigSegmentStore), wires them into their respective CMake builds, and adds schema constants/validation (notably strict synchronizedOn parsing and prefix-scoped keys/namespaces) with new unit and integration tests covering empty stores, include/exclude resolution, prefix isolation, malformed metadata, and unreachable Redis errors.

Reviewed by Cursor Bugbot for commit a3dfc38. Bugbot is set up for automated code reviews on this repo. Configure here.

@beekld beekld marked this pull request as ready for review May 20, 2026 18:50
@beekld beekld requested a review from a team as a code owner May 20, 2026 18:50
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default mode and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit a3dfc38. Configure here.

it != item.end()) {
for (auto const& ref : it->second.GetSS()) {
excluded.emplace_back(ref);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unvalidated GetSS() silently drops membership data on type mismatch

Medium Severity

The GetSS() calls on the included and excluded attributes are not validated for type correctness. AWS SDK's GetSS() silently returns an empty vector when the attribute is a non-SS type (S, N, BOOL, etc.), just as GetS() returns "" for non-String types. Since DynamoDB enforces that SS attributes must have at least one element, an empty result from GetSS() when the attribute key exists in the item indicates a type mismatch. The GetN() call for synchronizedOn at line 131 correctly validates for empty, but these GetSS() calls do not, causing silent membership data loss instead of surfacing an error.

Fix in Cursor Fix in Web

Triggered by learned rule: DynamoDB source: validate AttributeValue type before using GetS()

Reviewed by Cursor Bugbot for commit a3dfc38. Configure here.

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.

1 participant