haridsv commented on code in PR #7584:
URL: https://github.com/apache/hbase/pull/7584#discussion_r2671102851
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java:
##########
@@ -7592,37 +7654,60 @@ public String toString() {
}
// Utility methods
+ @InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.UNITTEST)
+ public static HRegion newHRegion(Path tableDir, WAL wal, FileSystem fs,
Configuration conf,
+ RegionInfo regionInfo, final TableDescriptor htd, RegionServerServices
rsServices) {
+ return newHRegion(tableDir, wal, fs, conf, regionInfo, htd, rsServices,
null);
+ }
+
/**
* A utility method to create new instances of HRegion based on the {@link
HConstants#REGION_IMPL}
* configuration property.
- * @param tableDir qualified path of directory where region should be
located, usually the table
- * directory.
- * @param wal The WAL is the outbound log for any updates to the
HRegion The wal file is a
- * logfile from the previous execution that's
custom-computed for this HRegion.
- * The HRegionServer computes and sorts the appropriate
wal info for this
- * HRegion. If there is a previous file (implying that the
HRegion has been
- * written-to before), then read it from the supplied path.
- * @param fs is the filesystem.
- * @param conf is global configuration settings.
- * @param regionInfo - RegionInfo that describes the region is new), then
read them from the
- * supplied path.
- * @param htd the table descriptor
+ * @param tableDir qualified path of directory where region
should be located, usually
+ * the table directory.
+ * @param wal The WAL is the outbound log for any updates
to the HRegion The wal
+ * file is a logfile from the previous execution
that's
+ * custom-computed for this HRegion. The
HRegionServer computes and
+ * sorts the appropriate wal info for this
HRegion. If there is a
+ * previous file (implying that the HRegion has
been written-to
+ * before), then read it from the supplied path.
+ * @param fs is the filesystem.
+ * @param conf is global configuration settings.
+ * @param regionInfo - RegionInfo that describes the region is
new), then read them from
+ * the supplied path.
+ * @param htd the table descriptor
+ * @param keyManagementService reference to {@link KeyManagementService} or
null
* @return the new instance
*/
public static HRegion newHRegion(Path tableDir, WAL wal, FileSystem fs,
Configuration conf,
- RegionInfo regionInfo, final TableDescriptor htd, RegionServerServices
rsServices) {
+ RegionInfo regionInfo, final TableDescriptor htd, RegionServerServices
rsServices,
+ final KeyManagementService keyManagementService) {
+ List<Class<?>> ctorArgTypes =
+ Arrays.asList(Path.class, WAL.class, FileSystem.class,
Configuration.class, RegionInfo.class,
+ TableDescriptor.class, RegionServerServices.class,
KeyManagementService.class);
+ List<Object> ctorArgs =
+ Arrays.asList(tableDir, wal, fs, conf, regionInfo, htd, rsServices,
keyManagementService);
+
+ try {
+ return createInstance(conf, ctorArgTypes, ctorArgs);
+ } catch (Throwable e) {
+ // Try the old signature for the sake of test code.
+ return createInstance(conf, ctorArgTypes.subList(0, ctorArgTypes.size()
- 1),
+ ctorArgs.subList(0, ctorArgs.size() - 1));
+ }
Review Comment:
Actually, the existing `createInstance` is already doing a catch all and
convert it to `IllegalStateException`, so there is technically no difference
between catching `Throwable` or more specific `IllegalStateException` in this
catch block. I will still change it to be clear about the intention and avoid
any potential confusion.
--
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]
