[
https://issues.apache.org/jira/browse/HDDS-6682?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17678835#comment-17678835
]
Ethan Rose commented on HDDS-6682:
----------------------------------
Hi [~pifta] Thanks for looking at this again. There's two problems that need to
be solved:
1. FSO request can end up being applied to a non-FSO bucket and vise-versa
2. Old client can incorrectly interact with a non-legacy bucket.
For 1, I agree we can just add a bucket layout check to each
validateAndUpdateCache. If we want to do this as it's own PR I'm +1 for that.
For 2, I am a bit concerned about the readability of putting the client version
checks in validateAndUpdateCache. It would split client validation between the
pre-processors and validateAndUpdateCache, making it harder to identify which
operations have client compatibility constraints from inspecting the code. I
think this points to a larger problem with the pre and post processors, where
they are not flexible enough to handle our client version validation needs so
we now need to use other alternatives as well.
I think the pre/post processors work well as request translation layers, but
actual blocking could be done similar to the layout version annotations. By
using AOP instead of In memory maps built from reflection, aspectj can weave
the bytecode in wherever we put the annotations, which gives us the flexibility
to do request blocking at the beginning of preexecute, validateAndUpdateCache,
or read methods. Of course the harder part there is getting the required
context into the advice.
> Validate Bucket ID of bucket associated with in-flight requests.
> ----------------------------------------------------------------
>
> Key: HDDS-6682
> URL: https://issues.apache.org/jira/browse/HDDS-6682
> Project: Apache Ozone
> Issue Type: Sub-task
> Reporter: Jyotinder Singh
> Assignee: Jyotinder Singh
> Priority: Major
> Labels: pull-request-available
>
> In high concurrency scenarios (which will become more common once we
> introduced prefix-based locking), there is a possibility of the following
> race condition:
> Take for instance the following scenario and 3 concurrent write requests:
> Bucket {{vol/buck1}} exists with {{LEGACY}} layout.
> {*}Request 1{*}: {{CreateKey}} by an older client (pre- bucket layout) on a
> bucket {{{}vol/buck1{}}}.
> {*}Request 2{*}: {{DeleteBucket}} by a new client on the bucket
> {{{}vol/buck1{}}}.
> {*}Request 3{*}: {{CreateBucket}} by a new client on the bucket {{vol/buck1}}
> with layout {{{}FILE_SYSTEM_OPTIMIZED{}}}.
> Let's say that these requests are processed in the following order:
> # {{Request 1}} is picked up by one of the threads, which proceeds to run
> the {{PRE_PROCESS}} validations on this request. The validator we are
> interested in is called {{{}blockCreateKeyWithBucketLayoutFromOldClient{}}}.
> This validator will make sure that the bucket associated with this request is
> a {{LEGACY}} bucket - which is the pre-defined behavior in the case of old
> client/new cluster interactions since we do not want an old client operating
> on buckets using a new metadata layout.
> One thing to know here is that at this stage, the OM does not hold a bucket
> lock (which only happens inside the {{updateAndValidateCache}} method
> associated with the write request's handler class).
> # While {{Request 1}} was being processed, another thread was processing
> {{{}Request 2{}}}. Let's say `Request2' managed to get hold of the bucket
> lock and successfully completed the bucket deletion.
> # Now before {{Request 1}} got a chance to acquire the bucket lock,
> {{Request 3}} manages to acquire it. It proceeds with the bucket creation and
> creates a new bucket {{vol/buck1}} with {{FILE_SYSTEM_OPTIMIZED}} bucket
> layout.
> # Finally, {{Request 1}} is able to acquire the bucket lock and proceeds to
> enter its {{validateAndUpdateCache}} method. However, even though it is able
> to find the bucket it is looking for, this is not the same bucket that was
> validated in its pre-processing hook. This new bucket has the same name, but
> a different bucket layout. The request ends up modifying a bucket that it
> should not be allowed to touch.
> This race condition can lead to undefined behavior of the Ozone cluster,
> where older clients might be modifying information they do not understand.
> This PR aims to add bucket ID validation to the request processing flow,
> which would make sure that the bucket that ends up being processed is the
> same one that was validated.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
