infraio commented on a change in pull request #1746:
URL: https://github.com/apache/hbase/pull/1746#discussion_r430349196
##########
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();
- // it could be null in some tests
- if (zkw != null) {
- RegionState regionState = MetaTableLocator.getMetaRegionState(zkw);
- RegionStateNode regionNode =
-
regionStates.getOrCreateRegionStateNode(RegionInfoBuilder.FIRST_META_REGIONINFO);
- regionNode.lock();
- try {
- regionNode.setRegionLocation(regionState.getServerName());
- regionNode.setState(regionState.getState());
- if (regionNode.getProcedure() != null) {
- regionNode.getProcedure().stateLoaded(this, regionNode);
+ // load meta region states.
+ // notice that, here we will load all replicas, and in MasterMetaBootstrap
we may assign new
+ // replicas, or remove excess replicas.
+ try (RegionScanner scanner =
+ localStore.getScanner(new Scan().addFamily(HConstants.CATALOG_FAMILY))) {
+ List<Cell> cells = new ArrayList<>();
+ boolean moreRows;
+ do {
+ moreRows = scanner.next(cells);
+ if (cells.isEmpty()) {
+ continue;
}
- setMetaAssigned(regionState.getRegion(), regionState.getState() ==
State.OPEN);
- } finally {
- regionNode.unlock();
+ Result result = Result.create(cells);
+ cells.clear();
+ RegionStateStore
+ .visitMetaEntry((r, regionInfo, state, regionLocation, lastHost,
openSeqNum) -> {
+ RegionStateNode regionNode =
regionStates.getOrCreateRegionStateNode(regionInfo);
+ regionNode.lock();
+ try {
+ regionNode.setState(state);
+ regionNode.setLastHost(lastHost);
Review comment:
Seems the old code didn't setLastHost and setOpenSeqNum? Why add this?
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterMetaBootstrap.java
##########
@@ -43,73 +49,103 @@
private final HMaster master;
- public MasterMetaBootstrap(HMaster master) {
+ private final LocalStore localStore;
+
+ public MasterMetaBootstrap(HMaster master, LocalStore localStore) {
this.master = master;
+ this.localStore = localStore;
}
/**
* For assigning hbase:meta replicas only.
- * TODO: The way this assign runs, nothing but chance to stop all replicas
showing up on same
- * server as the hbase:meta region.
*/
- void assignMetaReplicas()
- throws IOException, InterruptedException, KeeperException {
+ void assignMetaReplicas() throws IOException, InterruptedException,
KeeperException {
int numReplicas =
master.getConfiguration().getInt(HConstants.META_REPLICAS_NUM,
- HConstants.DEFAULT_META_REPLICA_NUM);
- if (numReplicas <= 1) {
- // No replicaas to assign. Return.
- return;
- }
- final AssignmentManager assignmentManager = master.getAssignmentManager();
- if (!assignmentManager.isMetaLoaded()) {
- throw new IllegalStateException("hbase:meta must be initialized first
before we can " +
- "assign out its replicas");
- }
- ServerName metaServername =
MetaTableLocator.getMetaRegionLocation(this.master.getZooKeeper());
- for (int i = 1; i < numReplicas; i++) {
- // Get current meta state for replica from zk.
- RegionState metaState =
MetaTableLocator.getMetaRegionState(master.getZooKeeper(), i);
- RegionInfo hri = RegionReplicaUtil.getRegionInfoForReplica(
- RegionInfoBuilder.FIRST_META_REGIONINFO, i);
- LOG.debug(hri.getRegionNameAsString() + " replica region state from
zookeeper=" + metaState);
- if (metaServername.equals(metaState.getServerName())) {
- metaState = null;
- LOG.info(hri.getRegionNameAsString() +
- " old location is same as current hbase:meta location; setting
location as null...");
+ HConstants.DEFAULT_META_REPLICA_NUM);
+ // only try to assign meta replicas when there are more than 1 replicas
+ if (numReplicas > 1) {
+ final AssignmentManager am = master.getAssignmentManager();
+ if (!am.isMetaLoaded()) {
+ throw new IllegalStateException(
+ "hbase:meta must be initialized first before we can " + "assign out
its replicas");
}
- // These assigns run inline. All is blocked till they complete. Only
interrupt is shutting
- // down hosting server which calls AM#stop.
- if (metaState != null && metaState.getServerName() != null) {
- // Try to retain old assignment.
- assignmentManager.assign(hri, metaState.getServerName());
- } else {
- assignmentManager.assign(hri);
+ RegionStates regionStates = am.getRegionStates();
+ for (RegionInfo regionInfo :
regionStates.getRegionsOfTable(TableName.META_TABLE_NAME)) {
+ if (!RegionReplicaUtil.isDefaultReplica(regionInfo)) {
+ continue;
+ }
+ RegionState regionState = regionStates.getRegionState(regionInfo);
+ Set<ServerName> metaServerNames = new HashSet<ServerName>();
+ if (regionState.getServerName() != null) {
+ metaServerNames.add(regionState.getServerName());
+ }
+ for (int i = 1; i < numReplicas; i++) {
+ RegionInfo secondaryRegionInfo =
RegionReplicaUtil.getRegionInfoForReplica(regionInfo, i);
+ RegionState secondaryRegionState =
regionStates.getRegionState(secondaryRegionInfo);
+ ServerName sn = null;
+ if (secondaryRegionState != null) {
+ sn = secondaryRegionState.getServerName();
+ if (sn != null && !metaServerNames.add(sn)) {
+ LOG.info("{} old location {} is same with other hbase:meta
replica location;" +
+ " setting location as null...",
secondaryRegionInfo.getRegionNameAsString(), sn);
Review comment:
Donot need unassign first?
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateStore.java
##########
@@ -216,12 +200,32 @@ private void updateUserRegionLocation(RegionInfo
regionInfo, State state,
.build());
LOG.info(info.toString());
updateRegionLocation(regionInfo, state, put);
+ if (regionInfo.isMetaRegion() && regionInfo.isFirst()) {
+ // mirror the meta location to
Review comment:
mirror the meta location to zookeeper?
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterMetaBootstrap.java
##########
@@ -43,73 +49,103 @@
private final HMaster master;
- public MasterMetaBootstrap(HMaster master) {
+ private final LocalStore localStore;
+
+ public MasterMetaBootstrap(HMaster master, LocalStore localStore) {
this.master = master;
+ this.localStore = localStore;
}
/**
* For assigning hbase:meta replicas only.
- * TODO: The way this assign runs, nothing but chance to stop all replicas
showing up on same
- * server as the hbase:meta region.
*/
- void assignMetaReplicas()
- throws IOException, InterruptedException, KeeperException {
+ void assignMetaReplicas() throws IOException, InterruptedException,
KeeperException {
int numReplicas =
master.getConfiguration().getInt(HConstants.META_REPLICAS_NUM,
- HConstants.DEFAULT_META_REPLICA_NUM);
- if (numReplicas <= 1) {
- // No replicaas to assign. Return.
- return;
- }
- final AssignmentManager assignmentManager = master.getAssignmentManager();
- if (!assignmentManager.isMetaLoaded()) {
- throw new IllegalStateException("hbase:meta must be initialized first
before we can " +
- "assign out its replicas");
- }
- ServerName metaServername =
MetaTableLocator.getMetaRegionLocation(this.master.getZooKeeper());
- for (int i = 1; i < numReplicas; i++) {
- // Get current meta state for replica from zk.
- RegionState metaState =
MetaTableLocator.getMetaRegionState(master.getZooKeeper(), i);
- RegionInfo hri = RegionReplicaUtil.getRegionInfoForReplica(
- RegionInfoBuilder.FIRST_META_REGIONINFO, i);
- LOG.debug(hri.getRegionNameAsString() + " replica region state from
zookeeper=" + metaState);
- if (metaServername.equals(metaState.getServerName())) {
- metaState = null;
- LOG.info(hri.getRegionNameAsString() +
- " old location is same as current hbase:meta location; setting
location as null...");
+ HConstants.DEFAULT_META_REPLICA_NUM);
+ // only try to assign meta replicas when there are more than 1 replicas
+ if (numReplicas > 1) {
+ final AssignmentManager am = master.getAssignmentManager();
+ if (!am.isMetaLoaded()) {
+ throw new IllegalStateException(
+ "hbase:meta must be initialized first before we can " + "assign out
its replicas");
}
- // These assigns run inline. All is blocked till they complete. Only
interrupt is shutting
- // down hosting server which calls AM#stop.
- if (metaState != null && metaState.getServerName() != null) {
- // Try to retain old assignment.
- assignmentManager.assign(hri, metaState.getServerName());
- } else {
- assignmentManager.assign(hri);
+ RegionStates regionStates = am.getRegionStates();
+ for (RegionInfo regionInfo :
regionStates.getRegionsOfTable(TableName.META_TABLE_NAME)) {
+ if (!RegionReplicaUtil.isDefaultReplica(regionInfo)) {
+ continue;
+ }
+ RegionState regionState = regionStates.getRegionState(regionInfo);
+ Set<ServerName> metaServerNames = new HashSet<ServerName>();
+ if (regionState.getServerName() != null) {
+ metaServerNames.add(regionState.getServerName());
+ }
+ for (int i = 1; i < numReplicas; i++) {
+ RegionInfo secondaryRegionInfo =
RegionReplicaUtil.getRegionInfoForReplica(regionInfo, i);
+ RegionState secondaryRegionState =
regionStates.getRegionState(secondaryRegionInfo);
+ ServerName sn = null;
+ if (secondaryRegionState != null) {
+ sn = secondaryRegionState.getServerName();
+ if (sn != null && !metaServerNames.add(sn)) {
+ LOG.info("{} old location {} is same with other hbase:meta
replica location;" +
+ " setting location as null...",
secondaryRegionInfo.getRegionNameAsString(), sn);
+ sn = null;
+ }
+ }
+ // These assigns run inline. All is blocked till they complete. Only
interrupt is shutting
+ // down hosting server which calls AM#stop.
+ if (sn != null) {
+ am.assign(secondaryRegionInfo, sn);
+ } else {
+ am.assign(secondaryRegionInfo);
+ }
+ }
}
}
+ // always try to remomve excess meta replicas
unassignExcessMetaReplica(numReplicas);
}
private void unassignExcessMetaReplica(int numMetaReplicasConfigured) {
- final ZKWatcher zooKeeper = master.getZooKeeper();
- // unassign the unneeded replicas (for e.g., if the previous master was
configured
- // with a replication of 3 and now it is 2, we need to unassign the 1
unneeded replica)
- try {
- List<String> metaReplicaZnodes = zooKeeper.getMetaReplicaNodes();
- for (String metaReplicaZnode : metaReplicaZnodes) {
- int replicaId =
zooKeeper.getZNodePaths().getMetaReplicaIdFromZnode(metaReplicaZnode);
- if (replicaId >= numMetaReplicasConfigured) {
- RegionState r = MetaTableLocator.getMetaRegionState(zooKeeper,
replicaId);
- LOG.info("Closing excess replica of meta region " + r.getRegion());
- // send a close and wait for a max of 30 seconds
-
ServerManager.closeRegionSilentlyAndWait(master.getAsyncClusterConnection(),
- r.getServerName(), r.getRegion(), 30000);
- ZKUtil.deleteNode(zooKeeper,
zooKeeper.getZNodePaths().getZNodeForReplica(replicaId));
+ ZKWatcher zooKeeper = master.getZooKeeper();
+ AssignmentManager am = master.getAssignmentManager();
+ RegionStates regionStates = am.getRegionStates();
+ Map<RegionInfo, Integer> region2MaxReplicaId = new HashMap<>();
+ for (RegionInfo regionInfo :
regionStates.getRegionsOfTable(TableName.META_TABLE_NAME)) {
+ RegionInfo primaryRegionInfo =
RegionReplicaUtil.getRegionInfoForDefaultReplica(regionInfo);
+ region2MaxReplicaId.compute(primaryRegionInfo,
+ (k, v) -> v == null ? regionInfo.getReplicaId() : Math.max(v,
regionInfo.getReplicaId()));
+ if (regionInfo.getReplicaId() < numMetaReplicasConfigured) {
+ continue;
+ }
+ RegionState regionState = regionStates.getRegionState(regionInfo);
+ try {
+
ServerManager.closeRegionSilentlyAndWait(master.getAsyncClusterConnection(),
+ regionState.getServerName(), regionInfo, 30000);
+ if (regionInfo.isFirst()) {
+ // for compatibility, also try to remove the replicas on zk.
+ ZKUtil.deleteNode(zooKeeper,
+
zooKeeper.getZNodePaths().getZNodeForReplica(regionInfo.getReplicaId()));
}
+ } catch (Exception e) {
+ // ignore the exception since we don't want the master to be wedged
due to potential
+ // issues in the cleanup of the extra regions. We can do that cleanup
via hbck or manually
+ LOG.warn("Ignoring exception " + e);
}
- } catch (Exception ex) {
- // ignore the exception since we don't want the master to be wedged due
to potential
- // issues in the cleanup of the extra regions. We can do that cleanup
via hbck or manually
- LOG.warn("Ignoring exception " + ex);
+ regionStates.deleteRegion(regionInfo);
}
+ region2MaxReplicaId.forEach((regionInfo, maxReplicaId) -> {
+ if (maxReplicaId >= numMetaReplicasConfigured) {
+ byte[] metaRow = MetaTableAccessor.getMetaKeyForRegion(regionInfo);
+ Delete delete = MetaTableAccessor.removeRegionReplica(metaRow,
numMetaReplicasConfigured,
Review comment:
Why don't delete in previous for-loop? Same with the delete for znode.
##########
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();
- // it could be null in some tests
- if (zkw != null) {
- RegionState regionState = MetaTableLocator.getMetaRegionState(zkw);
- RegionStateNode regionNode =
-
regionStates.getOrCreateRegionStateNode(RegionInfoBuilder.FIRST_META_REGIONINFO);
- regionNode.lock();
- try {
- regionNode.setRegionLocation(regionState.getServerName());
- regionNode.setState(regionState.getState());
- if (regionNode.getProcedure() != null) {
- regionNode.getProcedure().stateLoaded(this, regionNode);
+ // load meta region states.
+ // notice that, here we will load all replicas, and in MasterMetaBootstrap
we may assign new
+ // replicas, or remove excess replicas.
+ try (RegionScanner scanner =
+ localStore.getScanner(new Scan().addFamily(HConstants.CATALOG_FAMILY))) {
+ List<Cell> cells = new ArrayList<>();
+ boolean moreRows;
+ do {
+ moreRows = scanner.next(cells);
+ if (cells.isEmpty()) {
+ continue;
}
- setMetaAssigned(regionState.getRegion(), regionState.getState() ==
State.OPEN);
- } finally {
- regionNode.unlock();
+ Result result = Result.create(cells);
+ cells.clear();
+ RegionStateStore
+ .visitMetaEntry((r, regionInfo, state, regionLocation, lastHost,
openSeqNum) -> {
+ RegionStateNode regionNode =
regionStates.getOrCreateRegionStateNode(regionInfo);
+ regionNode.lock();
+ try {
+ regionNode.setState(state);
+ regionNode.setLastHost(lastHost);
+ regionNode.setRegionLocation(regionLocation);
+ regionNode.setOpenSeqNum(openSeqNum);
+ if (regionNode.getProcedure() != null) {
+ regionNode.getProcedure().stateLoaded(this, regionNode);
+ }
+ if (RegionReplicaUtil.isDefaultReplica(regionInfo)) {
+ setMetaAssigned(regionInfo, state == State.OPEN);
+ }
+ } finally {
+ regionNode.unlock();
+ }
+ if (regionInfo.isFirst()) {
+ // for compatibility, mirror the meta region state to zookeeper
Review comment:
This compatibility is for which case?
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterMetaBootstrap.java
##########
@@ -43,73 +49,103 @@
private final HMaster master;
- public MasterMetaBootstrap(HMaster master) {
+ private final LocalStore localStore;
+
+ public MasterMetaBootstrap(HMaster master, LocalStore localStore) {
this.master = master;
+ this.localStore = localStore;
}
/**
* For assigning hbase:meta replicas only.
- * TODO: The way this assign runs, nothing but chance to stop all replicas
showing up on same
- * server as the hbase:meta region.
*/
- void assignMetaReplicas()
- throws IOException, InterruptedException, KeeperException {
+ void assignMetaReplicas() throws IOException, InterruptedException,
KeeperException {
int numReplicas =
master.getConfiguration().getInt(HConstants.META_REPLICAS_NUM,
- HConstants.DEFAULT_META_REPLICA_NUM);
- if (numReplicas <= 1) {
- // No replicaas to assign. Return.
- return;
- }
- final AssignmentManager assignmentManager = master.getAssignmentManager();
- if (!assignmentManager.isMetaLoaded()) {
- throw new IllegalStateException("hbase:meta must be initialized first
before we can " +
- "assign out its replicas");
- }
- ServerName metaServername =
MetaTableLocator.getMetaRegionLocation(this.master.getZooKeeper());
- for (int i = 1; i < numReplicas; i++) {
- // Get current meta state for replica from zk.
- RegionState metaState =
MetaTableLocator.getMetaRegionState(master.getZooKeeper(), i);
- RegionInfo hri = RegionReplicaUtil.getRegionInfoForReplica(
- RegionInfoBuilder.FIRST_META_REGIONINFO, i);
- LOG.debug(hri.getRegionNameAsString() + " replica region state from
zookeeper=" + metaState);
- if (metaServername.equals(metaState.getServerName())) {
- metaState = null;
- LOG.info(hri.getRegionNameAsString() +
- " old location is same as current hbase:meta location; setting
location as null...");
+ HConstants.DEFAULT_META_REPLICA_NUM);
+ // only try to assign meta replicas when there are more than 1 replicas
+ if (numReplicas > 1) {
+ final AssignmentManager am = master.getAssignmentManager();
+ if (!am.isMetaLoaded()) {
+ throw new IllegalStateException(
+ "hbase:meta must be initialized first before we can " + "assign out
its replicas");
}
- // These assigns run inline. All is blocked till they complete. Only
interrupt is shutting
- // down hosting server which calls AM#stop.
- if (metaState != null && metaState.getServerName() != null) {
- // Try to retain old assignment.
- assignmentManager.assign(hri, metaState.getServerName());
- } else {
- assignmentManager.assign(hri);
+ RegionStates regionStates = am.getRegionStates();
+ for (RegionInfo regionInfo :
regionStates.getRegionsOfTable(TableName.META_TABLE_NAME)) {
+ if (!RegionReplicaUtil.isDefaultReplica(regionInfo)) {
+ continue;
+ }
+ RegionState regionState = regionStates.getRegionState(regionInfo);
+ Set<ServerName> metaServerNames = new HashSet<ServerName>();
+ if (regionState.getServerName() != null) {
+ metaServerNames.add(regionState.getServerName());
+ }
+ for (int i = 1; i < numReplicas; i++) {
+ RegionInfo secondaryRegionInfo =
RegionReplicaUtil.getRegionInfoForReplica(regionInfo, i);
+ RegionState secondaryRegionState =
regionStates.getRegionState(secondaryRegionInfo);
+ ServerName sn = null;
+ if (secondaryRegionState != null) {
+ sn = secondaryRegionState.getServerName();
+ if (sn != null && !metaServerNames.add(sn)) {
+ LOG.info("{} old location {} is same with other hbase:meta
replica location;" +
+ " setting location as null...",
secondaryRegionInfo.getRegionNameAsString(), sn);
Review comment:
Yes. Maybe another issuer.
##########
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();
- // it could be null in some tests
- if (zkw != null) {
- RegionState regionState = MetaTableLocator.getMetaRegionState(zkw);
- RegionStateNode regionNode =
-
regionStates.getOrCreateRegionStateNode(RegionInfoBuilder.FIRST_META_REGIONINFO);
- regionNode.lock();
- try {
- regionNode.setRegionLocation(regionState.getServerName());
- regionNode.setState(regionState.getState());
- if (regionNode.getProcedure() != null) {
- regionNode.getProcedure().stateLoaded(this, regionNode);
+ // load meta region states.
+ // notice that, here we will load all replicas, and in MasterMetaBootstrap
we may assign new
+ // replicas, or remove excess replicas.
+ try (RegionScanner scanner =
+ localStore.getScanner(new Scan().addFamily(HConstants.CATALOG_FAMILY))) {
+ List<Cell> cells = new ArrayList<>();
+ boolean moreRows;
+ do {
+ moreRows = scanner.next(cells);
+ if (cells.isEmpty()) {
+ continue;
}
- setMetaAssigned(regionState.getRegion(), regionState.getState() ==
State.OPEN);
- } finally {
- regionNode.unlock();
+ Result result = Result.create(cells);
+ cells.clear();
+ RegionStateStore
+ .visitMetaEntry((r, regionInfo, state, regionLocation, lastHost,
openSeqNum) -> {
+ RegionStateNode regionNode =
regionStates.getOrCreateRegionStateNode(regionInfo);
+ regionNode.lock();
+ try {
+ regionNode.setState(state);
+ regionNode.setLastHost(lastHost);
+ regionNode.setRegionLocation(regionLocation);
+ regionNode.setOpenSeqNum(openSeqNum);
+ if (regionNode.getProcedure() != null) {
+ regionNode.getProcedure().stateLoaded(this, regionNode);
+ }
+ if (RegionReplicaUtil.isDefaultReplica(regionInfo)) {
+ setMetaAssigned(regionInfo, state == State.OPEN);
+ }
+ } finally {
+ regionNode.unlock();
+ }
+ if (regionInfo.isFirst()) {
+ // for compatibility, mirror the meta region state to zookeeper
Review comment:
Got it.
----------------------------------------------------------------
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]
