[GitHub] [hbase] Apache9 commented on a change in pull request #1811: HBASE-24474 Rename LocalRegion to MasterRegion
Apache9 commented on a change in pull request #1811:
URL: https://github.com/apache/hbase/pull/1811#discussion_r433584900
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
##
@@ -1563,7 +1564,7 @@ protected void stopServiceThreads() {
private void createProcedureExecutor() throws IOException {
MasterProcedureEnv procEnv = new MasterProcedureEnv(this);
procedureStore =
- new RegionProcedureStore(this, localStore, new
MasterProcedureEnv.FsUtilsLeaseRecovery(this));
+ new RegionProcedureStore(this, masterRegion, new
MasterProcedureEnv.FsUtilsLeaseRecovery(this));
Review comment:
The idea is to use different families. There is a known risk that, if
someone stores a lot data in one of the families, it will slow down the start
up of the whole HMaster, even if it is not necessary. We should document this
in our ref guide. Can be a follow on issue?
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:
us...@infra.apache.org
[GitHub] [hbase] Apache9 commented on a change in pull request #1811: HBASE-24474 Rename LocalRegion to MasterRegion
Apache9 commented on a change in pull request #1811:
URL: https://github.com/apache/hbase/pull/1811#discussion_r433583919
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
##
@@ -258,6 +259,14 @@
public static final String SPECIAL_RECOVERED_EDITS_DIR =
"hbase.hregion.special.recovered.edits.dir";
+ /**
+ * Whether to use {@link MetaCellComparator} even if we are not meta region.
Used when creating
+ * master local region.
+ */
+ public static final String USE_META_CELL_COMPARATOR =
"hbase.region.use.meta.cell.comparator";
Review comment:
For now it is only used in HMaster but I do not think it should be
prefixed with hbase.master, as it is a configuration for HRegion.
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:
us...@infra.apache.org
[GitHub] [hbase] Apache9 commented on a change in pull request #1811: HBASE-24474 Rename LocalRegion to MasterRegion
Apache9 commented on a change in pull request #1811:
URL: https://github.com/apache/hbase/pull/1811#discussion_r433582964
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/region/MasterRegion.java
##
@@ -79,14 +79,14 @@
* Notice that, you can use different root file system and WAL file system.
Then the above directory
* will be on two file systems, the root file system will have the data
directory while the WAL
* filesystem will have the WALs directory. The archived HFile will be moved
to the global HFile
- * archived directory with the {@link LocalRegionParams#archivedWalSuffix()}
suffix. The archived
+ * archived directory with the {@link MasterRegionParams#archivedWalSuffix()}
suffix. The archived
* WAL will be moved to the global WAL archived directory with the
- * {@link LocalRegionParams#archivedHFileSuffix()} suffix.
+ * {@link MasterRegionParams#archivedHFileSuffix()} suffix.
*/
@InterfaceAudience.Private
-public final class LocalRegion {
+public final class MasterRegion {
Review comment:
This is intentional. You can see the implementation of
MasterRegion.update, we have to call `flusherAndCompactor.onUpdate();` after
each update. So if we expose the HRegion directly, the callers have to do this
by their own, and I believe it will be easy to forget and then cause big
problem...
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:
us...@infra.apache.org
[GitHub] [hbase] Apache9 commented on a change in pull request #1811: HBASE-24474 Rename LocalRegion to MasterRegion
Apache9 commented on a change in pull request #1811:
URL: https://github.com/apache/hbase/pull/1811#discussion_r432913252
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/region/MasterRegionFactory.java
##
@@ -89,45 +82,8 @@
private static final TableDescriptor TABLE_DESC =
TableDescriptorBuilder.newBuilder(TABLE_NAME)
.setColumnFamily(ColumnFamilyDescriptorBuilder.of(PROC_FAMILY)).build();
- private final LocalRegion region;
-
- private LocalStore(LocalRegion region) {
-this.region = region;
- }
-
- public void update(UpdateLocalRegion action) throws IOException {
-region.update(action);
- }
-
- public Result get(Get get) throws IOException {
-return region.get(get);
- }
-
- public RegionScanner getScanner(Scan scan) throws IOException {
-return region.getScanner(scan);
- }
-
- public void close(boolean abort) {
-region.close(abort);
- }
-
- @VisibleForTesting
- public FlushResult flush(boolean force) throws IOException {
-return region.flush(force);
- }
-
- @VisibleForTesting
- public void requestRollAll() {
-region.requestRollAll();
- }
-
- @VisibleForTesting
- public void waitUntilWalRollFinished() throws InterruptedException {
-region.waitUntilWalRollFinished();
- }
-
- public static LocalStore create(Server server) throws IOException {
-LocalRegionParams params = new LocalRegionParams().server(server)
Review comment:
We will return MasterRegion directly after this change, so we do not
need these methods any more, users can use the methods in MasterRegion directly.
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:
us...@infra.apache.org
[GitHub] [hbase] Apache9 commented on a change in pull request #1811: HBASE-24474 Rename LocalRegion to MasterRegion
Apache9 commented on a change in pull request #1811:
URL: https://github.com/apache/hbase/pull/1811#discussion_r432913204
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/region/MasterRegion.java
##
@@ -284,18 +284,21 @@ public static LocalRegion create(LocalRegionParams
params) throws IOException {
Configuration conf = new Configuration(baseConf);
CommonFSUtils.setRootDir(conf, rootDir);
CommonFSUtils.setWALRootDir(conf, walRootDir);
-LocalRegionFlusherAndCompactor.setupConf(conf, params.flushSize(),
params.flushPerChanges(),
+MasterRegionFlusherAndCompactor.setupConf(conf, params.flushSize(),
params.flushPerChanges(),
params.flushIntervalMs());
conf.setInt(AbstractFSWAL.MAX_LOGS, params.maxWals());
if (params.useHsync() != null) {
conf.setBoolean(HRegion.WAL_HSYNC_CONF_KEY, params.useHsync());
}
+if (params.useMetaCellComparator() != null) {
Review comment:
For used as procedure store, this is not necessary. This is for
preparing for storing root table in the future, as for root table, the row key
is the same with meta table, and we need to use a special comparator. I think
it is fine to change the comparator in the future?
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:
us...@infra.apache.org
