Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Evidence added to the PR descriptionThe description now includes a table of contents, before/after measurements, database plan evidence, Redis/Valkey smoke evidence, ACID results, FMEA coverage, validation commands, and explicit open gates. Key measured result with a fixed batch of 300:
The 1M-row candidate result is approximately 157.6x faster at p99. The smaller fixtures expose index overhead and are reported as such. PostgreSQL ACID checks pass for rollback, atomic commit, idempotent replay, tenant scope, foreign-key ownership, and duplicate prevention. Redis 7.4.11 smoke checks pass for These are evidence gates, not claims that the runtime implementation is complete. Worker kill/restart, broker-loss recovery, domain-aware redrive, both queue strategies, connection-pool soak, operator audit, and rollback drills remain open and are listed in the description. |
|
Clarification: the SRE/FMEA material in this description is non-normative reader context. It is included to make failure modes and deliverability concerns easier to reason about. It does not propose adding a repository-wide FMEA framework, new process files, or external operational tooling. The only contribution obligations are the affected Open Mercato specs and, in a later implementation PR, the minimal repo-native code and tests required by the accepted design. |
|
Clarification for reviewers: the evidence has four lanes. A = current Open Mercato baseline ( |
Removed internal benchmark command examples from the implementation PR requirements.
|
The docs/spec lane has now been integrated with PR 9 onto the PR open-mercato#5450 base in integration PR 10. |
PR 5450 Staging Description
Summary
Review-only staging PR for the background-work reliability specifications from upstream PR open-mercato#5450. This branch adds a narrowly scoped complexity and verification gate to the existing spec decisions.
The intended contract is not absolute O(1). For a fixed repair batch
B, unrelated live jobs must not increase repair work, rows read, or memory beyond index-depth effects. Actual due work costsO(B)per tick and drains across ticks.Comparison lanes
These are four separate states. Results must never be described as if they compare the wrong pair.
developis the normal contribution baseline.mainis the release baseline.open-mercato/open-mercato:developandmainopen-mercato/open-mercato#5450, head60e0bac3b7TWN-Systems/open-mercato,docs/pr5450-review-candidateTWN-Systems/open-mercato#9,feat/pr5450-repair-cellsThe benchmark below is not a measurement of lane D or production Open Mercato. It is a disposable PostgreSQL model comparing a faithful sweep-shaped query with an indexed due-cell query. Lane D must reproduce this comparison inside Open Mercato using real progress/data-sync tables and workers.
Table of contents
Scope
review/pr5450-base, pinned to upstream PR docs(specs): background-work reliability series v5 — problems, requirements, staged decision, phase-1 specs open-mercato/open-mercato#5450 head60e0bac3b7.Model before and after
This is a model comparison, not a claim that lane C changed runtime behavior. The model baseline represents lane A's sweep-shaped behavior. The model candidate represents the intended lane D repair-cell behavior. Lanes B and C are documentation/spec states.
The model baseline is the faithful wide-row live-job predicate. The model candidate is an indexed due-cell lookup with a fixed batch of 300.
At 1M live rows, the model candidate is approximately 157.6x faster at p99. The small-fixture model candidate is slower because index and sort overhead dominate. That is reported rather than hidden. No speedup is attributed to lane B or lane C.
The JS oracle reports identical selected IDs and changes inspected rows from live count to 300 due cells. It is an algorithmic shape check, not a production claim.
Database model evidence
PostgreSQL 17.11 was run in the Open Mercato compose service with disposable benchmark tables. The benchmark records
EXPLAIN (ANALYZE, BUFFERS)execution time, p50/p95/p99, rows, shared hits/reads, sequential scans, and index scans. This is evidence for the shape of lane D, not evidence that lane D exists.ACID checks pass:
Redis and Valkey evidence
The compose Redis service was started and smoke-tested:
This confirms service reachability and basic key semantics only. It does not claim worker recovery after broker loss, eviction safety, or at-least-once delivery under a real Open Mercato worker. Those remain explicit integration gates.
Correctness and safety
The regression suite passes due-cell selection shape, atomic state movement, durable retry attempt state, tenant scope, duplicate prevention, monotone backlog drain, foreign-key ownership, and basic Redis reachability.
Every implementation PR must additionally test worker kill/restart, concurrent redrive, domain-row reopen, operator audit, and both queue strategies (
localandasync) inside Open Mercato module integration tests.FMEA and failure conditions
The SRE/FMEA material is reader context only. It gives the failure vocabulary and mental model behind the performance and deliverability questions. It is not a request to add an Open Mercato-wide FMEA framework, new repository process files, or external operational tooling.
The concrete review risks remain local to this spec: wide reconciliation scans, unbounded due work, lost repair coordinates, duplicate transitions, retry-budget loss, redrive/domain divergence, and tenant-scope errors. The implementation PR must test those risks in the affected modules. Worker death/restart, Redis/Valkey loss, operator audit, and rollback are follow-up operational scenarios, not claims that this documentation-only PR has implemented them.
The failure rule is fail-closed: a faster result with missed repair, duplicate repair, scope leakage, lost attempts, or missing operator evidence is a failure.
Validation commands
Private modeling and evidence commands:
Open Mercato validation required before an implementation PR:
Current implementation evidence
PR #9 currently reports the following fork validation:
35/35tests passed, including directory recreation recovery.104/104tests passed in approximately8.6swith--forceExit.git diff --checkpassed.These results belong to lane D/PR #9, not this documentation-only PR.
Open gates
This staging PR is documentation-only. Its purpose is to make the complexity and deliverability gate reviewable. PR #9 is the current lane D implementation candidate. It must add only the repo-native tests and code required by the accepted specs, run the configured validation sequence, pass GitHub required checks, and receive maintainer review. The SRE/FMEA context does not require repository-level additions in this PR.