-
Notifications
You must be signed in to change notification settings - Fork 8.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HDFS-15079. RBF: Namenode needs to use the actual client Id and callId when going through RBF proxy. #4530
Conversation
…d when going through RBF proxy.
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@@ -46,6 +46,15 @@ | |||
public static final Logger LOG = LoggerFactory.getLogger( | |||
RetryInvocationHandler.class); | |||
|
|||
@VisibleForTesting | |||
public static final ThreadLocal<Boolean> SET_CALL_ID_FOR_TEST = | |||
new ThreadLocal<Boolean>() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no lambda or shorter version for this?
Path renameDst = new Path(testDir, "renameDst"); | ||
joeFS.mkdirs(renameSrc); | ||
|
||
Assert.assertEquals(HAServiceProtocol.HAServiceState.ACTIVE, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can statically import these asserts and just do assertEqual()
@@ -495,6 +498,72 @@ public static NameNodeMetrics getNameNodeMetrics() { | |||
return metrics; | |||
} | |||
|
|||
private static String clientInfoFromContext( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One line and javadoc.
return null; | ||
} | ||
|
||
private static String parseSpecialValue(String content, String key) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have unit tests for this?
...-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java
Show resolved
Hide resolved
...-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java
Show resolved
Hide resolved
@goiri Thanks for your review and I have updated the patch, please help me review it. |
🎊 +1 overall
This message was automatically generated. |
@goiri @ferhui @Hexiaoqiao @ayushtkn can you help me to review this patch? If you have any other ideas, please let me know. This is a very helpful issue for end users, so I want to push it forward. |
@@ -140,7 +140,7 @@ public long getExpirationTime() { | |||
|
|||
@Override | |||
public String toString() { | |||
return (new UUID(this.clientIdMsb, this.clientIdLsb)).toString() + ":" | |||
return (new UUID(this.clientIdMsb, this.clientIdLsb)) + ":" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe is cleaner to have this as String.format()
It would be nice to get feedback from others. |
🎊 +1 overall
This message was automatically generated. |
...-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNode.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good! Thanks!
…d when going through RBF proxy. (apache#4530)
Description of PR
Jira: HDFS-15079
Similarly with HDFS-13248, RBF adds the actual client Id and client call Id in CallerContext and carries them to NameNode. Then nameNode try to obtain the actual client Id and client call Id to ensure CacheEntry mechanism.