ndimiduk commented on a change in pull request #1485:
URL: https://github.com/apache/hbase/pull/1485#discussion_r420238552
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/webapp/RegionReplicaInfo.java
##########
@@ -17,14 +17,22 @@
*/
package org.apache.hadoop.hbase.master.webapp;
+import static
org.apache.hbase.thirdparty.org.apache.commons.collections4.ListUtils.emptyIfNull;
Review comment:
A few unused imports to cleanup.
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
##########
@@ -26,6 +26,7 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
+import java.util.HashMap;
Review comment:
unused import.
##########
File path: hbase-server/src/main/resources/hbase-webapps/master/table.jsp
##########
@@ -387,68 +388,93 @@ if (fqtn != null && master.isInitialized()) {
}
}
%>
-<table class="table table-striped">
- <tr>
- <th>RegionName</th>
- <th>Start Key</th>
- <th>End Key</th>
- <th>Replica ID</th>
- <th>RegionState</th>
- <th>ServerName</th>
- </tr>
-<%
- final boolean metaScanHasMore;
- byte[] lastRow = null;
- try (final MetaBrowser.Results results = metaBrowser.getResults()) {
- for (final RegionReplicaInfo regionReplicaInfo : results) {
- lastRow = Optional.ofNullable(regionReplicaInfo)
- .map(RegionReplicaInfo::getRow)
- .orElse(null);
- if (regionReplicaInfo == null) {
-%>
- <tr>
- <td colspan="6">Null result</td>
- </tr>
-<%
- continue;
- }
+<div style="overflow-x: auto">
+ <table class="table table-striped nowrap">
+ <tr>
+ <th title="Region name">RegionName</th>
+ <th title="The startKey of this region">Start Key</th>
+ <th title="The endKey of this region">End Key</th>
+ <th title="Region replica id">Replica ID</th>
+ <th title="State of the region while undergoing
transitions">RegionState</th>
+ <th title="Server hosting this region replica, stored in info:server
column">Server</th>
Review comment:
It's a bit tedious, but how about we build all these labels from the
values in `HConstants`? That way this page is captured by any future
refactoring that happens down the line. For instance, the bit in this tool tip
that says "info:server" would be replaced with `+ HConstants.CATALOG_FAMILY_STR
+ ":" + HConstants.SERVER_QUALIFIER_STR`.
Kind of fiddly. I'm not sure if it's worth it. Just a suggestion.
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
##########
@@ -966,6 +965,33 @@ public static ServerName getServerName(final Result r,
final int replicaId) {
}
}
+ /**
+ * Returns the {@link ServerName} from catalog table {@link Result} where
the region is
+ * transitioning on. It should be the same as {@link
MetaTableAccessor#getServerName(Result,int)}
+ * if the server is at OPEN state.
+ *
+ * @param r Result to pull the transitioning server name from
+ * @return A ServerName instance or {@link
MetaTableAccessor#getServerName(Result,int)}
+ * if necessary fields not found or empty.
+ */
+ @Nullable
+ public static ServerName getTargetServerName(final Result r, final int
replicaId) {
Review comment:
Long-stale conversation, perhaps...
I agree with @liuml07 on this one -- it's a good thing to have a centralized
place for encapsulating the serialization details of storing and retrieving
content from meta. `MetaTableAccessor` seems like the place for it.
##########
File path: hbase-server/src/main/resources/hbase-webapps/master/table.jsp
##########
@@ -387,68 +388,93 @@ if (fqtn != null && master.isInitialized()) {
}
}
%>
-<table class="table table-striped">
- <tr>
- <th>RegionName</th>
- <th>Start Key</th>
- <th>End Key</th>
- <th>Replica ID</th>
- <th>RegionState</th>
- <th>ServerName</th>
- </tr>
-<%
- final boolean metaScanHasMore;
- byte[] lastRow = null;
- try (final MetaBrowser.Results results = metaBrowser.getResults()) {
- for (final RegionReplicaInfo regionReplicaInfo : results) {
- lastRow = Optional.ofNullable(regionReplicaInfo)
- .map(RegionReplicaInfo::getRow)
- .orElse(null);
- if (regionReplicaInfo == null) {
-%>
- <tr>
- <td colspan="6">Null result</td>
- </tr>
-<%
- continue;
- }
+<div style="overflow-x: auto">
+ <table class="table table-striped nowrap">
+ <tr>
+ <th title="Region name">RegionName</th>
+ <th title="The startKey of this region">Start Key</th>
+ <th title="The endKey of this region">End Key</th>
+ <th title="Region replica id">Replica ID</th>
+ <th title="State of the region while undergoing
transitions">RegionState</th>
+ <th title="Server hosting this region replica, stored in info:server
column">Server</th>
+ <th title="The seqNum for the region at the time the server opened this
region replica">Sequence Number</th>
+ <th title="Server where the region is transitioning on, stored in
info:sn column">Target Server</th>
Review comment:
s/Server where the region is transitioning on/The server to which the
region is transiting/
##########
File path: hbase-server/src/main/resources/hbase-webapps/static/css/hbase.css
##########
@@ -54,3 +54,7 @@ table.tablesorter thead tr .headerSortUp {
table.tablesorter thead tr .headerSortDown {
background-image: url(desc.gif);
}
+
+table.nowrap th, td {
Review comment:
Do you intend to change the style of all `td` elements, or only those
under `table.nowrap`? If the latter, I think you need `table.nowrap th,
table.nowrap td`.
----------------------------------------------------------------
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]
