From 805c346e54bffbed2e7c80029855fcabc69be03f Mon Sep 17 00:00:00 2001 From: chenglei Date: Thu, 31 Mar 2022 21:46:20 +0800 Subject: [PATCH] HBASE-26811 Secondary replica may be disabled for read forever (#4182) --- .../hbase/client/TableDescriptorBuilder.java | 6 +- .../hadoop/hbase/regionserver/HRegion.java | 7 + .../hbase/regionserver/HRegionServer.java | 8 +- ...tRegionReplicaWaitForPrimaryFlushConf.java | 124 ++++++++++++++++++ 4 files changed, 139 insertions(+), 6 deletions(-) create mode 100644 hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionReplicaWaitForPrimaryFlushConf.java 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 0128458645c..7dbd98650af 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 @@ -1335,11 +1335,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 538264b87bf..cf65859356c 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; @@ -8750,4 +8751,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 57173d71884..c56157e6a0e 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 @@ -2240,7 +2240,13 @@ public class HRegionServer extends HBaseServerBase } 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 00000000000..efbd73aed43 --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionReplicaWaitForPrimaryFlushConf.java @@ -0,0 +1,124 @@ +/** + * 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.HBaseTestingUtil; +import org.apache.hadoop.hbase.StartTestingClusterOption; +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 HBaseTestingUtil HTU = new HBaseTestingUtil(); + + @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(StartTestingClusterOption.builder().numRegionServers(2).build()); + + } + + @AfterClass + public static void tearDownAfterClass() throws Exception { + HTU.shutdownMiniCluster(); + } + + /** + * This test is for 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. + */ + @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> regionAndRegionServers = + new ArrayList>(Arrays.asList(null, null)); + + for (int i = 0; i < 2; i++) { + HRegionServer rs = HTU.getMiniHBaseCluster().getRegionServer(i); + List onlineRegions = rs.getRegions(tableName); + for (HRegion region : onlineRegions) { + int replicaId = region.getRegionInfo().getReplicaId(); + assertNull(regionAndRegionServers.get(replicaId)); + regionAndRegionServers.set(replicaId, new Pair(region, rs)); + } + } + for (Pair 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()); + } + +}