Skip to content

feat(persistence): bounded directory discovery and advisory index repair - #74

Open
PoisonGodv wants to merge 1 commit into
agentic-ai-java:mainfrom
PoisonGodv:feat/bounded-directory-protocol
Open

PoisonGodv wants to merge 1 commit into
agentic-ai-java:mainfrom
PoisonGodv:feat/bounded-directory-protocol

Conversation

@PoisonGodv

Copy link
Copy Markdown
Contributor

Describe what this PR does / why we need it

Issue #8 asks for Redis polling claim, expiry recovery, and an advisory repair index without introducing MQ or Redis Streams. Its full scope also includes a complete Worker and independent-JVM takeover; this PR does not deliver those.

This PR submits an opt-in, single-primary protocol slice for design review, building on the LeasedProgressStore from #69 (#7): a bounded membership directory, an advisory index with a bounded repair scan, and a reference consumer that demonstrates the claim-to-dispatch safety checks. Existing APIs, default behavior, stored formats, and configuration prefixes are unchanged; nothing new is active unless directory mode is explicitly enabled. The module README documents the new opt-in modes and their boundaries.

Does this pull request fix one issue?

NONE. Refs #8. This is a partial protocol slice submitted for design review, not a complete Worker, and it does not claim to meet #8's acceptance criteria for crash recovery, dual-Worker single-owner guarantees, or bounded polling budgets end to end.

Describe how you did it

  • RedisProgressDirectory: an explicit, immutable-capacity directory (1..10000 retained session/run identities). Claim and membership registration complete atomically in the same Lua call: the wrapper limits the original CLAIM script's KEYS to the authority hashes and performs the membership ZADD only after an OK reply, preserving the existing type/data/fence validation. Capacity and wrong-type rejections leave no orphan authority, and concurrent contenders cannot over-admit.
  • Bounded discovery: directory scores are all zero, membership is immutable and retained, page(after, limit) uses exclusive lexicographic continuation with limit 1..1000, and there is no pending SCAN page. A quiescent full sweep covers registered membership; an insertion behind the finished cursor appears on the next sweep.
  • Controlled migration: existing legacy authority is invisible to discovery until an explicit registerExisting call; duplicate registration is idempotent.
  • RedisProgressAdvisoryIndex / RedisProgressIndexRepairer: advisory publication plus a repair scan that records bounded issues for corrupt records and advances past them, without treating the index as state truth. One repair call performs one page query, at most limit projections, and at most limit publications.
  • Exact int64 delivery tokens (HINCRBY then string read; no Lua double conversion) with ack/retry/backoff. Poll moves delivered scores against Redis TIME; exact token comparisons prevent an old delivery from acking or retrying a newer delivery or revision. Retry invalidates its token, so repeated retries cannot extend backoff. Preflight reserves room for the maximum 1000-token batch and fails closed near int64 exhaustion.
  • RedisProgressReferenceConsumer: after claim, metadata is read atomically together with reservation, live owner/fence, and server-time validation; candidate revision and an explicit runnable predicate are checked; the guard renews and revalidates immediately before dispatch. Revision/state changes, expires=0, reservation mismatch, and stale fences are rejected; commit is fenced and ack follows commit. This is a reference implementation for review, not a production Worker; unknown application states require an explicit adapter and need not be runnable.

Measured/mechanical boundaries, stated so reviewers can judge them directly:

  • Terminal and uncommitted revision-zero identities permanently retain their capacity slots under this protocol; no GC or reset is implemented. A full directory permanently rejects new identities.
  • Legacy or mixed writers that bypass directory mode remain invisible to discovery; explicit migration is required. A passing test counterexample establishes this.
  • The original claim/commit/snapshot paths still load accumulated run history and checkpoint payloads without hard upper bounds; this slice does not close that.
  • Byte bounds (directory member, RESP2 pages, projected fields) and complexity notes are listed in the module README's directory section.

Describe how you verified it

All Redis-touching tests run against real Valkey 8.1.2 via Testcontainers (AOF, appendfsync always); they are disabled when Docker is absent rather than silently passing.

Two executions back this submission:

  • Full module run: 133 tests, 0 failures, 0 errors, 0 skips across ten suites in one fresh Maven invocation — 19 new directory/protocol tests plus the prior 114 module tests. One intermediate failure (a new validator misclassifying expires=0, contrary to the existing store's canonical nonnegative expiry contract) was corrected before the final run without changing the failing assertion.
  • Independent review run, in a separate tree rebuilt byte-for-byte from the same base commit: 18 tests — six new adversarial probes plus twelve selected existing regressions — 0 failures, 0 errors, 0 skips. The probes cover terminal/revision-zero slot retention and reclamation, mixed-writer invisibility, claim-to-dispatch revision/state races via the real fenced store API, reservation mismatch and zero-expiry handling at commit, stale delivery-token ack/retry against newer deliveries and fresh tokens, and one-entry directory pages advancing past noncanonical revision, oversized state, and missing run authority while recording bounded issues.

Formatting and lint: scoped Spring JavaFormat 0.0.39 apply/validate passed, and root/module Checkstyle reported zero violations, in the review run. License-eye repository check on the implementation delivery: 773 checked, 585 valid, 188 ignored, 0 invalid. The review-probe test file added afterward carries the repository Apache header but was not covered by a new license-eye run. Whole-repository build and CI were not run locally.

To verify locally with Docker available:

./mvnw -B -pl :argi-graph-persistence-redis -am test

Special notes for reviews

Requesting design review before any further Worker work, because three policy decisions belong to maintainers rather than this slice:

  1. Capacity: whether to accept finite retained capacity (1..10000) with terminal/revision-zero retention semantics, and what capacity/partitioning to select. No GC or reset is implemented.
  2. Migration: whether registerExisting is the right migration contract, given that mixed writers must be quiesced and all future producers enforced through directory mode; other legacy keyspaces are not auto-migrated.
  3. Retention/resource goals: whether whole claim/commit/snapshot response costs must be hard bounded; if so, that needs a separate design for accumulated checkpoint/journal/outbox history.

Further limits: a single authoritative primary is assumed (no Cluster/Sentinel/failover certification; Lua gives isolation, not rollback); external effect fencing and idempotency remain application responsibilities; independent-JVM takeover and OS-kill recovery were not run and belong to the #8 follow-up.

This submission was prepared with AI assistance (Claude) under a two-stage workflow: implementation, then an independent review whose adversarial probes are included as RedisProgressProtocolReviewTest and whose findings are summarized above.

Add an opt-in, single-primary Redis directory protocol slice for the
leased progress store introduced for agentic-ai-java#7:

- RedisProgressDirectory: explicit fixed-capacity directory where claim
  and membership registration complete atomically in one Lua call, with
  bounded lexicographic continuation paging (limit 1..1000) and a
  controlled registerExisting migration path for legacy authority.
- RedisProgressAdvisoryIndex and RedisProgressIndexRepairer: advisory
  discovery/repair scan that records bounded issues for corrupt records
  and advances past them without treating the index as state truth.
- RedisLeasedProgressStore gains the directory mode integration points;
  existing API, default behavior, and stored formats are unchanged.
- RedisProgressReferenceConsumer: reference consumer that revalidates
  revision, state, reservation, owner, and fence atomically after claim
  and again before dispatch, with exact int64 delivery tokens for
  ack/retry/backoff.
- Tests: directory, repairer, and protocol suites including six
  adversarial review probes, all executed against real Valkey 8.1.2.

Refs agentic-ai-java#8. This is a protocol slice submitted for design review, not a
complete Worker: capacity, migration, and retention policy decisions
are listed as open maintainer decisions in the PR description.

Signed-off-by: lnw <951448078@qq.com>
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