HDFS-16684. Exclude the current JournalNode #4723
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Exclude bound local addresses, including the use of a wildcard address in the bound host configurations.
|
🎊 +1 overall
This message was automatically generated. |
saintstack
reviewed
Aug 18, 2022
Contributor
saintstack
left a comment
saintstack
left a comment
There was a problem hiding this comment.
LGTM... .would be good to clean up minor items in below if you can.
...ject/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/server/JournalNodeSyncer.java
Outdated
Show resolved
Hide resolved
| } | ||
| if (otherJNProxies.isEmpty()) { | ||
| // Check if any of there are any resolvable JournalNodes before starting the sync. | ||
| if (otherJNProxies.stream().filter(jnp -> !jnp.jnAddr.isUnresolved()).count() == 0) { |
Contributor
There was a problem hiding this comment.
We are waiting on them to resolve? If resolve fails, we make no progress ... This is better (and aligns w/ other checks done above in this method returning false...)
...ject/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/server/JournalNodeSyncer.java
Show resolved
Hide resolved
...ject/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/server/JournalNodeSyncer.java
Outdated
Show resolved
Hide resolved
...ct/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/qjournal/server/TestJournalNodeSync.java
Show resolved
Hide resolved
...ct/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/qjournal/server/TestJournalNodeSync.java
Show resolved
Hide resolved
added 2 commits
August 19, 2022 15:15
Update the comments.
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
Contributor
Author
|
@saintstack The changes were made in response to your questions, so I wouldn't classify them as draft. You also indirectly identified a scenario where the startup sequence was delayed unnecessarily, so I've gone ahead an incorporated the update. Let me know if you have any further questions. |
saintstack
approved these changes
Aug 22, 2022
HarshitGupta11
pushed a commit
to HarshitGupta11/hadoop
that referenced
this pull request
Nov 28, 2022
Exclude bound local addresses, including the use of a wildcard address in the bound host configurations. * Allow sync attempts with unresolved addresses * Update the comments. * Remove unused import Signed-off-by: stack <[email protected]>
aswinmprabhu
pushed a commit
to aswinmprabhu/hadoop
that referenced
this pull request
Jan 10, 2024
Exclude bound local addresses, including the use of a wildcard address in the bound host configurations. * Allow sync attempts with unresolved addresses * Update the comments. * Remove unused import Signed-off-by: stack <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of PR
The JournalNodeSyncer will include the local instance in syncing when using a bind host (e.g. 0.0.0.0). There is a mechanism that is supposed to exclude the local instance, but it doesn't recognize the meta-address as a local address.
Running with bind addresses set to 0.0.0.0, the JournalNodeSyncer will log attempts to sync with itself as part of the normal syncing rotation. For an HA configuration running 3 JournalNodes, the "other" list used by the JournalNodeSyncer will include 3 proxies.
Exclude bound local addresses, including the use of a wildcard address in the bound host configurations.
How was this patch tested?
An additional unit tests was added to verify the call getJournalAddrList dropped the current instance.
Integration tests were conducted in HA configuration in Kubernetes, running Java 11. After the patch, all JournalNodes stopped attempting to sync with themselves.
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?