[
https://issues.apache.org/jira/browse/HDDS-17246?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17246:
------------------------------
Attachment: CR-10-skip-trash-dir-creation-in-recreated-bucket.patch
> Lifecycle task can create the previous owner's .Trash directory in a bucket
> that was deleted and recreated while the task ran
> -----------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17246
> URL: https://issues.apache.org/jira/browse/HDDS-17246
> Project: Apache Ozone
> Issue Type: Bug
> Affects Versions: 2.3.0
> Reporter: Siyao Meng
> Priority: Minor
> Attachments: CR-10-skip-trash-dir-creation-in-recreated-bucket.patch,
> TestBugCR10TrashInRecreatedBucket.java
>
>
> h3. Mechanism
> {{KeyLifecycleService.LifecycleActionTask.call}} compares the bucket object
> ID with the lifecycle configuration once, at task start, and then uses the
> cached {{OmBucketInfo}} for the whole scan. When expired keys go to trash,
> {{moveToTrash}} calls {{checkAndCreateTrashDirIfNeeded}}, which looks the
> bucket up by volume and bucket name and, if {{.Trash/<owner>/Current}} does
> not exist, submits a recursive {{CreateDirectory}} as the cached owner.
> Nothing checks that the bucket is still the one the task started on. The
> renames and deletes the task sends afterwards carry the update ID of each
> scanned key, so they fail in a recreated bucket (on LEGACY buckets since
> HDDS-16993). The trash directories are the only write with an effect.
> h3. Trigger
> # An FSO or LEGACY bucket has an expiration rule, lifecycle move to trash is
> on ({{ozone.lifecycle.service.move.to.trash.enabled}}, default true) and the
> OM trash interval is above 0.
> # A lifecycle task passes its start check and collects expired keys.
> # Before the task handles that batch, the keys are deleted, the bucket is
> deleted and a bucket with the same name is created.
> # The task creates {{.Trash/<old owner>/Current}} in the new bucket. Its
> renames then fail with KEY_NOT_FOUND.
> Batches are handled while the scan is still running, so the window grows with
> the size of the bucket. I reproduced it with the service's existing pause
> hook and did not try to hit the window without one.
> h3. Impact
> * The recreated bucket has no lifecycle configuration and was never written,
> but it now holds {{.Trash/<old owner>/Current}} with an ACL for the old
> owner, and {{DeleteBucket}} on it fails with BUCKET_NOT_EMPTY.
> * The directories stay until someone deletes them by hand. The new bucket has
> no lifecycle rule, and from reading {{TrashPolicyOzone}} the trash emptier
> only rolls over and deletes checkpoints under {{.Trash/<user>}}, it never
> removes {{.Trash/<user>}} itself (argued, not run).
> * With native ACLs the {{CreateDirectory}} runs as the old owner and is
> refused unless the old owner can already create keys in the new bucket (WRITE
> on the bucket, or admin or volume owner), checked against
> {{OzoneNativeAuthorizer}}, not run end to end.
> h3. Reproduction
> PASS, deterministic, unmodified source.
> [^TestBugCR10TrashInRecreatedBucket.java] ({{hadoop-ozone/ozone-manager}})
> runs one OM with its real {{KeyLifecycleService}}. The bucket, the key, the
> lifecycle configuration and the bucket delete and create all go through
> {{OzoneManagerProtocol}}. The task is held with the service's existing fault
> injector just before it sends the delete for the expired key, and move to
> trash is switched on during that pause (the task reads the flag per batch),
> so the next batch takes the trash path. A passing test means the defect is
> present. Output on the unfixed code:
> {noformat}
> bob's bucket contains .Trash/alice/Current, acls=[user:alice:a[ACCESS]]
> bob deletes his empty bucket: BUCKET_NOT_EMPTY Bucket is not empty
> {noformat}
> h3. Suggested fix
> The patch below re-reads the bucket on the submitting side, which narrows the
> window to the time between that read and the apply of the
> {{CreateDirectory}}. A possible follow up closes it completely: an optional
> expected bucket object ID on the {{CreateDirectory}} request, set only by the
> lifecycle task and checked at apply under the bucket lock in
> {{OMDirectoryCreateRequest}} and {{OMDirectoryCreateRequestWithFSO}}. That is
> a request change. It needs no new layout feature if it lands before the
> lifecycle feature is released (no lifecycle configuration can be set before
> {{S3_LIFECYCLE_SUPPORT}} is finalized), but it touches the
> {{CreateDirectory}} request on both layouts.
> h3. Patch
> [^CR-10-skip-trash-dir-creation-in-recreated-bucket.patch], against
> 20d95ed37fe35013b5320713665db03c06ee36fa. It also applies to master at
> 64d4a791502.
> Before it creates a missing trash directory,
> {{checkAndCreateTrashDirIfNeeded}} reads the bucket by name and refuses if it
> is gone or has a different object ID. The task then ends the same way as when
> the trash root cannot be prepared (HDDS-16162): the error is logged and no
> directory is created. Nothing changes for a bucket that is not recreated.
> Covered by {{testMoveToTrashSkipsRecreatedBucket}} in the existing
> {{TestKeyLifecycleService.Normal}}. Without the change it fails for both
> parameterizations because the trash directory exists. With it all of
> {{TestKeyLifecycleService}} (726 tests, 18 skipped) passes and checkstyle is
> clean. With the patch the reproduction fails, because
> {{.Trash/alice/Current}} is not created.
> Found by code review of the OM volume, bucket and snapshot lifecycle paths,
> as part of the TLA+ verification effort under HDDS-15926, on commit
> 20d95ed37fe35013b5320713665db03c06ee36fa. Checked against HDDS issues and
> apache/ozone pull requests for duplicates before filing. The attached patch
> is a proposal for review. Generated with Specula (Claude Opus 5.5).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
