Skip to content

feat(health): observe PostgreSQL scoring-job backlogs - #113

Closed
cursor[bot] wants to merge 29 commits into
mainfrom
cursor/bc-5e1c026b-5d16-4f55-9339-c80ff153be28-a416
Closed

cursor[bot] wants to merge 29 commits into
mainfrom
cursor/bc-5e1c026b-5d16-4f55-9339-c80ff153be28-a416

Conversation

@cursor

@cursor cursor Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Why

PR #82 classifies outbox, inbox-consumption, and data-rights backlogs, but operators still cannot see stalled scoring work. A queued, leased, retry-scheduled, or quarantined scoring job can block results while readiness still looks healthy.

This branch is #82 at c4a50e9 plus the scoring-job observation slice. Prefer this head over merging #82 alone if scoring-job readiness is in the same integration window. Draft #103 remains superseded.

TDD

RED: tests/postgres_scoring_job_backlog_health.rs failed to compile (E0432) because probe_postgres_scoring_job_backlog, classify_postgres_scoring_job_backlog, and ScoringJobBacklogPolicy did not exist.

GREEN: the product probe and classifier exist, apply_scoring_job_migration applies 0021 after the immutable 0002 table, a second apply is idempotent, and missing scoring_job_state fails closed.

What changed

  • observe queued, leased, and retry-scheduled scoring-job counts plus quarantined count;
  • measure active age from created_at so a later lease or retry cannot hide a long-waiting job;
  • classify only against caller-supplied policy, with no universal SLO defaults;
  • fail closed on missing/future observation time and non-positive stored created-at;
  • expose aggregate counts/timestamps only — never request, worker, result, or payload identities;
  • apply partial readiness indexes through the existing product apply_scoring_job_migration path.

Verification

  • empty, mixed-state, count-stalled, age-stalled, quarantine, future-evidence, invalid created-at, transaction success, and missing-relation database-failure cases;
  • index contract for active and quarantined partial indexes;
  • cargo fmt, cargo clippy --all-targets -- -D warnings, cargo check --all-targets --locked.

Out of scope

HTTP probes remain #91 / #102. No universal SLO defaults. Live fast-mlsirm execution remains outside this repository.

Open in Web View Automation 

seonghobae and others added 28 commits August 15, 2026 12:42
The first positive-millis conversion returned before Linux line coverage
could see the later consumption and propagation timestamp checks.
Linux line coverage treats the later conversion ? as its own statement.
Return those invalid stored times through an explicit match arm.
Client-only invalid timestamp probes left the GenericClient Transaction
instantiation of each independent oldest-event conversion uncovered.
Backlog probes accept GenericClient, so an aborted transaction and a
closed connection must both surface typed database errors on the
query Result paths.
#76 already landed, so keep Active PR #82 as the remaining backlog-observation
slice. Name the caller-policy probes in TRACEABILITY, OPERABILITY, and the
changelog. HTTP probes and measured deployment-profile thresholds stay outside
this branch.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
The inherited #72 recovery fixture inserted a processing consumption
row without claim_deadline_at. Migration 0019 requires that column for
processing rows, and the deadline trigger is UPDATE-only, so exact-head
CI failed closed. Seed a valid persisted claim and assert the deadline
survives COPY restore.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Migration 0019 requires claim_deadline_at for processing consumption rows,
and the deadline trigger is UPDATE-only. Direct INSERT fixtures used by the
integration-backlog probe must persist that column so exact-head CI can
observe in-flight work without weakening the fail-closed shape check.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
PostgreSQL rejected $5 as both bigint claim expiry and double-precision
interval input. Persist claim_deadline_at with clock_timestamp() so the
0019 shape check stays fail-closed without weakening the probe contract.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Migration 0020 only ran from a test include_str, so callers using the
product apply functions never received readiness indexes. Add
apply_backlog_health_index_migration and require the index contract to
use that path, including missing-relation and idempotent apply cases.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
The capability-health row still described #82 as observation-only.
Record apply_backlog_health_index_migration so TRACEABILITY matches the
reviewed product apply contract.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Operators could not classify queued, leased, retry-scheduled, or
quarantined scoring work with the same content-free policy used for
outbox and data-rights readiness. Add the probe, caller-supplied
classifier, and product apply path for partial scoring-job indexes.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
@seonghobae
seonghobae marked this pull request as ready for review August 16, 2026 15:25
@cursor

cursor Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor Author

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

A healthy outbox or data-rights queue must not hide a stalled or
unknown scoring-job observation. Classify the three families together
and keep stalled above unknown above within-bounds.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
@cursor
cursor Bot requested a review from seonghobae August 16, 2026 15:26

@cursor cursor Bot left a comment •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Stale comment

Review — 49dbb2b scoring-job backlog observation

No blocking defect on this head. Prefer landing #113 over merging #82 alone. Do not also merge #82 or draft #103 as siblings. HTTP probes stay on #111.

What is sound

  • Queued, leased, and retry-scheduled jobs are active; completed/cancelled are excluded; quarantine is a separate cap.
  • Age is measured from created_at, so a later lease or retry cannot hide wait time.
  • Classification is fail-closed: missing/future observation → Unknown; over-count, over-age, or quarantine → Stalled.
  • classify_postgres_operational_backlog now folds integration, data-rights, and scoring-job families so a healthy outbox cannot hide stalled scoring work.
  • Evidence is counts plus one timestamp. Display does not leak payloads, worker, request, or tenant identities.
  • SQL is static. Indexes apply through apply_backlog_health_index_migration / apply_scoring_job_migration, not a test-only include_str!.
  • Missing scoring_job_state fails closed as a database probe error.

Required before this is treated as shipped truth

  1. Independent last-push review (already requested: seonghobae). This comment is not an approval.
  2. Exact-head required checks on 49dbb2b.
  3. Name 0020 / 0021 in the TRACEABILITY migrations/ tree and add the Active-PR note to docs/architecture/AS_BUILT_SCHEMA.md (inline on the Active-PR paragraph).

Non-blocking

  • Round the EXTRACT(EPOCH) millisecond conversion (inline on the scoring-job probe).
  • Unknown scoring_state values are ignored until the CHECK widens; fail closed if that contract ever grows.

Merge action

Land this PR after independent approval and green exact-head checks. Then close or retarget #82 and #103. Do not merge #91/#102 for HTTP while #111 is the serve-loop successor.

Open in Web View Automation 

Sent by Cursor Automation: Fix Issues

Comment thread docs/TRACEABILITY.md
Comment thread src/postgres_health.rs
cursor Bot pushed a commit that referenced this pull request Aug 16, 2026
Renumber the identity-link migration so it does not collide with #113 scoring-job health indexes on 0021. Name Active PR #124 as the merge candidate over #114.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Review (49dbb2b) — COMMENT, not approval

This head is #82 plus scoring-job backlog observation. Prefer it over merging #82 alone. Do not merge until exact-head required checks and an independent last-push review land. This comment is not that approval.

What holds

  • apply_scoring_job_migration applies immutable 0002 then 0021 (CREATE INDEX IF NOT EXISTS). A second apply is idempotent. Missing scoring_job_state fails closed.
  • The probe returns aggregate counts and one oldest created_at only. No request, worker, result, tenant, or payload identities leave the query.
  • Classification is fail-closed on a missing observation time, future created_at, and non-positive stored created-at. No SLO defaults.
  • classify_postgres_operational_backlog keeps a stalled scoring queue visible when the outbox is healthy.
  • Tests cover empty, mixed-state, count/age/quarantine policy, future evidence, epoch-zero created-at, transaction success, missing-relation, and index-contract apply.

Residual — successor, not a merge blocker

Expired leased rows stay in active_job_count and in MIN(created_at) age. A dead worker whose lease is already past active_lease_expires_at_unix_ms can still classify WithinBounds when count and enqueue-age are inside policy. expire_scoring_lease already models that state; this probe does not. Keep this created-at slice as-is. The next head should expose expired-lease count against caller policy and name the residual in TRACEABILITY.

Coordination

Keep migrations/0021_scoring_job_health_indexes.sql on this head. Draft #114 (0021_participant_identity_link.sql) must renumber to 0022 before it leaves draft.

HTTP probes stay on #111. Draft #103 remains superseded.

Open in Web View Automation 

Sent by Cursor Automation: Fix Issues

Comment thread src/postgres_health.rs
let row = client.query_one(
"SELECT \
(SELECT COUNT(*)::BIGINT FROM scoring_job_state \
WHERE scoring_state IN ('queued', 'leased', 'retry_scheduled')), \

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Expired leased rows stay in this predicate. scoring_state = 'leased' does not consult active_lease_expires_at_unix_ms, so a dead worker whose lease is already past expiry still increments active_job_count and still participates in MIN(created_at).

That cannot hide a long-waiting job (created-at age still moves). It can hide a dead worker behind a healthy count/age when the operator bound is enqueue-to-complete and that bound is much larger than the lease length. expire_scoring_lease already knows this state; the probe does not.

Not a blocking defect in this created-at slice. The successor should expose expired-lease count (and optionally oldest expiry) against caller policy. Name the residual in TRACEABILITY so OPERABILITY “not silently stalled” / “lease backlog” is not read as expiry observation.

Comment thread src/postgres_health.rs
WHERE scoring_state IN ('queued', 'leased', 'retry_scheduled')), \
(SELECT COUNT(*)::BIGINT FROM scoring_job_state \
WHERE scoring_state = 'quarantined'), \
(SELECT (EXTRACT(EPOCH FROM MIN(created_at)) * 1000)::BIGINT \

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

EXTRACT(EPOCH FROM timestamptz) is float8, then * 1000, then ::BIGINT (truncate toward zero). At 2026-era timestamps the ULP after * 1000 is well under 1 ms, so this is not a practical fail-open versus the other families’ stored unix-ms BIGINTs.

Fail-closed behavior that does hold: empty MIN → NULL → no oldest timestamp; epoch 0 → InvalidStoredValue; pre-epoch → InvalidStoredValue; truncation makes age look slightly older, not younger.

Keep this conversion if 0002 stays TIMESTAMPTZ. Do not treat float precision as a merge blocker. If a later slice wants exact ms parity with outbox/data-rights, persist created_at_unix_ms BIGINT on scoring_job_state rather than tightening this cast.

assert_eq!(evidence.quarantined_job_count(), 1);
assert_eq!(evidence.oldest_active_job_at_unix_ms(), Some(2_000));
assert_eq!(
classify_postgres_scoring_job_backlog(&evidence, 5_000, &policy()),

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This assertion encodes the expired-lease residual. scoring_job_leased_alpha is created at 2500 ms with active_lease_expires_at_unix_ms = 3500 (see insert_job leased branch). Observation is 5000 ms, so the lease is already expired, and classification is still WithinBounds under a 4-count / 5000 ms age policy.

That is correct for the stated created-at contract. It is also proof that a dead-worker leased row can sit behind a healthy active count/age. Keep this test; do not “fix” it by excluding expired leases from active_job_count in this slice. A successor should add a distinct expired-lease evidence field and a test that max_expired_lease_count = 0 fails closed.

@@ -0,0 +1,8 @@
-- Keep scoring-job readiness probes bounded as terminal history grows.
CREATE INDEX IF NOT EXISTS scoring_job_state_active_health_idx

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Numeric prefix 0021 collides with draft #114 (migrations/0021_participant_identity_link.sql at 2eb0b63). Filenames differ, so directory-order recovery would apply both, but the repo’s migration numbering contract is one version per change.

This PR is ready and first; keep 0021_scoring_job_health_indexes.sql. #114 must renumber to 0022 before it leaves draft. Do not take identity-link tables here, and do not reuse 0021 for a second aggregate.

Copy link
Copy Markdown
Contributor

Closing as superseded by PR #131. #131 is this exact scoring-job backlog line plus the expired-lease/dead-worker evidence that prevents a stalled worker from remaining WithinBounds. Its current body explicitly says to prefer #131 over #113 in this integration window. Keep the single health landing on #131 (or a later successor); do not merge this predecessor separately.

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