Fix: Improve transactional safety of payload offload reads and writes#3814
Merged
Fix: Improve transactional safety of payload offload reads and writes#3814
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to improve transactional safety during payload cutover/offload batches by opening a single transaction earlier and reusing it through window-size optimization, range chunk creation, payload reads, and the final temp-table insert + lease update.
Changes:
- Start a DB transaction at the beginning of each cutover batch iteration (payloadstore + OLAP).
- Thread the transaction through the window-size optimization and payload range chunk creation queries.
- Switch paginated payload reads to use the same transaction connection instead of the pool.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pkg/repository/payloadstore.go | Opens a long-running tx for ProcessPayloadCutoverBatch and passes it into sizing/chunking/listing queries. |
| pkg/repository/olap.go | Same pattern as payloadstore for OLAP payload cutover batches. |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
pkg/repository/payloadstore.go:662
- This function now uses two separate transactions but reuses the same
tx/commit/rollbackvariables and stacks multipledefer rollback()calls. It works, but it’s easy to misread/modify incorrectly later. Consider scoping each transaction in its own block and/or using distinct variable names so it’s obvious which rollback/commit corresponds to which transaction.
tx, commit, rollback, err = sqlchelpers.PrepareTx(ctx, p.pool, p.l)
if err != nil {
return nil, fmt.Errorf("failed to prepare transaction for copying offloaded payloads: %w", err)
}
defer rollback()
pkg/repository/olap.go:3130
- Similar to payloadstore, this function now uses multiple transactions but reuses
tx/commit/rollbackand defers multiple rollbacks in the same scope. To reduce the chance of future mistakes, consider scoping each transaction in its own block or using distinct variable names for each transaction.
tx, commit, rollback, err = sqlchelpers.PrepareTx(ctx, p.pool, p.l)
if err != nil {
return nil, fmt.Errorf("failed to prepare transaction for copying offloaded payloads: %w", err)
}
defer rollback()
grutt
approved these changes
May 5, 2026
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.
Description
Makes the payload offload job a little bit more transactionally safe by listing doing the original payload listing + the writes in separate txs
Type of change