[
https://issues.apache.org/jira/browse/HDDS-17246?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Priyesh Karatha resolved HDDS-17246.
------------------------------------
Resolution: Won't Fix
Closing this PR and Jira as Won't Fix.
As per discussion in the PR, The get-and-set approach reduces the chance of
this race condition but cannot completely eliminate it without carrying the
bucket ID in CreateDirectoryRequest.
In this case, even if a Trash directory gets created in the newly recreated
bucket, there is no significant impact. The subsequent key move will fail
anyway, since the bucket was deleted only when it was empty.
> 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
> Assignee: Priyesh Karatha
> Priority: Minor
> Labels: pull-request-available
> 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
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
