You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2020/06/13 15:25:42 UTC
[GitHub] [hbase] virajjasani commented on a change in pull request #1337: HBASE-24038 Add a metric to show the locality of ssd in table.jsp
virajjasani commented on a change in pull request #1337:
URL: https://github.com/apache/hbase/pull/1337#discussion_r439746497
##########
File path: hbase-server/src/main/resources/hbase-webapps/master/table.jsp
##########
@@ -319,6 +318,52 @@ if (fqtn != null && master.isInitialized()) {
</tbody>
</table>
</div>
+ <div class="tab-pane" id="metaTab_localityStats">
+ <table id="tableRegionTable" class="tablesorter table table-striped">
+ <thead>
+ <tr>
+ <th>Name</th>
+ <th>Region Server</th>
+ <th>Locality</th>
+ <th>LocalityForSsd</th>
+ </tr>
+ </thead>
+ <tbody>
+ <%
+ // NOTE: Presumes meta with one or more replicas
+ for (int j = 0; j < numMetaReplicas; j++) {
+ RegionInfo meta = RegionReplicaUtil.getRegionInfoForReplica(
+ RegionInfoBuilder.FIRST_META_REGIONINFO, j);
+ ServerName metaLocation = MetaTableLocator.waitMetaRegionLocation(master.getZooKeeper(), j, 1);
+ for (int i = 0; i < 1; i++) {
+ String hostAndPort = "";
+ float locality = 0.0f;
+ float localityForSsd = 0.0f;
+
+ if (metaLocation != null) {
+ ServerMetrics sl = master.getServerManager().getLoad(metaLocation);
+ hostAndPort = URLEncoder.encode(metaLocation.getHostname()) + ":" + master.getRegionServerInfoPort(metaLocation);
+ if (sl != null) {
+ Map<byte[], RegionMetrics> map = sl.getRegionMetrics();
+ if (map.containsKey(meta.getRegionName())) {
+ RegionMetrics load = map.get(meta.getRegionName());
+ locality = load.getDataLocality();
+ localityForSsd = load.getDataLocalityForSsd();
+ }
+ }
+ }
+ %>
+ <tr>
+ <td><%= escapeXml(meta.getRegionNameAsString()) %></td>
Review comment:
Use `ESCAPE_XML.translate()`?
##########
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:
Can we break this if condition? How about this?
```
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);
}
}
```
##########
File path: hbase-server/src/main/resources/hbase-webapps/master/table.jsp
##########
@@ -910,6 +954,64 @@ if (fqtn != null && master.isInitialized()) {
here</a> to see all regions.</p>
<% } %>
</div>
+ <div class="tab-pane" id="tab_localityStats">
+ <table id="regionServerDetailsTable" class="tablesorter table table-striped">
+ <thead>
+ <tr>
+ <th>Name(<%= String.format("%,1d", regions.size())%>)</th>
+ <th>Region Server</th>
+ <th>Locality</th>
+ <th>LocalityForSsd</th>
+ </tr>
+ </thead>
+ <tbody>
+ <%
+ numRegionsRendered = 0;
+ for (Map.Entry<RegionInfo, RegionMetrics> hriEntry : entryList) {
+ RegionInfo regionInfo = hriEntry.getKey();
+ ServerName addr = regionsToServer.get(regionInfo);
+ RegionMetrics load = hriEntry.getValue();
+ String urlRegionServer = null;
+ float locality = 0.0f;
+ float localityForSsd = 0.0f;
+ String state = "N/A";
+ if (load != null) {
+ locality = load.getDataLocality();
+ localityForSsd = load.getDataLocalityForSsd();
+ }
+
+ if (addr != null) {
+ // This port might be wrong if RS actually ended up using something else.
+ urlRegionServer =
+ "//" + URLEncoder.encode(addr.getHostname()) + ":" + master.getRegionServerInfoPort(addr) + "/rs-status";
+ }
+
+ if (numRegionsRendered < numRegionsToRender) {
+ numRegionsRendered++;
+ %>
+ <tr>
+ <td><%= escapeXml(Bytes.toStringBinary(regionInfo.getRegionName())) %></td>
+ <%
+ if (urlRegionServer != null) {
+ %>
+ <td>
+ <a href="<%= urlRegionServer %>"><%= addr == null? "-": StringEscapeUtils.escapeHtml4(addr.getHostname().toString()) + ":" + master.getRegionServerInfoPort(addr) %></a>
Review comment:
toString() seems redundant
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/HDFSBlocksDistribution.java
##########
@@ -199,10 +225,23 @@ public long getUniqueBlocksTotalWeight() {
* @return the locality index of the given host
*/
public float getBlockLocalityIndex(String host) {
+ return getBlockLocalityIndex(host, false);
+ }
+ /**
Review comment:
Keep 1 line b/ 2 methods?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/HDFSBlocksDistribution.java
##########
@@ -199,10 +225,23 @@ public long getUniqueBlocksTotalWeight() {
* @return the locality index of the given host
*/
public float getBlockLocalityIndex(String host) {
+ return getBlockLocalityIndex(host, false);
+ }
+ /**
+ * return the locality index of a given host
+ * @param host the host name
+ * @param forSsd only calc ssd type
+ * @return the locality index of the given host
+ */
+ public float getBlockLocalityIndex(String host, boolean forSsd) {
Review comment:
As of now it seems fine, but do you think we might need more metrics similar to ssd? If so, this could be an enum input?
```
enum weightFor{ SSD, DEFAULT, ......(more params possible in future?) }
```
Thought?
Also, if there is no such chance for additional metrics, maybe we can name this method: `getBlockLocalityIndexWithSsd()` or something similar?
----------------------------------------------------------------
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:
users@infra.apache.org