virajjasani commented on a change in pull request #1337:
URL: https://github.com/apache/hbase/pull/1337#discussion_r439798495
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/HDFSBlocksDistribution.java
##########
@@ -122,14 +134,27 @@ public synchronized String toString() {
* @param weight the weight
*/
public void addHostsAndBlockWeight(String[] hosts, long weight) {
+ addHostsAndBlockWeight(hosts, weight, null);
+ }
+ /**
+ * add some weight to a list of hosts, update the value of unique block
weight
+ * @param hosts the list of the host
+ * @param weight the weight
+ */
+ public void addHostsAndBlockWeight(String[] hosts, long weight,
StorageType[] storageTypes) {
if (hosts == null || hosts.length == 0) {
// erroneous data
return;
}
addUniqueWeight(weight);
- for (String hostname : hosts) {
- addHostAndBlockWeight(hostname, weight);
+ for (int i = 0; i < hosts.length; i++) {
+ long weightForSsd = 0;
+ if (storageTypes != null && storageTypes.length == hosts.length
+ && storageTypes[i] == StorageType.SSD) {
+ weightForSsd = weight;
Review comment:
Thanks @bsglz , I got your point. So, maybe this one is better?
```
if (storageTypes != null && storageTypes.length == hosts.length){
for (int i = 0; i < hosts.length; i++) {
long weightForSsd = 0;
if (storageTypes[i] == StorageType.SSD){
weightForSsd = weight;
}
addHostAndBlockWeight(hosts[i], weight, weightForSsd);
}
} else {
for (String hostname : hosts) {
addHostAndBlockWeight(hostname, weight, 0);
}
}
```
The point is for each host iteration, checking `storageTypes != null &&
storageTypes.length == hosts.length` seems redundant. What do you think?
----------------------------------------------------------------
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]
