apurtell commented on a change in pull request #2669:
URL: https://github.com/apache/hbase/pull/2669#discussion_r527011991
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
##########
@@ -3485,25 +3488,26 @@ boolean checkFileSystem() {
@Override
public void updateRegionFavoredNodesMapping(String encodedRegionName,
List<org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos.ServerName>
favoredNodes) {
- InetSocketAddress[] addr = new InetSocketAddress[favoredNodes.size()];
+ Address[] addr = new Address[favoredNodes.size()];
// Refer to the comment on the declaration of regionFavoredNodesMap on why
- // it is a map of region name to InetSocketAddress[]
+ // it is a map of region name to Address[]
for (int i = 0; i < favoredNodes.size(); i++) {
- addr[i] =
InetSocketAddress.createUnresolved(favoredNodes.get(i).getHostName(),
+ addr[i] = Address.fromParts(favoredNodes.get(i).getHostName(),
favoredNodes.get(i).getPort());
}
regionFavoredNodesMap.put(encodedRegionName, addr);
}
/**
* Return the favored nodes for a region given its encoded name. Look at the
- * comment around {@link #regionFavoredNodesMap} on why it is
InetSocketAddress[]
- *
+ * comment around {@link #regionFavoredNodesMap} on why we convert to
InetSocketAddress[]
+ * here.
+ * @param encodedRegionName
* @return array of favored locations
*/
@Override
public InetSocketAddress[] getFavoredNodesForRegion(String
encodedRegionName) {
- return regionFavoredNodesMap.get(encodedRegionName);
+ return
Address.toSocketAddress(regionFavoredNodesMap.get(encodedRegionName));
Review comment:
This will happen whenever the regionserver needs to build a list of ISA
of datanodes for handing to FileSystem#create, but only if Favored Nodes is
enabled. (As far as I know, nobody actually uses favored nodes, well, perhaps
Francis and ex-Yahoo team.) So yeah, just before store file creation time. We
can hand the ISA directly to HDFS without checking ISA#isUnresolved because
should an ISA be unresolved when HDFS tries to use it the Java network API will
throw an exception, which will propagate up to us. However if you would prefer
to check the resolution status of the ISAs and explicitly throw our own
exception, the place to do this would be FSUtils.java where the call to
FileSystem#create including ISA parameters is performed via reflection.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
