Revert "HBASE-27028 Add a shell command for flushing master local region (apache#4457)"

wenwj0 · web-flow · commit 050b2879d9c0 · 2022-06-14T14:23:27.000+08:00
This reverts commit 958ecc3.
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java
@@ -2539,9 +2539,4 @@ default BalanceResponse balanceRSGroup(String groupName) throws IOException {
    */
   List<LogEntry> getLogEntries(Set<ServerName> serverNames, String logType, ServerType serverType,
     int limit, Map<String, Object> filterParams) throws IOException;
-
-  /**
-   * Flush master local region
-   */
-  void flushMasterStore() throws IOException;
 }
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AdminOverAsyncAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AdminOverAsyncAdmin.java
@@ -1078,9 +1078,4 @@ public List<LogEntry> getLogEntries(Set<ServerName> serverNames, String logType,
     ServerType serverType, int limit, Map<String, Object> filterParams) throws IOException {
     return get(admin.getLogEntries(serverNames, logType, serverType, limit, filterParams));
   }
-
-  @Override
-  public void flushMasterStore() throws IOException {
-    get(admin.flushMasterStore());
-  }
 }
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncAdmin.java
@@ -1785,9 +1785,4 @@ default CompletableFuture<BalanceResponse> balanceRSGroup(String groupName) {
    */
   CompletableFuture<List<LogEntry>> getLogEntries(Set<ServerName> serverNames, String logType,
     ServerType serverType, int limit, Map<String, Object> filterParams);
-
-  /**
-   * Flush master local region
-   */
-  CompletableFuture<Void> flushMasterStore();
 }
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncHBaseAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncHBaseAdmin.java
@@ -949,9 +949,4 @@ public CompletableFuture<List<LogEntry>> getLogEntries(Set<ServerName> serverNam
     String logType, ServerType serverType, int limit, Map<String, Object> filterParams) {
     return wrap(rawAdmin.getLogEntries(serverNames, logType, serverType, limit, filterParams));
   }
-
-  @Override
-  public CompletableFuture<Void> flushMasterStore() {
-    return wrap(rawAdmin.flushMasterStore());
-  }
 }
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java
@@ -178,8 +178,6 @@
 import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.EnableTableResponse;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.ExecProcedureRequest;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.ExecProcedureResponse;
-import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.FlushMasterStoreRequest;
-import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.FlushMasterStoreResponse;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetClusterStatusRequest;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetClusterStatusResponse;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetCompletedSnapshotsRequest;
@@ -4282,14 +4280,4 @@ public CompletableFuture<List<LogEntry>> getLogEntries(Set<ServerName> serverNam
         return CompletableFuture.completedFuture(Collections.emptyList());
     }
   }
-
-  @Override
-  public CompletableFuture<Void> flushMasterStore() {
-    FlushMasterStoreRequest.Builder request = FlushMasterStoreRequest.newBuilder();
-    return this.<Void> newMasterCaller()
-      .action(((controller, stub) -> this.<FlushMasterStoreRequest, FlushMasterStoreResponse,
-        Void> call(controller, stub, request.build(),
-          (s, c, req, done) -> s.flushMasterStore(c, req, done), resp -> null)))
-      .call();
-  }
 }
diff --git a/hbase-protocol-shaded/src/main/protobuf/server/master/Master.proto b/hbase-protocol-shaded/src/main/protobuf/server/master/Master.proto
@@ -758,9 +758,6 @@ message ModifyColumnStoreFileTrackerResponse {
   optional uint64 proc_id = 1;
 }
 
-message FlushMasterStoreRequest {}
-message FlushMasterStoreResponse {}
-
 service MasterService {
   /** Used by the client to get the number of regions that have received the updated schema */
   rpc GetSchemaAlterStatus(GetSchemaAlterStatusRequest)
@@ -1200,9 +1197,6 @@ service MasterService {
 
   rpc ModifyColumnStoreFileTracker(ModifyColumnStoreFileTrackerRequest)
     returns(ModifyColumnStoreFileTrackerResponse);
-
-  rpc FlushMasterStore(FlushMasterStoreRequest)
-    returns(FlushMasterStoreResponse);
 }
 
 // HBCK Service definitions.
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java
@@ -1000,22 +1000,6 @@ default void postTableFlush(final ObserverContext<MasterCoprocessorEnvironment>
     final TableName tableName) throws IOException {
   }
 
-  /**
-   * Called before the master local region memstore is flushed to disk.
-   * @param ctx the environment to interact with the framework and master
-   */
-  default void preMasterStoreFlush(final ObserverContext<MasterCoprocessorEnvironment> ctx)
-    throws IOException {
-  }
-
-  /**
-   * Called after the master local region memstore is flushed to disk.
-   * @param ctx the environment to interact with the framework and master
-   */
-  default void postMasterStoreFlush(final ObserverContext<MasterCoprocessorEnvironment> ctx)
-    throws IOException {
-  }
-
   /**
    * Called before the quota for the user is stored.
    * @param ctx      the environment to interact with the framework and master
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
@@ -4225,26 +4225,6 @@ public List<HRegionLocation> getMetaLocations() {
     return metaRegionLocationCache.getMetaRegionLocations();
   }
 
-  @Override
-  public void flushMasterStore() throws IOException {
-    LOG.info("Force flush master local region.");
-    if (this.cpHost != null) {
-      try {
-        cpHost.preMasterStoreFlush();
-      } catch (IOException ioe) {
-        LOG.error("Error invoking master coprocessor preMasterStoreFlush()", ioe);
-      }
-    }
-    masterRegion.flush(true);
-    if (this.cpHost != null) {
-      try {
-        cpHost.postMasterStoreFlush();
-      } catch (IOException ioe) {
-        LOG.error("Error invoking master coprocessor postMasterStoreFlush()", ioe);
-      }
-    }
-  }
-
   public Collection<ServerName> getLiveRegionServers() {
     return regionServerTracker.getRegionServers();
   }
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java
@@ -1218,24 +1218,6 @@ public void call(MasterObserver observer) throws IOException {
     });
   }
 
-  public void preMasterStoreFlush() throws IOException {
-    execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() {
-      @Override
-      public void call(MasterObserver observer) throws IOException {
-        observer.preMasterStoreFlush(this);
-      }
-    });
-  }
-
-  public void postMasterStoreFlush() throws IOException {
-    execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() {
-      @Override
-      public void call(MasterObserver observer) throws IOException {
-        observer.postMasterStoreFlush(this);
-      }
-    });
-  }
-
   public void preSetUserQuota(final String user, final GlobalQuotaSettings quotas)
     throws IOException {
     execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() {
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
@@ -230,8 +230,6 @@
 import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.ExecProcedureResponse;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.FixMetaRequest;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.FixMetaResponse;
-import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.FlushMasterStoreRequest;
-import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.FlushMasterStoreResponse;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetClusterStatusRequest;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetClusterStatusResponse;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetCompletedSnapshotsRequest;
@@ -3471,16 +3469,4 @@ public ReplicateWALEntryResponse replicateToReplica(RpcController controller,
     ReplicateWALEntryRequest request) throws ServiceException {
     throw new ServiceException(new DoNotRetryIOException("Unsupported method on master"));
   }
-
-  @Override
-  public FlushMasterStoreResponse flushMasterStore(RpcController controller,
-    FlushMasterStoreRequest request) throws ServiceException {
-    rpcPreCheck("flushMasterStore");
-    try {
-      server.flushMasterStore();
-    } catch (IOException ioe) {
-      throw new ServiceException(ioe);
-    }
-    return FlushMasterStoreResponse.newBuilder().build();
-  }
 }
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java
@@ -510,9 +510,4 @@ boolean normalizeRegions(final NormalizeTableFilterParams ntfp, final boolean is
    * We need to get this in MTP to tell the syncer the new meta replica count.
    */
   MetaLocationSyncer getMetaLocationSyncer();
-
-  /**
-   * Flush master local region
-   */
-  void flushMasterStore() throws IOException;
 }
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/region/MasterRegion.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/region/MasterRegion.java
@@ -161,11 +161,10 @@ public RegionScanner getRegionScanner(Scan scan) throws IOException {
     return region.getScanner(scan);
   }
 
+  @RestrictedApi(explanation = "Should only be called in tests", link = "",
+      allowedOnPath = ".*/src/test/.*")
   public FlushResult flush(boolean force) throws IOException {
-    flusherAndCompactor.resetChangesAfterLastFlush();
-    FlushResult flushResult = region.flush(force);
-    flusherAndCompactor.recordLastFlushTime();
-    return flushResult;
+    return region.flush(force);
   }
 
   @RestrictedApi(explanation = "Should only be called in tests", link = "",
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/region/MasterRegionFlusherAndCompactor.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/region/MasterRegionFlusherAndCompactor.java
@@ -180,7 +180,7 @@ private boolean needCompaction() {
   }
 
   private void flushLoop() {
-    recordLastFlushTime();
+    lastFlushTime = EnvironmentEdgeManager.currentTime();
     while (!closed) {
       flushLock.lock();
       try {
@@ -202,10 +202,10 @@ private void flushLoop() {
         flushLock.unlock();
       }
       assert flushRequest;
-      resetChangesAfterLastFlush();
+      changesAfterLastFlush.set(0);
       try {
         region.flush(true);
-        recordLastFlushTime();
+        lastFlushTime = EnvironmentEdgeManager.currentTime();
       } catch (IOException e) {
         LOG.error(HBaseMarkers.FATAL, "Failed to flush master local region, aborting...", e);
         abortable.abort("Failed to flush master local region", e);
@@ -263,14 +263,6 @@ void requestFlush() {
     }
   }
 
-  void resetChangesAfterLastFlush() {
-    changesAfterLastFlush.set(0);
-  }
-
-  void recordLastFlushTime() {
-    lastFlushTime = EnvironmentEdgeManager.currentTime();
-  }
-
   @Override
   public void close() {
     closed = true;
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java
@@ -189,8 +189,6 @@ public static class CPMasterObserver implements MasterCoprocessor, MasterObserve
     private boolean postRequestLockCalled;
     private boolean preLockHeartbeatCalled;
     private boolean postLockHeartbeatCalled;
-    private boolean preMasterStoreFlushCalled;
-    private boolean postMasterStoreFlushCalled;
 
     public void resetStates() {
       preCreateTableRegionInfosCalled = false;
@@ -282,8 +280,6 @@ public void resetStates() {
       postRequestLockCalled = false;
       preLockHeartbeatCalled = false;
       postLockHeartbeatCalled = false;
-      preMasterStoreFlushCalled = false;
-      postMasterStoreFlushCalled = false;
     }
 
     @Override
@@ -1049,18 +1045,6 @@ public void postTableFlush(ObserverContext<MasterCoprocessorEnvironment> ctx,
       TableName tableName) throws IOException {
     }
 
-    @Override
-    public void preMasterStoreFlush(ObserverContext<MasterCoprocessorEnvironment> ctx)
-      throws IOException {
-      preMasterStoreFlushCalled = true;
-    }
-
-    @Override
-    public void postMasterStoreFlush(ObserverContext<MasterCoprocessorEnvironment> ctx)
-      throws IOException {
-      postMasterStoreFlushCalled = true;
-    }
-
     @Override
     public void preSetUserQuota(final ObserverContext<MasterCoprocessorEnvironment> ctx,
       final String userName, final GlobalQuotaSettings quotas) throws IOException {
@@ -1696,23 +1680,4 @@ public void testQueueLockAndLockHeartbeatOperations() throws Exception {
     ProcedureTestingUtility.waitNoProcedureRunning(master.getMasterProcedureExecutor());
     ProcedureTestingUtility.assertProcNotFailed(master.getMasterProcedureExecutor(), procId);
   }
-
-  @Test
-  public void testMasterStoreOperations() throws Exception {
-    SingleProcessHBaseCluster cluster = UTIL.getHBaseCluster();
-    HMaster master = cluster.getMaster();
-    MasterCoprocessorHost host = master.getMasterCoprocessorHost();
-    CPMasterObserver cp = host.findCoprocessor(CPMasterObserver.class);
-    cp.resetStates();
-    assertFalse("No master store flush call", cp.preMasterStoreFlushCalled);
-    assertFalse("No master store flush call", cp.postMasterStoreFlushCalled);
-
-    try (Connection connection = ConnectionFactory.createConnection(UTIL.getConfiguration());
-      Admin admin = connection.getAdmin()) {
-      admin.flushMasterStore();
-
-      assertTrue("Master store flush called", cp.preMasterStoreFlushCalled);
-      assertTrue("Master store flush called", cp.postMasterStoreFlushCalled);
-    }
-  }
 }
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockNoopMasterServices.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockNoopMasterServices.java
@@ -503,10 +503,6 @@ public MetaLocationSyncer getMetaLocationSyncer() {
     return null;
   }
 
-  @Override
-  public void flushMasterStore() {
-  }
-
   @Override
   public long modifyTableStoreFileTracker(TableName tableName, String dstSFT, long nonceGroup,
     long nonce) throws IOException {
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/region/TestMasterRegionFlush.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/region/TestMasterRegionFlush.java
@@ -148,5 +148,6 @@ public void testPeriodicalFlush() throws InterruptedException {
     assertEquals(1, flushCalled.get());
     Thread.sleep(1000);
     assertEquals(2, flushCalled.get());
+
   }
 }
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/rsgroup/VerifyingRSGroupAdmin.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/rsgroup/VerifyingRSGroupAdmin.java
@@ -934,9 +934,4 @@ public Future<Void> modifyTableStoreFileTrackerAsync(TableName tableName, String
     throws IOException {
     return admin.modifyTableStoreFileTrackerAsync(tableName, dstSFT);
   }
-
-  @Override
-  public void flushMasterStore() throws IOException {
-    admin.flushMasterStore();
-  }
 }
diff --git a/hbase-shell/src/main/ruby/hbase/admin.rb b/hbase-shell/src/main/ruby/hbase/admin.rb
@@ -1868,17 +1868,11 @@ def modify_table_sft(tableName, sft)
       @admin.modifyTableStoreFileTracker(tableName, sft)
     end
 
-    #----------------------------------------------------------------------------------------------
+     #----------------------------------------------------------------------------------------------
     # Change table column family's sft
     def modify_table_family_sft(tableName, family_bytes, sft)
       @admin.modifyColumnFamilyStoreFileTracker(tableName, family_bytes, sft)
     end
-
-    #----------------------------------------------------------------------------------------------
-    # Flush master local region
-    def flush_master_store()
-      @admin.flushMasterStore()
-    end
   end
   # rubocop:enable Metrics/ClassLength
 end
diff --git a/hbase-shell/src/main/ruby/shell.rb b/hbase-shell/src/main/ruby/shell.rb
@@ -446,7 +446,6 @@ def self.exception_handler(hide_traceback)
     compact
     compaction_switch
     flush
-    flush_master_store
     get_balancer_decisions
     get_balancer_rejections
     get_slowlog_responses
diff --git a/hbase-shell/src/main/ruby/shell/commands/flush_master_store.rb b/hbase-shell/src/main/ruby/shell/commands/flush_master_store.rb
diff --git a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/client/ThriftAdmin.java b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/client/ThriftAdmin.java

Original file line number	Diff line number	Diff line change
`@@ -1078,9 +1078,4 @@ public List<LogEntry> getLogEntries(Set<ServerName> serverNames, String logType,`
`1078`	`1078`	`ServerType serverType, int limit, Map<String, Object> filterParams) throws IOException {`
`1079`	`1079`	`return get(admin.getLogEntries(serverNames, logType, serverType, limit, filterParams));`
`1080`	`1080`	`}`
`1081`		`-`
`1082`		`- @Override`
`1083`		`- public void flushMasterStore() throws IOException {`
`1084`		`- get(admin.flushMasterStore());`
`1085`		`- }`
`1086`	`1081`	`}`
Original file line number	Diff line number	Diff line change
`@@ -949,9 +949,4 @@ public CompletableFuture<List<LogEntry>> getLogEntries(Set<ServerName> serverNam`
`949`	`949`	`String logType, ServerType serverType, int limit, Map<String, Object> filterParams) {`
`950`	`950`	`return wrap(rawAdmin.getLogEntries(serverNames, logType, serverType, limit, filterParams));`
`951`	`951`	`}`
`952`		`-`
`953`		`- @Override`
`954`		`- public CompletableFuture<Void> flushMasterStore() {`
`955`		`- return wrap(rawAdmin.flushMasterStore());`
`956`		`- }`
`957`	`952`	`}`