This is an automated email from the ASF dual-hosted git repository. andor pushed a commit to branch HBASE-29081_rebased in repository https://gitbox.apache.org/repos/asf/hbase.git
commit 0a77a329f841222cc6046d1601a4b17add8e38f1 Author: Anuj Sharma <[email protected]> AuthorDate: Mon Oct 27 21:39:36 2025 +0530 HBASE-29611: With FILE based SFT, the list of HFiles we maintain in .filelist is n… (#7361) * HBASE-29611: With FILE based SFT, the list of HFiles we maintain in .filelist is not getting updated for read replica Link to JIRA: https://issues.apache.org/jira/browse/HBASE-29611 Description: Steps to Repro (For detailed steps, check JIRA): - Create two clusters on the same storage location. - Create table on active, then refresh meta on the read replica to get the table meta data updated. - Add some rows and flush on the active cluster, do refresh_hfiles on the read replica and scan table. - If you now again add the rows in the table on active and do refresh_hfiles then the rows added are not visible in the read replica. Cause: The refresh store file is a two step process: 1. Load the existing store file from the .filelist (choose the file with higher timestamp for loading) 2. refresh store file internals (clean up old/compacted files, replace store file in .filelist) In the current scenario, what is happening is that for the first time read-replica is loading the list of Hfiles from the file in .filelist created by active cluster but then it is creating the new file with greater timestamp. Now we have two files in .filelist. On the subsequent flush from active the file in .filelist created by the active gets updated but the file created by read-replica is not. While loading in the refresh_hfiles as we take the file with higher timestamp the file c [...] Fix: As we just wanted the file from active to be loaded anytime we perform refresh store files, we must not create a new file in the .filelist from the read-replica, in this way we will stop the timestamp mismatch. NOTE: Also we don't want to initialize the tracker file (StoreFileListFile.java:load()) from read-replica as we are not writing it hence we have added check for read only property in StoreFileTrackerBase.java:load() * Make read only cluster to behave like secondary replica --- .../storefiletracker/StoreFileTrackerBase.java | 19 ++- .../DummyStoreFileTrackerForReadOnlyMode.java | 79 +++++++++++ .../TestStoreFileTrackerBaseReadOnlyMode.java | 146 +++++++++++++++++++++ 3 files changed, 238 insertions(+), 6 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerBase.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerBase.java index 87eca7b93c9..97389c392c1 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerBase.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerBase.java @@ -32,6 +32,7 @@ import org.apache.hadoop.fs.FSDataOutputStream; import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.backup.HFileArchiver; import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor; @@ -85,12 +86,12 @@ abstract class StoreFileTrackerBase implements StoreFileTracker { @Override public final List<StoreFileInfo> load() throws IOException { - return doLoadStoreFiles(!isPrimaryReplica); + return doLoadStoreFiles(!isPrimaryReplica || isReadOnlyEnabled()); } @Override public final void add(Collection<StoreFileInfo> newFiles) throws IOException { - if (isPrimaryReplica) { + if (isPrimaryReplica && !isReadOnlyEnabled()) { doAddNewStoreFiles(newFiles); } } @@ -98,14 +99,14 @@ abstract class StoreFileTrackerBase implements StoreFileTracker { @Override public final void replace(Collection<StoreFileInfo> compactedFiles, Collection<StoreFileInfo> newFiles) throws IOException { - if (isPrimaryReplica) { + if (isPrimaryReplica && !isReadOnlyEnabled()) { doAddCompactionResults(compactedFiles, newFiles); } } @Override public final void set(List<StoreFileInfo> files) throws IOException { - if (isPrimaryReplica) { + if (isPrimaryReplica && !isReadOnlyEnabled()) { doSetStoreFiles(files); } } @@ -140,8 +141,9 @@ abstract class StoreFileTrackerBase implements StoreFileTracker { @Override public final StoreFileWriter createWriter(CreateStoreFileWriterParams params) throws IOException { - if (!isPrimaryReplica) { - throw new IllegalStateException("Should not call create writer on secondary replicas"); + if (!isPrimaryReplica || isReadOnlyEnabled()) { + throw new IllegalStateException( + "Should not call create writer on secondary replicas or in read only mode"); } // creating new cache config for each new writer final CacheConfig cacheConf = ctx.getCacheConf(); @@ -390,6 +392,11 @@ abstract class StoreFileTrackerBase implements StoreFileTracker { storeFiles); } + private boolean isReadOnlyEnabled() { + return conf.getBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY, + HConstants.HBASE_GLOBAL_READONLY_ENABLED_DEFAULT); + } + /** * For primary replica, we will call load once when opening a region, and the implementation could * choose to do some cleanup work. So here we use {@code readOnly} to indicate that whether you diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/DummyStoreFileTrackerForReadOnlyMode.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/DummyStoreFileTrackerForReadOnlyMode.java new file mode 100644 index 00000000000..5f8268b3ac7 --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/DummyStoreFileTrackerForReadOnlyMode.java @@ -0,0 +1,79 @@ +/* + * 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.storefiletracker; + +import java.io.IOException; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.regionserver.StoreFileInfo; + +public class DummyStoreFileTrackerForReadOnlyMode extends StoreFileTrackerBase { + private boolean readOnlyUsed = false; + private boolean compactionExecuted = false; + private boolean addExecuted = false; + private boolean setExecuted = false; + + public DummyStoreFileTrackerForReadOnlyMode(Configuration conf, boolean isPrimaryReplica) { + super(conf, isPrimaryReplica, null); + } + + @Override + protected void doAddCompactionResults(Collection<StoreFileInfo> compactedFiles, + Collection<StoreFileInfo> newFiles) { + compactionExecuted = true; + } + + @Override + protected void doSetStoreFiles(Collection<StoreFileInfo> files) throws IOException { + setExecuted = true; + } + + @Override + protected List<StoreFileInfo> doLoadStoreFiles(boolean readOnly) { + readOnlyUsed = readOnly; + return Collections.emptyList(); + } + + @Override + protected void doAddNewStoreFiles(Collection<StoreFileInfo> newFiles) throws IOException { + addExecuted = true; + } + + boolean wasReadOnlyLoad() { + return readOnlyUsed; + } + + boolean wasCompactionExecuted() { + return compactionExecuted; + } + + boolean wasAddExecuted() { + return addExecuted; + } + + boolean wasSetExecuted() { + return setExecuted; + } + + @Override + public boolean requireWritingToTmpDirFirst() { + return false; + } +} diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestStoreFileTrackerBaseReadOnlyMode.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestStoreFileTrackerBaseReadOnlyMode.java new file mode 100644 index 00000000000..8a1cee55832 --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/storefiletracker/TestStoreFileTrackerBaseReadOnlyMode.java @@ -0,0 +1,146 @@ +/* + * 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.storefiletracker; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.util.Collections; +import org.apache.hadoop.hbase.HBaseClassTestRule; +import org.apache.hadoop.hbase.TestRefreshHFilesBase; +import org.apache.hadoop.hbase.master.procedure.TestRefreshHFilesProcedureWithReadOnlyConf; +import org.apache.hadoop.hbase.regionserver.CreateStoreFileWriterParams; +import org.apache.hadoop.hbase.testclassification.RegionServerTests; +import org.apache.hadoop.hbase.testclassification.SmallTests; +import org.junit.After; +import org.junit.Before; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +@Category({ RegionServerTests.class, SmallTests.class }) +public class TestStoreFileTrackerBaseReadOnlyMode extends TestRefreshHFilesBase { + private DummyStoreFileTrackerForReadOnlyMode tracker; + + @ClassRule + public static final HBaseClassTestRule CLASS_RULE = + HBaseClassTestRule.forClass(TestRefreshHFilesProcedureWithReadOnlyConf.class); + + @Before + public void setup() throws Exception { + // When true is passed only setup for readonly property is done. + // The initial ReadOnly property will be false for table creation + baseSetup(true); + } + + @After + public void tearDown() throws Exception { + baseTearDown(); + } + + @Test + public void testLoadReadOnlyWhenGlobalReadOnlyEnabled() throws Exception { + try { + setReadOnlyMode(true); + tracker = new DummyStoreFileTrackerForReadOnlyMode(conf, true); + tracker.load(); + assertTrue("Tracker should be in read-only mode", tracker.wasReadOnlyLoad()); + } catch (Exception e) { + throw new RuntimeException(e); + } finally { + setReadOnlyMode(false); + } + } + + @Test + public void testReplaceSkippedWhenGlobalReadOnlyEnabled() throws Exception { + try { + setReadOnlyMode(true); + tracker = new DummyStoreFileTrackerForReadOnlyMode(conf, true); + tracker.replace(Collections.emptyList(), Collections.emptyList()); + assertFalse("Compaction should not be executed in readonly mode", + tracker.wasCompactionExecuted()); + } catch (Exception e) { + throw new RuntimeException(e); + } finally { + setReadOnlyMode(false); + } + } + + @Test + public void testReplaceExecutedWhenWritable() throws Exception { + tracker = new DummyStoreFileTrackerForReadOnlyMode(conf, true); + tracker.replace(Collections.emptyList(), Collections.emptyList()); + assertTrue("Compaction should run when not readonly", tracker.wasCompactionExecuted()); + } + + @Test + public void testAddSkippedWhenGlobalReadOnlyEnabled() throws Exception { + try { + setReadOnlyMode(true); + tracker = new DummyStoreFileTrackerForReadOnlyMode(conf, true); + tracker.add(Collections.emptyList()); + assertFalse("Add should not be executed in readonly mode", tracker.wasAddExecuted()); + } catch (Exception e) { + throw new RuntimeException(e); + } finally { + setReadOnlyMode(false); + } + } + + @Test + public void testAddExecutedWhenWritable() throws Exception { + tracker = new DummyStoreFileTrackerForReadOnlyMode(conf, true); + tracker.add(Collections.emptyList()); + assertTrue("Add should run when not readonly", tracker.wasAddExecuted()); + } + + @Test + public void testSetSkippedWhenGlobalReadOnlyEnabled() throws Exception { + try { + setReadOnlyMode(true); + tracker = new DummyStoreFileTrackerForReadOnlyMode(conf, true); + tracker.set(Collections.emptyList()); + assertFalse("Set should not be executed in readonly mode", tracker.wasSetExecuted()); + } catch (Exception e) { + throw new RuntimeException(e); + } finally { + setReadOnlyMode(false); + } + } + + @Test + public void testSetExecutedWhenWritable() throws Exception { + tracker = new DummyStoreFileTrackerForReadOnlyMode(conf, true); + tracker.set(Collections.emptyList()); + assertTrue("Set should run when not readonly", tracker.wasSetExecuted()); + } + + @Test(expected = IllegalStateException.class) + public void testCreateWriterThrowExceptionWhenGlobalReadOnlyEnabled() throws Exception { + try { + setReadOnlyMode(true); + tracker = new DummyStoreFileTrackerForReadOnlyMode(conf, true); + CreateStoreFileWriterParams params = CreateStoreFileWriterParams.create().maxKeyCount(4) + .isCompaction(false).includeMVCCReadpoint(true).includesTag(false).shouldDropBehind(false); + tracker.createWriter(params); + } finally { + setReadOnlyMode(false); + } + } +}
