apurtell commented on a change in pull request #4136:
URL: https://github.com/apache/hbase/pull/4136#discussion_r839096014
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
##########
@@ -964,7 +969,8 @@ private void finishActiveMasterInitialization(MonitoredTask
status) throws IOExc
this.regionServerTracker.upgrade(
procsByType.getOrDefault(ServerCrashProcedure.class,
Collections.emptyList()).stream()
.map(p -> (ServerCrashProcedure) p).map(p ->
p.getServerName()).collect(Collectors.toSet()),
- walManager.getLiveServersFromWALDir(),
walManager.getSplittingServersFromWALDir());
+ Sets.union(rsListStorage.getAll(),
walManager.getLiveServersFromWALDir()),
Review comment:
Do we still need to also track, and take a union here of, live servers
via WALManager? Is the WALManager based live server stuff redundant now? Can we
remove it?
##########
File path:
hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRestartWithEmptyWALDirectory.java
##########
@@ -0,0 +1,104 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.master;
+
+import static org.junit.Assert.assertArrayEquals;
+
+import java.io.IOException;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseTestingUtil;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.Get;
+import org.apache.hadoop.hbase.client.Put;
+import org.apache.hadoop.hbase.client.Table;
+import org.apache.hadoop.hbase.testclassification.MasterTests;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.CommonFSUtils;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+/**
+ * Simulate the scenario described in HBASE-26245, where we clean the WAL
directory and try to start
Review comment:
This is excellent! Have been bitten by this scenario in test
environments, nice to see a passing test for a solution.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionServerListStorage.java
##########
@@ -0,0 +1,47 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.master;
+
+import java.io.IOException;
+import java.util.Set;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.yetus.audience.InterfaceAudience;
+
+/**
+ * For storing the region server list.
+ * <p/>
+ * Mainly be used when restarting master, to load the previous active region
server list.
+ */
[email protected]
+public interface RegionServerListStorage {
Review comment:
I think the 'Storage' part has a redundant meaning, let's just call this
`RegionServerList` ?
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRegionRegionServerListStorage.java
##########
@@ -0,0 +1,112 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.master;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.util.HashSet;
+import java.util.Set;
+import org.apache.hadoop.hbase.Abortable;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.client.Delete;
+import org.apache.hadoop.hbase.client.Put;
+import org.apache.hadoop.hbase.client.Result;
+import org.apache.hadoop.hbase.client.ResultScanner;
+import org.apache.hadoop.hbase.client.Scan;
+import org.apache.hadoop.hbase.log.HBaseMarkers;
+import org.apache.hadoop.hbase.master.assignment.ServerState;
+import org.apache.hadoop.hbase.master.region.MasterRegion;
+import org.apache.hadoop.hbase.master.region.MasterRegionFactory;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * {@link MasterRegion} based {@link RegionServerListStorage}.
+ * <p/>
+ * This is useful when we want to restart a cluster with only the data on file
system, as we
+ * restarting, we need to get the previous live region servers for scheduling
SCP. Before we have
+ * this class, we need to scan the WAL directory on WAL file system to find
out the previous live
+ * region servers, which means we can not restart a cluster without the
previous WAL file system,
+ * even if we have flushed all the data.
+ * <p/>
+ * Please see HBASE-26245 for more details.
+ */
[email protected]
+public class MasterRegionRegionServerListStorage implements
RegionServerListStorage {
Review comment:
Why not just `MasterRegionServerList` ? The 'Region' part of
MasterRegion can be implied. See elsewhere that the 'Storage' part of
RegionServerListStorage can also be implied.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
