Skip to content

Commit 28808cc

Browse files
authored
HBASE-30301: Flush command hangs instead of failing for read-only clusters (#8503)
Co-authored-by: Claude Code Opus 4.6 <noreply@anthropic.com> Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org> Signed-off-by: Tak Lon (Stephen) Wu <taklwu@apache.org>
1 parent 1de0aef commit 28808cc

5 files changed

Lines changed: 181 additions & 24 deletions

File tree

hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/MasterReadOnlyController.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,13 @@ public void preMasterStoreFlush(ObserverContext<MasterCoprocessorEnvironment> ct
262262
MasterObserver.super.preMasterStoreFlush(ctx);
263263
}
264264

265+
@Override
266+
public void preTableFlush(final ObserverContext<MasterCoprocessorEnvironment> ctx,
267+
final TableName tableName) throws IOException {
268+
internalReadOnlyGuard();
269+
MasterObserver.super.preTableFlush(ctx, tableName);
270+
}
271+
265272
@Override
266273
public void preSetUserQuota(ObserverContext<MasterCoprocessorEnvironment> ctx, String userName,
267274
GlobalQuotaSettings quotas) throws IOException {

hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/SecureTestUtil.java

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.apache.hadoop.conf.Configuration;
3333
import org.apache.hadoop.hbase.Coprocessor;
3434
import org.apache.hadoop.hbase.HBaseTestingUtil;
35+
import org.apache.hadoop.hbase.HConstants;
3536
import org.apache.hadoop.hbase.NamespaceDescriptor;
3637
import org.apache.hadoop.hbase.SingleProcessHBaseCluster;
3738
import org.apache.hadoop.hbase.TableName;
@@ -53,9 +54,12 @@
5354
import org.apache.hadoop.hbase.coprocessor.ObserverContext;
5455
import org.apache.hadoop.hbase.io.hfile.HFile;
5556
import org.apache.hadoop.hbase.ipc.RemoteWithExtrasException;
57+
import org.apache.hadoop.hbase.master.HMaster;
5658
import org.apache.hadoop.hbase.regionserver.HRegion;
59+
import org.apache.hadoop.hbase.regionserver.HRegionServer;
5760
import org.apache.hadoop.hbase.security.AccessDeniedException;
5861
import org.apache.hadoop.hbase.security.User;
62+
import org.apache.hadoop.hbase.util.ConfigurationUtil;
5963
import org.apache.hadoop.hbase.util.JVMClusterUtil.RegionServerThread;
6064
import org.slf4j.Logger;
6165
import org.slf4j.LoggerFactory;
@@ -860,4 +864,31 @@ private static void checkPermissions(Configuration conf, Permission... perms) th
860864
}
861865
}
862866
}
867+
868+
public static void enableReadOnlyMode(Configuration conf, HMaster hMaster,
869+
HRegionServer hRegionServer) {
870+
if (!ConfigurationUtil.isReadOnlyModeEnabledInConf(conf)) {
871+
LOG.info("Dynamically enabling Read-Only mode by setting {} to true",
872+
HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY);
873+
conf.setBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY, true);
874+
notifyReadOnlyObservers(conf, hMaster, hRegionServer);
875+
}
876+
}
877+
878+
public static void disableReadOnlyMode(Configuration conf, HMaster hMaster,
879+
HRegionServer hRegionServer) {
880+
if (ConfigurationUtil.isReadOnlyModeEnabledInConf(conf)) {
881+
LOG.info("Dynamically disabling Read-Only mode by setting {} to false",
882+
HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY);
883+
conf.setBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY, false);
884+
notifyReadOnlyObservers(conf, hMaster, hRegionServer);
885+
}
886+
}
887+
888+
public static void notifyReadOnlyObservers(Configuration conf, HMaster hMaster,
889+
HRegionServer hRegionServer) {
890+
LOG.info("Notifying observers about configuration changes");
891+
hMaster.getConfigurationManager().notifyAllObservers(conf);
892+
hRegionServer.getConfigurationManager().notifyAllObservers(conf);
893+
}
863894
}

hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestReadOnlyController.java

Lines changed: 2 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -40,20 +40,16 @@
4040
import org.apache.hadoop.hbase.testclassification.LargeTests;
4141
import org.apache.hadoop.hbase.testclassification.SecurityTests;
4242
import org.apache.hadoop.hbase.util.Bytes;
43-
import org.apache.hadoop.hbase.util.ConfigurationUtil;
4443
import org.junit.jupiter.api.AfterEach;
4544
import org.junit.jupiter.api.BeforeEach;
4645
import org.junit.jupiter.api.Tag;
4746
import org.junit.jupiter.api.Test;
48-
import org.slf4j.Logger;
49-
import org.slf4j.LoggerFactory;
5047

5148
@Tag(SecurityTests.TAG)
5249
@Tag(LargeTests.TAG)
5350
@SuppressWarnings("deprecation")
5451
public class TestReadOnlyController {
5552

56-
private static final Logger LOG = LoggerFactory.getLogger(TestReadOnlyController.class);
5753
private final HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil();
5854
private static final TableName TEST_TABLE = TableName.valueOf("read_only_test_table");
5955
private static final byte[] TEST_FAMILY = Bytes.toBytes("read_only_table_col_fam");
@@ -107,29 +103,11 @@ public void afterClass() throws Exception {
107103
}
108104

109105
private static void enableReadOnlyMode() {
110-
// Dynamically enable Read-Only mode if it is not active
111-
if (!ConfigurationUtil.isReadOnlyModeEnabledInConf(conf)) {
112-
LOG.info("Dynamically enabling Read-Only mode by setting {} to true",
113-
HConstants.HBASE_GLOBAL_READONLY_ENABLED_DEFAULT);
114-
conf.setBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY, true);
115-
notifyObservers();
116-
}
106+
SecureTestUtil.enableReadOnlyMode(conf, hMaster, hRegionServer);
117107
}
118108

119109
private static void disableReadOnlyMode() {
120-
// Dynamically disable Read-Only mode if it is active
121-
if (ConfigurationUtil.isReadOnlyModeEnabledInConf(conf)) {
122-
LOG.info("Dynamically disabling Read-Only mode by setting {} to false",
123-
HConstants.HBASE_GLOBAL_READONLY_ENABLED_DEFAULT);
124-
conf.setBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY, false);
125-
notifyObservers();
126-
}
127-
}
128-
129-
private static void notifyObservers() {
130-
LOG.info("Notifying observers about configuration changes");
131-
hMaster.getConfigurationManager().notifyAllObservers(conf);
132-
hRegionServer.getConfigurationManager().notifyAllObservers(conf);
110+
SecureTestUtil.disableReadOnlyMode(conf, hMaster, hRegionServer);
133111
}
134112

135113
// The test case for successfully creating a table with Read-Only mode disabled happens when
Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
package org.apache.hadoop.hbase.security.access;
19+
20+
import static org.apache.hadoop.hbase.HConstants.HBASE_CLIENT_RETRIES_NUMBER;
21+
import static org.junit.jupiter.api.Assertions.assertThrows;
22+
import static org.junit.jupiter.api.Assertions.assertTrue;
23+
24+
import java.io.IOException;
25+
import java.util.concurrent.TimeUnit;
26+
import org.apache.hadoop.conf.Configuration;
27+
import org.apache.hadoop.hbase.HBaseTestingUtil;
28+
import org.apache.hadoop.hbase.HConstants;
29+
import org.apache.hadoop.hbase.SingleProcessHBaseCluster;
30+
import org.apache.hadoop.hbase.TableName;
31+
import org.apache.hadoop.hbase.client.Admin;
32+
import org.apache.hadoop.hbase.client.Connection;
33+
import org.apache.hadoop.hbase.client.ConnectionFactory;
34+
import org.apache.hadoop.hbase.client.Put;
35+
import org.apache.hadoop.hbase.client.Table;
36+
import org.apache.hadoop.hbase.master.HMaster;
37+
import org.apache.hadoop.hbase.regionserver.HRegionServer;
38+
import org.apache.hadoop.hbase.testclassification.LargeTests;
39+
import org.apache.hadoop.hbase.testclassification.SecurityTests;
40+
import org.apache.hadoop.hbase.util.Bytes;
41+
import org.junit.jupiter.api.AfterEach;
42+
import org.junit.jupiter.api.BeforeEach;
43+
import org.junit.jupiter.api.Tag;
44+
import org.junit.jupiter.api.Test;
45+
import org.junit.jupiter.api.Timeout;
46+
47+
@Tag(SecurityTests.TAG)
48+
@Tag(LargeTests.TAG)
49+
@SuppressWarnings("deprecation")
50+
public class TestReadOnlyControllerFlush {
51+
52+
private final HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil();
53+
private static final TableName TEST_TABLE = TableName.valueOf("read_only_flush_test_table");
54+
private static final byte[] TEST_FAMILY = Bytes.toBytes("read_only_flush_col_fam");
55+
private static HRegionServer hRegionServer;
56+
private static HMaster hMaster;
57+
private static Configuration conf;
58+
private static Connection connection;
59+
private static SingleProcessHBaseCluster cluster;
60+
61+
private static Table testTable;
62+
63+
@BeforeEach
64+
public void beforeClass() throws Exception {
65+
conf = TEST_UTIL.getConfiguration();
66+
67+
// Shorten the run time of failed unit tests by limiting retries and the session timeout
68+
// threshold
69+
conf.setInt(HBASE_CLIENT_RETRIES_NUMBER, 1);
70+
conf.setInt(HConstants.ZK_SESSION_TIMEOUT, 1000);
71+
72+
// Set up test class with Read-Only mode disabled so a table can be created
73+
conf.setBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY, false);
74+
75+
try {
76+
// Start the test cluster
77+
cluster = TEST_UTIL.startMiniCluster(1);
78+
79+
hMaster = cluster.getMaster();
80+
hRegionServer = cluster.getRegionServerThreads().get(0).getRegionServer();
81+
connection = ConnectionFactory.createConnection(conf);
82+
83+
// Create a test table and insert a row so the memstore has data to flush
84+
testTable = TEST_UTIL.createTable(TEST_TABLE, TEST_FAMILY);
85+
Put put = new Put(Bytes.toBytes("row1"));
86+
put.addColumn(TEST_FAMILY, null, Bytes.toBytes("value1"));
87+
testTable.put(put);
88+
} catch (Exception e) {
89+
disableReadOnlyMode();
90+
TEST_UTIL.deleteTable(TEST_TABLE);
91+
if (connection != null) {
92+
connection.close();
93+
}
94+
TEST_UTIL.shutdownMiniCluster();
95+
throw new RuntimeException(e);
96+
}
97+
}
98+
99+
@AfterEach
100+
public void afterClass() throws Exception {
101+
if (connection != null) {
102+
connection.close();
103+
}
104+
TEST_UTIL.shutdownMiniCluster();
105+
}
106+
107+
private static void enableReadOnlyMode() {
108+
SecureTestUtil.enableReadOnlyMode(conf, hMaster, hRegionServer);
109+
}
110+
111+
private static void disableReadOnlyMode() {
112+
SecureTestUtil.disableReadOnlyMode(conf, hMaster, hRegionServer);
113+
}
114+
115+
@Test
116+
public void testFlushTableWithReadOnlyDisabled() throws IOException {
117+
disableReadOnlyMode();
118+
try (Admin admin = TEST_UTIL.getAdmin()) {
119+
admin.flush(TEST_TABLE);
120+
}
121+
}
122+
123+
@Test
124+
@Timeout(value = 60, unit = TimeUnit.SECONDS)
125+
public void testCannotFlushTableWithReadOnlyEnabled() throws IOException {
126+
enableReadOnlyMode();
127+
try (Admin admin = TEST_UTIL.getAdmin()) {
128+
IOException exception = assertThrows(IOException.class, () -> {
129+
admin.flush(TEST_TABLE);
130+
});
131+
assertTrue(exception.getMessage().contains("Operation not allowed in Read-Only Mode"));
132+
}
133+
}
134+
}

hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestReadOnlyControllerMasterObserver.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,13 @@ public void testPreMasterStoreFlushReadOnlyException() {
318318
});
319319
}
320320

321+
@Test
322+
public void testPreTableFlushReadOnlyException() {
323+
assertThrows(WriteAttemptedOnReadOnlyClusterException.class, () -> {
324+
MasterReadOnlyController.preTableFlush(ctx, tableName);
325+
});
326+
}
327+
321328
@Test
322329
public void testPreSetUserQuotaReadOnlyException() {
323330
assertThrows(WriteAttemptedOnReadOnlyClusterException.class, () -> {

0 commit comments

Comments
 (0)