HBASE-25292 [branch-1] Improve InetSocketAddress usage discipline #2671
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
apurtell
commented
Nov 17, 2020
| this.socket.bind(this.rpcClient.localAddr); | ||
| } | ||
| NetUtils.connect(this.socket, remoteId.getAddress(), this.rpcClient.connectTO); | ||
| if (this.rpcClient.metrics != null) { |
Contributor
Author
There was a problem hiding this comment.
This change fixes a bug we encountered in production while running in Amazon's Elastic Kubernetes Service.
apurtell
commented
Nov 17, 2020
| private void connect() throws UnknownHostException { | ||
| if (LOG.isDebugEnabled()) { | ||
| LOG.debug("Connecting to " + remoteId.address); | ||
| LOG.debug("Connecting to " + remoteId.getAddress()); |
Contributor
Author
There was a problem hiding this comment.
This is the netty version of the bug fix here: https://github.com/apache/hbase/pull/2671/files#diff-1a7ec27a8107293b6c87132823c262fc250570687a40f45646c43ae46dc6b04eR255
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
apurtell
force-pushed
the
HBASE-25292-branch-1
branch
from
7d95a26 to
d927435
Compare
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
Network identities should be bound late. Remote addresses should be resolved at the last possible moment, just before connect(). Network identity mappings can change, so our code should not inappropriately cache them. Otherwise we might miss a change and fail to operate normally. Revert "HBASE-14544 Allow HConnectionImpl to not refresh the dns on errors" Removes hbase.resolve.hostnames.on.failure and related code. We always resolve hostnames, as late as possible. Preserve InetSocketAddress caching per RPC connection. Avoids potential lookups per Call. Replace InetSocketAddress with Address where used as a map key. If we want to key by hostname and/or resolved address we should be explicit about it. Using Address chooses mapping by hostname and port only. Add metrics for potential nameservice resolution attempts, whenever an InetSocketAddress is instantiated for connect; and metrics for failed resolution, whenever InetSocketAddress#isUnresolved on the new instance is true.
apurtell
force-pushed
the
HBASE-25292-branch-1
branch
from
f23a7f2 to
56a4cd1
Compare
|
💔 -1 overall
This message was automatically generated. |
Contributor
Author
|
#2669 was merged, closing this and picking back the master commit. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
See description on #2669