[NBS] Dont fire CleanupBlobMetaBlocksMismatch impossible event if leaked blocks has another blob id#6596
Conversation
|
Note This is an automated comment that will be appended during run. Note All workloads for linux-x86_64-relwithdebinfo have completed. Tip Planned checks for linux-x86_64-relwithdebinfo. 🟢 linux-x86_64-relwithdebinfo target: cloud/blockstore/ (test time: 11899s): all tests PASSED for commit dfb28ce.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dfb28ce217
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (blobId != OriginalBlobId) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Keep mismatched mixed-block keys out of original-meta cleanup
When this verifier is reached from cleanup, UseRecreatedBlobMeta is false because the recreated-meta path returns before verification, so PrepareCleanupTransaction still queues the original DB blob meta for deletion. In the duplicate-flush case this branch now treats a missing original block as safe when the same (blockIndex, commitId) key belongs to another blob, but ExecuteCleanupTransaction later deletes mixed blocks by (blockIndex, commitId) only via state.DeleteMixedBlock, so cleanup of the old blob will remove the other blob's live index entry. Please keep this as a mismatch or otherwise avoid deleting the original-meta block when the index row is owned by another blob.
Useful? React with 👍 / 👎.
Notes
We can flush the same block multiple times when restarting a tablet, so while checking blob metas, we will find a block with the same commitId, but it belongs to a different blob. Therefore, we should not fire an imspl event in this case.
Issue
#6279