ndimiduk commented on code in PR #7563:
URL: https://github.com/apache/hbase/pull/7563#discussion_r2631140628
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java:
##########
@@ -1077,6 +1077,47 @@ default Future<Void> modifyTableAsync(TableDescriptor
td) throws IOException {
*/
Future<Void> modifyTableAsync(TableDescriptor td, boolean reopenRegions)
throws IOException;
+ /**
+ * Reopen all regions of a table. This is useful after calling
+ * {@link #modifyTableAsync(TableDescriptor, boolean)} with
reopenRegions=false to gradually roll
+ * out table descriptor changes to regions. Regions are reopened in-place
(no move).
+ * @param tableName table whose regions to reopen
+ * @throws IOException if a remote or network exception occurs
+ */
+ default void reopenTableRegions(TableName tableName) throws IOException {
+ get(reopenTableRegionsAsync(tableName), getSyncWaitTimeout(),
TimeUnit.MILLISECONDS);
+ }
+
+ /**
+ * Reopen specific regions of a table. Useful for canary testing table
descriptor changes on a
+ * subset of regions before rolling out to the entire table.
+ * @param tableName table whose regions to reopen
+ * @param regions specific regions to reopen
+ * @throws IOException if a remote or network exception occurs
+ */
+ default void reopenTableRegions(TableName tableName, List<RegionInfo>
regions)
Review Comment:
We don't return a list of results here? Or at least PIDs for each region
touched? I'm curious how we expect an admin to observe the outcome of such an
API call.
I see that you're following existing style, so I guess better to maintain
consistency. From that perspective, I have no objections with continuing as is.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -116,12 +136,12 @@ public ReopenTableRegionsProcedure(final TableName
tableName, final List<byte[]>
PROGRESSIVE_BATCH_SIZE_MAX_DISABLED);
}
- ReopenTableRegionsProcedure(final TableName tableName, long
reopenBatchBackoffMillis,
+ public ReopenTableRegionsProcedure(final TableName tableName, long
reopenBatchBackoffMillis,
Review Comment:
Why have you made these public?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java:
##########
@@ -4251,6 +4251,54 @@ protected String getDescription() {
}
+ /**
+ * Reopen regions provided in the argument. Applies throttling to the
procedure to avoid
+ * overwhelming the system. This is used by the reopenTableRegions methods
in the Admin API via
+ * HMaster.
+ * @param tableName The current table name
+ * @param regionNames The region names of the regions to reopen
+ * @param nonceGroup Identifier for the source of the request, a client or
process
+ * @param nonce A unique identifier for this operation from the client
or process identified
+ * by <code>nonceGroup</code> (the source must ensure
each operation gets a
+ * unique id).
+ * @return procedure Id
+ * @throws IOException if reopening region fails while running procedure
+ */
+ long reopenRegionsThrottled(final TableName tableName, final List<byte[]>
regionNames,
Review Comment:
Throttling is a net new feature with this change? I'm surprised it's not
introduced as a separate feature.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
