You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by ch...@apache.org on 2022/04/01 03:30:09 UTC
[hbase] branch branch-2.5 updated: HBASE-26811 Secondary replica may be disabled for read incorrectly forever (#4309)
This is an automated email from the ASF dual-hosted git repository.
chenglei pushed a commit to branch branch-2.5
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-2.5 by this push:
new 69ea6f5 HBASE-26811 Secondary replica may be disabled for read incorrectly forever (#4309)
69ea6f5 is described below
commit 69ea6f579fc5d87f979eeb9e71bb999ace112096
Author: chenglei <ch...@apache.org>
AuthorDate: Fri Apr 1 11:28:29 2022 +0800
HBASE-26811 Secondary replica may be disabled for read incorrectly forever (#4309)
---
.../hbase/client/TableDescriptorBuilder.java | 6 +-
.../apache/hadoop/hbase/regionserver/HRegion.java | 7 ++
.../hadoop/hbase/regionserver/HRegionServer.java | 8 +-
.../TestRegionReplicaWaitForPrimaryFlushConf.java | 125 +++++++++++++++++++++
4 files changed, 140 insertions(+), 6 deletions(-)
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java
index c84388d..813232c 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java
@@ -1393,11 +1393,7 @@ public class TableDescriptorBuilder {
* @return the modifyable TD
*/
public ModifyableTableDescriptor setRegionMemStoreReplication(boolean memstoreReplication) {
- setValue(REGION_MEMSTORE_REPLICATION_KEY, Boolean.toString(memstoreReplication));
- // If the memstore replication is setup, we do not have to wait for observing a flush event
- // from primary before starting to serve reads, because gaps from replication is not applicable
- return setValue(REGION_REPLICA_WAIT_FOR_PRIMARY_FLUSH_CONF_KEY,
- Boolean.toString(memstoreReplication));
+ return setValue(REGION_MEMSTORE_REPLICATION_KEY, Boolean.toString(memstoreReplication));
}
public ModifyableTableDescriptor setPriority(int priority) {
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
index a17fbc8..eae9e0b 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
@@ -23,6 +23,7 @@ import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.REGION_NAMES
import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.ROW_LOCK_READ_LOCK_KEY;
import static org.apache.hadoop.hbase.util.ConcurrentMapUtils.computeIfAbsent;
+import com.google.errorprone.annotations.RestrictedApi;
import edu.umd.cs.findbugs.annotations.Nullable;
import io.opentelemetry.api.trace.Span;
import java.io.EOFException;
@@ -8692,4 +8693,10 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi
public void addWriteRequestsCount(long writeRequestsCount) {
this.writeRequestsCount.add(writeRequestsCount);
}
+
+ @RestrictedApi(explanation = "Should only be called in tests", link = "",
+ allowedOnPath = ".*/src/test/.*")
+ boolean isReadsEnabled() {
+ return this.writestate.readsEnabled;
+ }
}
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
index 85a5f22..810ff66 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
@@ -2600,7 +2600,13 @@ public class HRegionServer extends Thread implements
}
TableName tn = region.getTableDescriptor().getTableName();
if (!ServerRegionReplicaUtil.isRegionReplicaReplicationEnabled(region.conf, tn) ||
- !ServerRegionReplicaUtil.isRegionReplicaWaitForPrimaryFlushEnabled(region.conf)) {
+ !ServerRegionReplicaUtil.isRegionReplicaWaitForPrimaryFlushEnabled(region.conf) ||
+ // If the memstore replication not setup, we do not have to wait for observing a flush event
+ // from primary before starting to serve reads, because gaps from replication is not
+ // applicable,this logic is from
+ // TableDescriptorBuilder.ModifyableTableDescriptor.setRegionMemStoreReplication by
+ // HBASE-13063
+ !region.getTableDescriptor().hasRegionMemStoreReplication()) {
region.setReadsEnabled(true);
return;
}
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionReplicaWaitForPrimaryFlushConf.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionReplicaWaitForPrimaryFlushConf.java
new file mode 100644
index 0000000..566b2de
--- /dev/null
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionReplicaWaitForPrimaryFlushConf.java
@@ -0,0 +1,125 @@
+/**
+ * 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.regionserver;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.StartMiniClusterOption;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.TableNameTestRule;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
+import org.apache.hadoop.hbase.client.TableDescriptor;
+import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
+import org.apache.hadoop.hbase.executor.ExecutorType;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.testclassification.RegionServerTests;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.Pair;
+import org.apache.hadoop.hbase.util.ServerRegionReplicaUtil;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+@Category({ RegionServerTests.class, MediumTests.class })
+public class TestRegionReplicaWaitForPrimaryFlushConf {
+ @ClassRule
+ public static final HBaseClassTestRule CLASS_RULE =
+ HBaseClassTestRule.forClass(TestRegionReplicaWaitForPrimaryFlushConf.class);
+
+ private static final byte[] FAMILY = Bytes.toBytes("family_test");
+
+ private TableName tableName;
+
+ @Rule
+ public final TableNameTestRule name = new TableNameTestRule();
+ private static final HBaseTestingUtility HTU = new HBaseTestingUtility();
+
+ @BeforeClass
+ public static void setUpBeforeClass() throws Exception {
+ Configuration conf = HTU.getConfiguration();
+ conf.setBoolean(ServerRegionReplicaUtil.REGION_REPLICA_REPLICATION_CONF_KEY, true);
+ conf.setBoolean(ServerRegionReplicaUtil.REGION_REPLICA_WAIT_FOR_PRIMARY_FLUSH_CONF_KEY, false);
+ HTU.startMiniCluster(StartMiniClusterOption.builder().numRegionServers(2).build());
+
+ }
+
+ @AfterClass
+ public static void tearDownAfterClass() throws Exception {
+ HTU.shutdownMiniCluster();
+ }
+
+ /**
+ * This test is for HBASE-26811,before HBASE-26811,when
+ * {@link ServerRegionReplicaUtil#REGION_REPLICA_WAIT_FOR_PRIMARY_FLUSH_CONF_KEY} is false and set
+ * {@link TableDescriptorBuilder#setRegionMemStoreReplication} to true explicitly,the secondary
+ * replica would be disabled for read after open,after HBASE-26811,the secondary replica would be
+ * enabled for read after open.
+ */
+ @Test
+ public void testSecondaryReplicaReadEnabled() throws Exception {
+ tableName = name.getTableName();
+ TableDescriptor tableDescriptor = TableDescriptorBuilder.newBuilder(tableName)
+ .setRegionReplication(2).setColumnFamily(ColumnFamilyDescriptorBuilder.of(FAMILY))
+ .setRegionMemStoreReplication(true).build();
+ HTU.getAdmin().createTable(tableDescriptor);
+
+ final ArrayList<Pair<HRegion, HRegionServer>> regionAndRegionServers =
+ new ArrayList<Pair<HRegion, HRegionServer>>(Arrays.asList(null, null));
+
+ for (int i = 0; i < 2; i++) {
+ HRegionServer rs = HTU.getMiniHBaseCluster().getRegionServer(i);
+ List<HRegion> onlineRegions = rs.getRegions(tableName);
+ for (HRegion region : onlineRegions) {
+ int replicaId = region.getRegionInfo().getReplicaId();
+ assertNull(regionAndRegionServers.get(replicaId));
+ regionAndRegionServers.set(replicaId, new Pair<HRegion, HRegionServer>(region, rs));
+ }
+ }
+ for (Pair<HRegion, HRegionServer> pair : regionAndRegionServers) {
+ assertNotNull(pair);
+ }
+
+ HRegionServer secondaryRs = regionAndRegionServers.get(1).getSecond();
+
+ try {
+ secondaryRs.getExecutorService()
+ .getExecutorThreadPool(ExecutorType.RS_REGION_REPLICA_FLUSH_OPS);
+ fail();
+ } catch (NullPointerException e) {
+ assertTrue(e != null);
+ }
+ HRegion secondaryRegion = regionAndRegionServers.get(1).getFirst();
+ assertFalse(
+ ServerRegionReplicaUtil.isRegionReplicaWaitForPrimaryFlushEnabled(secondaryRegion.conf));
+ assertTrue(secondaryRegion.isReadsEnabled());
+ }
+
+}