Apache9 commented on a change in pull request #1753:
URL: https://github.com/apache/hbase/pull/1753#discussion_r429091002
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/procedure2/store/region/RegionProcedureStore.java
##########
@@ -183,51 +106,14 @@ public void start(int numThreads) throws IOException {
this.numThreads = numThreads;
}
- private void shutdownWAL() {
- if (walFactory != null) {
- try {
- walFactory.shutdown();
- } catch (IOException e) {
- LOG.warn("Failed to shutdown WAL", e);
- }
- }
- }
-
- private void closeRegion(boolean abort) {
- if (region != null) {
- try {
- region.close(abort);
- } catch (IOException e) {
- LOG.warn("Failed to close region", e);
- }
- }
-
- }
-
@Override
public void stop(boolean abort) {
if (!setRunning(false)) {
return;
}
LOG.info("Stopping the Region Procedure Store, isAbort={}", abort);
- if (cleaner != null) {
- cleaner.cancel(abort);
- }
- if (flusherAndCompactor != null) {
- flusherAndCompactor.close();
- }
- // if abort, we shutdown wal first to fail the ongoing updates to the
region, and then close the
- // region, otherwise there will be dead lock.
- if (abort) {
- shutdownWAL();
- closeRegion(true);
- } else {
- closeRegion(false);
- shutdownWAL();
- }
-
- if (walRoller != null) {
- walRoller.close();
+ if (localStore != null) {
Review comment:
The ideal answer is, no. It should be initialized by master and also
stop by master. You can see the draft PR here
https://github.com/apache/hbase/pull/1746
Where I moved the initialization and stop out from the RegionProcedureStore.
And here, since RegionProcedureStore is the only place where we use the
LocalStore, there will be no technical problem to initialize and stop it inside
RegionProcedureStore. Moving it out will have big impact on tests, as in most
tests we will stop the ProcedureStore and reinitialize it again to test fail
recovery. If we move the initialization and stop out, all these tests have to
be rewritten as we need to recreate the LocalStore by our own.
So I prefer we do this in another issue, where we do need to store data
other than procedure to the LocalStore. For example, in HBASE-24388 I have to
do this.
----------------------------------------------------------------
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]
