Repository navigation
HDDS-16497. Support OM ThreadLocal context propagation for Ratis read - #11257
Conversation
|
Please take a look at intermittent test failure, only seen in this PR so far. |
|
@adoroszlai Thanks for checking this, I have attached the possible race on HDDS-16513 related to trash calling Should not be directly related to this patch. |
# Conflicts: # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerStateMachine.java # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java # hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis/TestOzoneManagerStateMachine.java
peterxcli
left a comment
There was a problem hiding this comment.
other lgtm, thanks for the update!
peterxcli
left a comment
There was a problem hiding this comment.
+1 again, thanks for the update!
|
Thanks @peterxcli for the review and @adoroszlai for helping with the merge conflicts. |
|
@ivandika3 , @peterxcli , thanks for working on this! (and sorry that I was not able to reviw.) |
|
@szetszwo Thanks for always thoughtfully reviewing the PRs. I imagine it's not easy (and time consuming) to review numerous PRs especially with recent increase in volume of contributions. |
What changes were proposed in this pull request?
While working on HDDS-15089, encountered an NPE issue that was caused by null
Server.getRemoteUser()inOzoneManager#getS3VolumeContext. This reason is that Ratis would callStateMachine#queryin a separate thread (underRaftServerImpl#clientExecutor). Therefore, theThreadLocalinOzoneManagerand theCurrCallwhich is used byServer.getRemoteUser()is not going to be propagated.This affects most read requests submitted to Ratis (executed in
OzoneManagerStateMachine#query) that require using ThreadLocal one way or another.We need a way to propagate the
Server#getCurrcallThreadLocalcontext to theStateMachine#query.Note this suggests that follower read feature is not production ready until this is resolved.
The main mechanism is the introduction of
OMRatisRequestContextwhich is a context mechanism to pass the OM ThreadLocal info OMRequest UserInfo and allows theOzoneManagerStateMachine#query(that is executed in a separate thread) to create am artificialServer.CallwithgetRemoteUserandgetHostInetAddressso thatServer.getRemoteUser()will not return null. Additionally, thisOmRatisRequestContextalso helps to handle propagation toOzoneManagerThreadLocallikestsTokenIdentifier.Note that OM might need to use the
RaftServer#readOnlyAsync(apache/ratis#1448) that might either be executed directly in caller thread (if we useDEFAULTReadOption) or executed in another thread (if we useLINEARIZABLEReadOption). Therefore,OMRatisRequestContextshould handle context propagation within a thread and across differentthreads (seepreviousCallandpreviousS3Context).I chose this approach since it does not need to change every OM read implementation. However, any suggestions to improve this design is welcome since this is a core OM logic.
This patch also includes some refactoring on
OMLockDetailsUtilandS3AuthenticationContextto reduce duplications.Saw projects like https://github.com/alibaba/transmittable-thread-local to pass the ThreadLocal context across threads, but the current solution should be enough for now. In the future, we should also think whether this ThreadLocal passing mechanisms should be a first-class feature for Ratis (or we can support something like ScopedValues in Ratis).
Generated by: GPT 5.6
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-16497
How was this patch tested?
UT and IT.
Clean CI: https://github.com/ivandika3/ozone/actions/runs/35331947876