backup: clean up BACKUP-LOCK on compaction failure

[shivamshaw23]

Fixes #172889

When a compaction job fails, the leftover BACKUP_LOCK file is now removed in OnFailOrCancel. Subsequent compactions will no longer be blocked by stale locks.

[blathers-crl]

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

Before a member of our team reviews your PR, I have some potential action items for you:

• Please ensure your git commit message contains a release note.
• When CI has completed, please ensure no errors have appeared.

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

[shivamshaw23]

I have read the CLA Document and I hereby sign the CLA

[cockroachlabs-cla-agent]

All committers have signed the CLA.

[dt]

unlike

I don't really follow the "unlike" framing here, since producing a backup from a compaction or from fresh KV export are both equally bound by the write-only/WORM policies enforced on the collection. Worth stating the actual reason cleanup is safe here directly instead.

[shivamshaw23]

Good point, thanks. I removed the comparison to regular backups and stated the reason directly: a failed compaction does not produce a valid backup at the destination, so the stale lock only blocks subsequent compactions. Updated in 98b5afc.

[dt]

@kev-cao is this the same lock you were thinking about deprecating?

[blathers-crl]

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

• Please ensure your git commit message contains a release note.
• When CI has completed, please ensure no errors have appeared.

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

[kev-cao]

We can use besteffort.Warning here

[kev-cao]

I don't think passing NULL for the start and end times works here, we expect decimal values. I'd recommend looking at the pkg/backup/testdata/backup-restore/rangekeys test to see how to set those times.

[blathers-crl]

Thank you for updating your pull request.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

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

[shivamshaw23]

Thanks for the review @kev-cao! I've updated the PR with the requested changes:

1. Switched to using \�esteffort.Warning\ for lock deletion.
2. Completely rewrote the datadriven test following the
   angekeys\ pattern to use \save-cluster-ts\ and properly format the decimal timestamps for the compaction start/end boundaries.
3. Added a release note to the commit message to satisfy the bot.

Ready for another look!

[kev-cao]

Unfortunately, this still did not pass the unit tests in CI. The new datadriven test failed with error:

    datadriven_test.go:548: 
        	Error Trace:	pkg/backup/datadriven_test.go:548
        	            				pkg/backup/datadriven_test.go:903
        	            				external/com_github_cockroachdb_datadriven/datadriven.go:120
        	            				external/com_github_cockroachdb_datadriven/datadriven.go:367
        	            				external/com_github_cockroachdb_datadriven/datadriven.go:373
        	            				external/com_github_cockroachdb_datadriven/datadriven.go:215
        	            				external/com_github_cockroachdb_datadriven/datadriven.go:188
        	            				external/com_github_cockroachdb_datadriven/datadriven.go:148
        	            				external/com_github_cockroachdb_datadriven/datadriven.go:119
        	            				pkg/backup/datadriven_test.go:517
        	            				bazel-out/k8-fastbuild/bin/pkg/backup/data_driven_generated_test.go:105
        	Error:      	Expected value not to be nil.
        	Test:       	TestDataDriven_compaction_failed_lock_cleanup
        	Messages:   	expected pause point error
    datadriven.go:343: 
        fatal during /mnt/engflow/worker/work/0/exec/bazel-out/k8-fastbuild/bin/pkg/backup/backup_test_/backup_test.runfiles/com_github_cockroachdb_cockroach/pkg/backup/testdata/backup-restore/compaction-failed-lock-cleanup:55:
        SELECT crdb_internal.backup_compaction(0, 'BACKUP INTO LATEST IN ''nodelocal://1/test-root/''', '/2026/07/31-185258.72', '1785523978726471839.0000000000'::DECIMAL, '1785523986667139219.0000000000'::DECIMAL);

[blathers-crl]

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

• We notice you have more than one commit in your PR. We try break logical changes into separate commits, but commits such as "fix typo" or "address review commits" should be squashed into one commit and pushed with --force
• When CI has completed, please ensure no errors have appeared.

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

[shivamshaw23]

Thanks for catching the CI failure. Root cause: the test used expect-pausepoint on the compact directive, which asserts that the SQL call returns a synchronous "pause point ... hit" error. But crdb_internal.backup_compaction starts an asynchronous job (via StartCompactionJob → CreateAdoptableJobWithTxn) and returns immediately with the job ID, so no synchronous pausepoint error ever surfaces — hence expected pause point error.

Fixed in 84864bb: start the compaction without expect-pausepoint and instead wait for the job to reach the paused state (job tag=comp1 wait-for-state=paused) before cancelling it. This matches the async pattern already used by the rangekeys test. The pausepoint (backup_compaction.after.details_has_checkpoint) still fires after the BACKUP-LOCK is written, so cancelling from the paused state exercises the OnFailOrCancel cleanup path as intended, and the follow-up compaction (comp2) verifies the lock no longer blocks it.