rbd: replace O(n) trash list scan with O(1) ID-based trash removal (backport #6472) - #6484
rbd: replace O(n) trash list scan with O(1) ID-based trash removal (backport #6472)#6484mergify[bot] wants to merge 4 commits into
Conversation
mergify
Bot
added
ok-to-test
|
/test ci/centos/k8s-e2e-external-storage/1.34 |
|
/test ci/centos/mini-e2e-helm/k8s-1.34 |
|
/test ci/centos/mini-e2e/k8s-1.34 |
|
/test ci/centos/upgrade-tests-cephfs |
|
/test ci/centos/k8s-e2e-external-storage/1.33 |
|
/test ci/centos/k8s-e2e-external-storage/1.35 |
|
/test ci/centos/upgrade-tests-rbd |
|
/test ci/centos/mini-e2e-helm/k8s-1.35 |
|
/test ci/centos/mini-e2e-helm/k8s-1.33 |
|
/test ci/centos/mini-e2e/k8s-1.35 |
|
/test ci/centos/mini-e2e/k8s-1.33 |
ceph-csi-bot
added
ci/in-progress/e2e
mergify
Bot
removed
the
ci/in-progress/e2e
|
/retest ci/centos/mini-e2e-helm/k8s-1.33 |
|
/retest ci/centos/mini-e2e-helm/k8s-1.35 |
|
/retest ci/centos/mini-e2e/k8s-1.33 |
|
/retest ci/centos/upgrade-tests-rbd |
|
/queue |
|
@Mergifyio rebase |
Store the parent image's ID (from GetParent().Image.ImageID) in a new ParentImageID field on rbdImage. This ID is valid even when the parent is in trash, enabling O(1) trash removal without scanning the entire trash list. Reset ParentImageID to empty when GetParent returns ErrNotFound (no parent or parent link severed by flatten). Assisted-by: Claude <noreply@anthropic.com> Signed-off-by: Rakshith R <rar@redhat.com> (cherry picked from commit 18bd880)
Make trashRemoveImage() treat ENOENT as success on both the ceph mgr (AddTrashRemove) and direct (TrashRemove) paths - the image was already purged from trash. The mgr path checks rados.ErrNotFound before isCephMgrSupported to avoid misclassifying ENOENT as a mgr failure. Add removeImageFromTrash() as an O(1) helper that removes a trashed image by its known ID. Assisted-by: Claude <noreply@anthropic.com> Signed-off-by: Rakshith R <rar@redhat.com> (cherry picked from commit 608265a)
In checkErrAndUndoReserve() and cleanUpImageAndSnapReservation(), use removeImageFromTrash() to remove the trashed image directly by its known ID from the OMAP journal instead of scanning the entire trash list with ensureImageCleanup(). The image ID is always available in OMAP: modern volumes store it at creation time, and legacy volumes get it backfilled by storeImageID() during the first (successful) generateVolumeFromVolumeID() call while the image is still alive. Also fix pre-existing ErrorLog format string mismatch in cleanUpImageAndSnapReservation (2 format verbs but 3 arguments). Assisted-by: Claude <noreply@anthropic.com> Signed-off-by: Rakshith R <rar@redhat.com> (cherry picked from commit 1529daf)
|
/test ci/centos/mini-e2e/k8s-1.34 |
|
/test ci/centos/k8s-e2e-external-storage/1.33 |
|
/test ci/centos/mini-e2e/k8s-1.35 |
|
/test ci/centos/mini-e2e-helm/k8s-1.33 |
|
/test ci/centos/mini-e2e/k8s-1.33 |
|
/test ci/centos/k8s-e2e-external-storage/1.33 |
|
/test ci/centos/k8s-e2e-external-storage/1.35 |
|
/test ci/centos/k8s-e2e-external-storage/1.34 |
|
/test ci/centos/mini-e2e-helm/k8s-1.33 |
|
/test ci/centos/upgrade-tests-cephfs |
|
/test ci/centos/mini-e2e-helm/k8s-1.34 |
|
/test ci/centos/mini-e2e-helm/k8s-1.35 |
|
/test ci/centos/mini-e2e/k8s-1.33 |
|
/test ci/centos/upgrade-tests-rbd |
|
/test ci/centos/mini-e2e/k8s-1.34 |
|
/test ci/centos/mini-e2e/k8s-1.35 |
|
/test ci/centos/upgrade-tests-cephfs |
|
/test ci/centos/k8s-e2e-external-storage/1.35 |
|
/test ci/centos/upgrade-tests-rbd |
|
/test ci/centos/mini-e2e-helm/k8s-1.35 |
|
/test ci/centos/k8s-e2e-external-storage/1.34 |
|
/test ci/centos/k8s-e2e-external-storage/1.33 |
|
/test ci/centos/mini-e2e/k8s-1.35 |
|
/test ci/centos/mini-e2e-helm/k8s-1.33 |
|
/test ci/centos/mini-e2e-helm/k8s-1.34 |
|
/test ci/centos/mini-e2e/k8s-1.33 |
|
/test ci/centos/mini-e2e/k8s-1.34 |
|
even as Draft the bots continue restarting CI jobs. Added |
Merge Protections🔴 1 of 2 protections blocking · waiting on ⛓️ dependency
🔴 ⛓️ Depends-On RequirementsWaiting for
This rule is failing.Requirement based on the presence of
Show 1 satisfied protection🟢 🚦 Auto-queueWhen all merge protections are satisfied, this pull request will be queued automatically. |
|
Once #6502 is merged, this can be scheduled for merging with |
4 participants
Describe what this PR does
Replace
ensureImageCleanup()- which scans the entire RBD trash listto find an image by name - with direct O(1) removal by image ID using
the new
removeImageFromTrash()helper.The image ID is always available: modern volumes store it in OMAP at
creation time, and legacy volumes get it backfilled by
storeImageID()during the first
generateVolumeFromVolumeID()call.Commit 1 -
rbd: capture parent image ID in getImageInfoAdd
ParentImageIDfield torbdImageand populate it ingetImageInfo()fromGetParent().Image.ImageID. Valid even when theparent is in trash.
Commit 2 -
rbd: make trash remove idempotent and add removeImageFromTrashAdd
removeImageFromTrash()as an O(1) helper that removes a trashedimage by its known ID. Make
trashRemoveImage()treat ENOENT as successon both the ceph mgr and direct paths (image already purged).
Commit 3 -
rbd: avoid trash ls in DeleteVolume/DeleteSnapshot retry pathsIn
checkErrAndUndoReserve()andcleanUpImageAndSnapReservation(),switch from
ensureImageCleanup()toremoveImageFromTrash(). Also fixa pre-existing
ErrorLogformat string mismatch (2 format verbs, 3 args).Commit 4 -
rbd: remove ensureImageCleanup and use ID-based trash removalIn
DeleteTempImage(), use the volume'sParentImageIDto identify atrashed temp clone instead of scanning the trash list. Remove
ensureImageCleanup()entirely - no callers remain.Is there anything that requires special attention
DeleteTempImage()relies on being called beforerbdVol.Delete()sothat
GetParent()data is still valid. This ordering already exists incleanupRBDImage()and is now documented with a comment.DeleteTempImage()(temp clone notin trash), it returns success - same behavior as the old code when the
trash scan found nothing.
The change is backward compatible. No API or OMAP schema changes.
Future concerns
GetTrashListis still used inwaitForCleanTrash()(e2e test helper).That is unrelated to production code paths.
Checklist:
guidelines in the developer
guide.
Request
notes
updated with breaking and/or notable changes for the next major release.
N/A - internal optimization, no user-facing changes.
N/A - no doc changes needed, internal refactor only.
N/A - functions require a live Ceph cluster (librbd calls), not unit-testable.
Existing e2e delete/snapshot tests exercise these code paths.
N/A - existing e2e tests for DeleteVolume, DeleteSnapshot, and volume
cloning already cover the trash cleanup paths exercised by this change.
Show available bot commands
These commands are normally not required, but in case of issues, leave any of
the following bot commands in an otherwise empty comment in this PR:
/retest ci/centos/<job-name>: retest the<job-name>after unrelatedfailure (please report the failure too!)
This is an automatic backport of pull request #6472 done by [Mergify](https://mergify.com).
Depends-on: #6502