Repository navigation
Allow editing a partition's range end - #602
Open
guodong-sq wants to merge 4 commits into
Open
guodong-sq wants to merge 4 commits into
guodong-sq wants to merge 4 commits into
Conversation
A partition's range end is fixed when the backfill is prepared. A run that turns out to cover far more of the key space than it needs has no way to stop early: the Edit Cursor form moves only the start cursor, and the range on the creation screen is applied to every partition identically, so it cannot express a per shard bound. This is the common shape for a `filterCondition` that matches a narrow slice of a large table. `computeOverallRange` derives the range from `getUnfilteredBoundaryKeyValue`, which ignores the condition, so the run walks every remaining key to the end of the table and emits empty batches the whole way. Add an optional `new_range_end` to the edit form and its handler. The handler now requires a new cursor or a new range end rather than a new cursor alone, and applies whichever is supplied. Nothing else needs to change. `DbRunPartition.pkey_range_end` is already a per partition column, and `BackfillRunner.loadMetaData` already reads it per partition, so a narrowed end reaches the client on the next lease. Both the runner and the precomputer stop when the client reports no batches past the end, so neither can run past a lowered bound.
There was a problem hiding this comment.
🟡 Changes recommended
Resolve concurrency races, invalidate stale precomputation state, and update the API dump.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds per-partition range-end editing to the paused backfill cursor workflow.
Changes:
- Supports optional
new_range_endupdates. - Updates the edit form and button text.
- Adds tests for range-end editing and concurrency behavior.
File summaries
| File | Reviewed changes |
|---|---|
service/src/test/kotlin/app/cash/backfila/actions/EditPartitionCursorActionTest.kt |
Tests range-end updates and stale snapshots. |
service/src/main/kotlin/app/cash/backfila/dashboard/EditPartitionCursorHandlerAction.kt |
Applies cursor and range-end changes; requires concurrency protection, precomputation invalidation, and API dump updates. |
service/src/main/kotlin/app/cash/backfila/dashboard/EditPartitionCursorAction.kt |
Adds the range-end form field; unchanged values can overwrite concurrent edits. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Validate both snapshots and require runners to release their leases before editing. Preserve untouched binary bounds, reject unbounded range edits, and reset counts only when the end changes. Cover paused-runner recovery and retain the cursor-only ABI. Co-authored-by: OpenAI Codex <codex@openai.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The overall progress label can exceed 100% after a range is shortened.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
Resolved since last review (4)
pkey_cursor, so two forms loaded with the same cursor can race…service/api/service.api…pkey_range_enddoes not invalidate precomputation. If this partition was already…
Wait for persisted precomputation before completing a partition, while allowing pause and shutdown to cancel the wait. Cap the overall progress label at 100% and cover delayed counting and cancellation. Co-authored-by: OpenAI Codex <codex@openai.com>
Leave the new cursor field blank and validate its encoding and snapshot only when explicitly changing it. Preserve untouched cursor bytes during end-only updates and cover form submission, stale bounds, and atomic rejection of binary cursor changes. Co-authored-by: OpenAI Codex <codex@openai.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
A filtered backfill can spend hours scanning beyond the last matching record. Creation accepts one key range for all partitions, while each shard may need a different stopping point. Operators need to shorten an existing partition's range without moving its cursor.
What
Add an optional range-end edit to the partition form. Leave either edit field blank to preserve its current value.
How
The handler checks the snapshot for each field being changed. End-only edits preserve the cursor bytes, including binary cursors. It requires a paused partition with no outstanding runner lease and rejects edits to unbounded or non-UTF-8 ends. This excludes S3 partitions, whose client does not support custom ends.
An actual end change clears completed or partial precomputation so the next runner counts the new range. Historical processed counts remain intact; progress displays stop at 100%. Runners wait for counting to finish before marking a partition complete. Pause and cancellation still stop waiting runners. The existing cursor-only JVM signatures remain available, and the API dump includes the new endpoint signature.
Risk
This changes the persisted scope of an explicitly edited partition. Operators must use the client's key format and end-bound convention. Recounting the new range adds read load. The completion wait applies to all runs. Slow or failing count requests can delay completion or pause a run after processing ends. If a crashed runner leaves a lease behind, resume to recover the lease, then pause again before editing.
No database or wire-format migration is required.
Testing
No manual UI testing. Local database-backed validation used an isolated schema through
BACKFILA_TEST_DATABASE=backfila_pr602_service_testto avoid an existing fixture-schema collision.Generated with Codex