[GitHub] [hbase] saintstack commented on a change in pull request #1746: HBASE-24388 Store the locations of meta regions in master local store

[GitBox <gi...@apache.org>]

saintstack commented on a change in pull request #1746:
URL: https://github.com/apache/hbase/pull/1746#discussion_r430831105



##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
##########
@@ -1403,6 +1403,21 @@ private static void deleteFromMetaTable(final Connection connection, final List<
     }
   }
 
+  public static Delete removeRegionReplica(byte[] metaRow, int replicaIndexToDeleteFrom,
+    int numReplicasToRemove) {
+    int absoluteIndex = replicaIndexToDeleteFrom + numReplicasToRemove;
+    long now = EnvironmentEdgeManager.currentTime();
+    Delete deleteReplicaLocations = new Delete(metaRow);
+    for (int i = replicaIndexToDeleteFrom; i < absoluteIndex; i++) {
+      deleteReplicaLocations.addColumns(getCatalogFamily(), getServerColumn(i), now);
+      deleteReplicaLocations.addColumns(getCatalogFamily(), getSeqNumColumn(i), now);
+      deleteReplicaLocations.addColumns(getCatalogFamily(), getStartCodeColumn(i), now);
+      deleteReplicaLocations.addColumns(getCatalogFamily(), getServerNameColumn(i), now);
+      deleteReplicaLocations.addColumns(getCatalogFamily(), getRegionStateColumn(i), now);

Review comment:
       Man. Region Replicas are messy in hbase:meta. Not your fault. For later.

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
##########
@@ -866,8 +874,50 @@ protected void initializeZKBasedSystemTrackers()
 
   // Will be overriden in test to inject customized AssignmentManager
   @VisibleForTesting
-  protected AssignmentManager createAssignmentManager(MasterServices master) {
-    return new AssignmentManager(master);
+  protected AssignmentManager createAssignmentManager(MasterServices master,
+    LocalStore localStore) {
+    return new AssignmentManager(master, localStore);

Review comment:
       Good.
   
   Was going to suggest AM ask the Master for its localStore but that probably TMI for the AM to know of. This is better separation of concerns.
   
   Good.

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/store/LocalStore.java
##########
@@ -87,6 +90,10 @@
   public static final byte[] PROC_FAMILY = Bytes.toBytes("proc");
 
   private static final TableDescriptor TABLE_DESC = TableDescriptorBuilder.newBuilder(TABLE_NAME)
+    .setColumnFamily(ColumnFamilyDescriptorBuilder.newBuilder(HConstants.CATALOG_FAMILY)
+      .setMaxVersions(HConstants.DEFAULT_HBASE_META_VERSIONS).setInMemory(true)
+      .setBlocksize(HConstants.DEFAULT_HBASE_META_BLOCK_SIZE).setBloomFilterType(BloomType.ROWCOL)
+      .setDataBlockEncoding(DataBlockEncoding.ROW_INDEX_V1).build())

Review comment:
       Yeah, the ROW_INDEX_V1 is good change.

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java
##########
@@ -225,23 +230,52 @@ public void start() throws IOException, KeeperException {
     // Start the Assignment Thread
     startAssignmentThread();
 
-    // load meta region state
-    ZKWatcher zkw = master.getZooKeeper();

Review comment:
       Radical. No ZKW in AM. Good.




----------------------------------------------------------------
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:
users@infra.apache.org