Skip to content

Conversation

@blathers-crl
Copy link

@blathers-crl blathers-crl bot commented Dec 19, 2025

Backport 1/1 commits from #159627 on behalf of @jeffswenson.
Backport 1/1 commits from #161475 on behalf of @ZhouXing19.


Currently, it is possible that canceling an import after the job marked the descriptor as online could cause the non-mvcc compliant rollback code to run on an online range. Note, this fix is still slightly incomplete. If there is a zombie import job revert logic, that zombie may revert changes its not supposed to, but the liveness subsystem makes zombie jobs unlikely.

Release note: Fixes an unlikely bug where import rollback could revert writes in an online descriptor. This is not a silent failure. It would only occur if the job entered a failed or cancelled state after the actual import succeeded and marked the descriptor as on line.
Fixes: #159603


Release justification: bug fix.

@blathers-crl blathers-crl bot requested a review from a team as a code owner December 19, 2025 16:45
@blathers-crl blathers-crl bot requested review from mw5h and removed request for a team December 19, 2025 16:45
@blathers-crl blathers-crl bot added blathers-backport This is a backport that Blathers created automatically. O-robot Originated from a bot. labels Dec 19, 2025
@blathers-crl
Copy link
Author

blathers-crl bot commented Dec 19, 2025

Thanks for opening a backport.

Before merging, please confirm that it falls into one of the following categories (select one):

  • Non-production code changes. Includes test-only changes, build system changes, etc.
  • Fixes for serious issues. Defined in the policy as correctness, stability, or security issues, data corruption/loss, significant performance regressions, breaking working and widely used functionality, or an inability to detect and debug production issues.
  • Other approved changes. These changes must be gated behind a disabled-by-default feature flag unless there is a strong justification not to.

Add a brief release justification to the PR description explaining your selection.

Also, confirm that the change does not break backward compatibility and complies with all aspects of the backport policy.

All backports must be reviewed by the TL and EM for the owning area.

@blathers-crl blathers-crl bot added the backport Label PR's that are backports to older release branches label Dec 19, 2025
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@blathers-crl
Copy link
Author

blathers-crl bot commented Dec 19, 2025

❌ PR #159900 does not comply with backport policy

Confidence: high
Explanation: The pull request modifies production code in 'pkg/sql/importer/import_job.go' by changing the handling logic for import job cancellations. Specifically, it prevents reverting data if a cancel is called after an import job has been successfully completed and the table descriptor has gone online. This change addresses a significant issue where rollback could incorrectly affect online data. Although this qualifies as fixing a stability or data corruption issue (meeting the critical bug criteria), there is no evidence provided in the PR that the changes are guarded by a feature flag. Furthermore, the release justification only mentions this as a 'bug fix' without qualifying it as addressing a critical bug.
Recommendation: Gate the PR behind a feature flag or provide additional justification that explains why this fix qualifies as a critical bug and can be safely backported without a feature flag

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

Copy link
Collaborator

@michae2 michae2 left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions
Copy link

github-actions bot commented Jan 5, 2026

Reminder: it has been 2 weeks please merge or close your backport!

@yuzefovich yuzefovich force-pushed the blathers/backport-release-24.3-159627 branch from b403073 to 5cb884b Compare January 21, 2026 06:11
jeffswenson and others added 2 commits January 20, 2026 22:14
Currently, it is possible that canceling an import after the job marked
the descriptor as online could cause the non-mvcc compliant rollback
code to run on an online range. Note, this fix is still slightly
incomplete. If there is a zombie import job revert logic, that zombie
may revert changes its not supposed to, but the liveness subsystem makes
zombie jobs unlikely.

Release note: Fixes an unlikely bug where import rollback could revert
writes in an online descriptor. This is not a silent failure. It would
only occur if the job entered a failed or cancelled state after the
actual import succeeded and marked the descriptor as on line.
Fixes: #159603
…IntoCSVCancel

Fixes #161045

Following #159627, data rollback during import cancellation only occurs
if the table descriptor has not yet been published (set back online).
This introduced a race condition in TestImportIntoCSVCancel -- the
final row count was non-deterministic depending on whether cancellation
finished before or after the descriptor update.

This change stabilizes the test by ensuring cancellation is triggered
before the descriptor is brought online, guaranteeing that ingested
data is always reverted.

Release note: None
@yuzefovich yuzefovich force-pushed the blathers/backport-release-24.3-159627 branch from 5cb884b to 764ed02 Compare January 21, 2026 06:15
@yuzefovich yuzefovich merged commit 6bf54b9 into release-24.3 Jan 21, 2026
19 of 20 checks passed
@yuzefovich yuzefovich deleted the blathers/backport-release-24.3-159627 branch January 21, 2026 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport Label PR's that are backports to older release branches blathers-backport This is a backport that Blathers created automatically. O-robot Originated from a bot. target-release-24.3.27

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants