[worker] Reconcile downstream processing (3/7) - #1867
Conversation
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
|
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. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
Next review available in: 46 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughChangesProcessing lifecycle
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CDCHealth as check-clickhouse-cdc
participant Reconciler as reconcilePendingProcessingOperations
participant Postgres
participant ClickHouse
participant EventStore as Processing event store
CDCHealth->>Reconciler: reconcile pending operations
Reconciler->>Postgres: read expected processing evidence
Reconciler->>ClickHouse: read markers and acknowledgements
Reconciler->>EventStore: persist CDC and analytics stages
Reconciler->>Postgres: complete reconciled outbox entries
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
Thanks 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 |
|
Storybook previews for This comment updates automatically on each PR push. |
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Asherlc
changed the title
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
…conciliation-v1 # Conflicts: # src/db/clickhouse-cdc.test.ts # src/db/clickhouse-cdc.ts # src/jobs/process-import-job.test.ts # src/jobs/process-sync-job.test.ts # src/jobs/process-sync-job.ts # src/processing/metric-stream-processing-publisher.test.ts # src/processing/metric-stream-processing-publisher.ts
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
|
@greptile review |
Greptile SummaryThis PR adds reconciliation for downstream processing. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Pending processing operation] --> B{CDC output path}
B -->|Relational| C[Validate marker and source watermark]
B -->|Metric stream| D[Validate batch acknowledgement]
C --> E{Evidence complete?}
D --> E
E -->|No| F[Keep CDC running]
E -->|Yes| G[Record CDC success]
G --> H[Queue dataset analytics]
H --> I[Run analytics build]
I --> J[Warm query caches]
J --> K[Record cache-refresh outcome]
Reviews (4): Last reviewed commit: "fix: clarify reconciliation review behav..." | Re-trigger Greptile |
Greptile SummaryThis PR adds reconciliation for downstream processing. The main changes are:
Confidence Score: 4/5CDC and cache reconciliation can publish incorrect downstream state, so these paths need fixes before merging.
src/processing/processing-reconciler.ts and src/processing/cache-processing.ts Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
P[Pending operation] --> R[CDC reconciliation]
C[(ClickHouse evidence)] --> R
R -->|Evidence complete| A[Queue analytics]
A --> B[Run analytics build]
B --> W[Warm query caches]
W --> S[Record cache status]
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
src/processing/processing-reconciler.ts:162
**Source Watermark Is Not Verified**
A ClickHouse marker with the expected operation, dataset, flow, and batch key is accepted even when its `source_watermark` differs from the Postgres expectation. A replayed or inconsistent marker can therefore queue analytics and publish the expected watermark as serving evidence before that exact source position is available.
### Issue 2 of 2
src/processing/cache-processing.ts:45-48
**Shared Families Cross Dataset Boundaries**
Cache outcomes are assigned by query-family name, but `providerDetail` belongs to both the activity and providers contracts. When that query fails for a user with both datasets pending, this code records `cache_refresh_failed` for both operations even if only one dataset owns the affected refresh.
Reviews (2): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/clickhouse-metric-stream.md`:
- Around line 139-150: The documentation uses incorrect “casual fence”
terminology and leaves the new acknowledgement and mirror-behavior claims
uncited. Update the relevant text in clickhouse-metric-stream.md to use “causal
fence,” and add links to the defining migration, reconciliation code, or other
official primary sources for ingest.metric_stream_processing_acknowledgement and
postgres_fitness behavior.
In `@entrypoint.sh`:
- Line 27: Update the analytics build command in entrypoint.sh to invoke
scripts/run-analytics-build.ts through pnpm tsx instead of the local Node shim,
preserving the existing command chaining behavior.
In `@scripts/check-clickhouse-cdc.test.ts`:
- Around line 205-208: Strengthen the test around
mockedReconcilePendingProcessingOperations by asserting the exact configured
ClickHouse client and database mock instances instead of expect.any(Object).
Retain the mock return values and add an invocation-order assertion proving
reconciliation occurs after assertClickHouseCdcHealth.
In `@scripts/check-clickhouse-cdc.ts`:
- Around line 126-128: Update the reconciliation status log near
reconcilePendingProcessingOperations to include reconciliation.checked and
clearly identify completed and waiting as counts from the current batch,
preventing them from being interpreted as the total backlog.
In `@src/processing/processing-reconciler.test.ts`:
- Around line 255-258: Reorder the mocked outputs in the processing reconciler
test to match the production ORDER BY result, placing the metric_stream entry
before the relational entry. Keep the existing idempotencyKey assertion aligned
with the resulting reconcileProcessingEvidence order and production-generated
key.
In `@src/processing/processing-reconciler.ts`:
- Around line 359-369: Update the pendingOperations claim query in the
processing reconciler to use PostgreSQL row-level locking with FOR UPDATE SKIP
LOCKED, preserving the existing filtering, grouping, ordering, and limit
behavior so concurrent runs claim disjoint pending operations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c8255997-b326-4c73-95d5-cd0f53e04eb9
📒 Files selected for processing (14)
docs/clickhouse-cdc-health-runbook.mddocs/clickhouse-metric-stream.mdentrypoint.shpackages/server/src/lib/cache-module.test.tspackages/server/src/lib/cache-redis.test.tsscripts/check-clickhouse-cdc.test.tsscripts/check-clickhouse-cdc.tsscripts/warm-query-cache.test.tsscripts/warm-query-cache.tssrc/processing/cache-processing.test.tssrc/processing/cache-processing.tssrc/processing/processing-reconciler.integration.test.tssrc/processing/processing-reconciler.test.tssrc/processing/processing-reconciler.ts
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
Part 3 of the #1852 processing-status stack.
Adds CDC, analytics, and cache-refresh reconciliation, health checks, cache warming integration, and operational documentation for downstream processing.
Previous: #1866. Next: #1868.
Changed files: 14.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests