anmolnar commented on code in PR #8377:
URL: https://github.com/apache/hbase/pull/8377#discussion_r3582375880


##########
hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestReadOnlyManageActiveClusterFile.java:
##########
@@ -146,4 +166,176 @@ public void 
testDoNotDeleteActiveClusterIdFileWhenSwitchingToReadOnlyIfNotOwnedB
     // switching to readonly mode
     assertTrue(activeClusterIdFileExists());
   }
+
+  @Test
+  public void testCannotDisableReadOnlyWhenAnotherClusterIsActive() throws 
Exception {
+    // First enable read-only mode (simulating a replica cluster)
+    setReadOnlyMode(true);
+    assertFalse(activeClusterIdFileExists());
+
+    // Now write an active cluster file with a DIFFERENT cluster's data 
(simulating another active
+    // cluster owning the storage)
+    overwriteExistingFile();
+    assertTrue(activeClusterIdFileExists());
+
+    // Attempt to disable read-only mode, but get an exception because another 
active cluster
+    // already exists.
+    
master.getConfiguration().setBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY,
 false);
+
+    // Master's updateConfiguration should throw because another cluster is 
active
+    assertThrows(ReadOnlyTransitionException.class, () -> 
master.updateConfiguration());
+
+    // Verify read-only coprocessors are still loaded on the master
+    
assertTrue(CoprocessorConfigurationUtil.areReadOnlyCoprocessorsLoaded(master.getConfiguration(),
+      CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY));
+
+    // Verify read-only coprocessors are still loaded on the region server
+    assertTrue(CoprocessorConfigurationUtil.areReadOnlyCoprocessorsLoaded(
+      regionServer.getConfiguration(), 
CoprocessorHost.REGIONSERVER_COPROCESSOR_CONF_KEY));
+  }
+
+  @Test
+  public void testCanDisableReadOnlyWhenOwnClusterIsActive() throws Exception {
+    // Enable read-only mode
+    setReadOnlyMode(true);
+    assertFalse(activeClusterIdFileExists());
+
+    // Verify read-only coprocessors are loaded
+    
assertTrue(CoprocessorConfigurationUtil.areReadOnlyCoprocessorsLoaded(master.getConfiguration(),
+      CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY));
+
+    // Disable read-only mode (our own cluster, no conflicting active cluster 
file)
+    setReadOnlyMode(false);
+
+    // Active cluster file should be recreated
+    assertTrue(activeClusterIdFileExists());
+
+    // Verify read-only coprocessors are removed
+    assertFalse(CoprocessorConfigurationUtil.areReadOnlyCoprocessorsLoaded(
+      master.getConfiguration(), CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY));
+  }
+
+  @Test
+  public void testRegionCoprocessorsStillLoadWhenReadOnlyTransitionBlocked() 
throws Exception {
+    // Create a table so we have a region to work with
+    TableName tableName = TableName.valueOf("testCoprocessorLoadTable");
+    TableDescriptor desc = TableDescriptorBuilder.newBuilder(tableName)
+      .setColumnFamily(ColumnFamilyDescriptorBuilder.of("cf")).build();
+    TEST_UTIL.getAdmin().createTable(desc);
+    List<HRegion> regions = regionServer.getRegions(tableName);
+    assertFalse(regions.isEmpty());
+    HRegion region = regions.get(0);
+
+    // Enable read-only mode to load ReadOnly coprocessors on the region
+    Configuration readOnlyConf = new Configuration(conf);
+    readOnlyConf.setBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY, 
true);
+    region.onConfigurationChange(readOnlyConf);
+
+    // Verify ReadOnly coprocessors are loaded
+    RegionCoprocessorHost regionCPHost = region.getCoprocessorHost();
+    
assertNotNull(regionCPHost.findCoprocessor(RegionReadOnlyController.class.getName()),
+      "RegionReadOnlyController should be loaded after enabling read-only 
mode");
+
+    // Simulate another active cluster by writing a foreign cluster ID to the 
active cluster file
+    overwriteExistingFile();
+    assertTrue(activeClusterIdFileExists());
+
+    // Build a config that attempts to disable read-only AND adds new 
coprocessors
+    Configuration newConf = new Configuration(conf);
+    newConf.setBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY, false);
+
+    // Add a system and user region coprocessors
+    newConf.set(CoprocessorHost.REGION_COPROCESSOR_CONF_KEY, 
SimpleRegionObserver.class.getName());
+    newConf.set(CoprocessorHost.USER_REGION_COPROCESSOR_CONF_KEY,
+      NoOpScanPolicyObserver.class.getName());
+
+    // Trigger dynamic configuration change on the region
+    region.onConfigurationChange(newConf);
+
+    // Verify ReadOnly coprocessors are still loaded since the read-only 
transition was blocked
+    regionCPHost = region.getCoprocessorHost();
+    assertTrue(
+      
CoprocessorConfigurationUtil.areReadOnlyCoprocessorsLoaded(region.getConfiguration(),
+        CoprocessorHost.REGION_COPROCESSOR_CONF_KEY),
+      "ReadOnly coprocessors should remain in the region configuration");
+
+    // Verify new system and user coprocessors were loaded despite the blocked 
read-only transition
+    
assertNotNull(regionCPHost.findCoprocessor(SimpleRegionObserver.class.getName()),
+      "SimpleRegionObserver should be loaded even when read-only transition is 
blocked");
+    
assertNotNull(regionCPHost.findCoprocessor(NoOpScanPolicyObserver.class.getName()),
+      "NoOpScanPolicyObserver should be loaded even when read-only transition 
is blocked");
+  }
+
+  @Test
+  public void 
testRegionServerCannotDisableReadOnlyWhenAnotherClusterIsActive() throws 
Exception {
+    // First enable read-only mode
+    setReadOnlyMode(true);
+    assertFalse(activeClusterIdFileExists());
+
+    // Write an active cluster file with a different cluster's data
+    overwriteExistingFile();
+    assertTrue(activeClusterIdFileExists());
+
+    // Attempt to disable read-only mode, but get an exception because another 
active cluster
+    // already exists
+    
regionServer.getConfiguration().setBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY,
 false);
+
+    // RegionServer's updateConfiguration should throw because another cluster 
is active
+    assertThrows(ReadOnlyTransitionException.class, () -> 
regionServer.updateConfiguration());
+
+    // Verify read-only coprocessors are still loaded on the region server
+    assertTrue(CoprocessorConfigurationUtil.areReadOnlyCoprocessorsLoaded(
+      regionServer.getConfiguration(), 
CoprocessorHost.REGIONSERVER_COPROCESSOR_CONF_KEY));
+  }
+
+  @Test
+  public void testBlockedThenSuccessfulReadOnlyTransition() throws Exception {
+    // Put cluster in read-only mode with a foreign active cluster file
+    setReadOnlyMode(true);
+    assertFalse(activeClusterIdFileExists());
+    overwriteExistingFile();
+    assertTrue(activeClusterIdFileExists());
+
+    // Attempt to disable read-only mode. This should fail because another 
cluster is active.
+    // We need to run updateConfiguration() to ensure the 
readOnlyTransitionBlocked boolean in
+    // HBaseServerBase gets set.
+    
master.getConfiguration().setBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY,
 false);
+    assertThrows(ReadOnlyTransitionException.class, () -> 
master.updateConfiguration());
+    
regionServer.getConfiguration().setBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY,
 false);
+    assertThrows(ReadOnlyTransitionException.class, () -> 
regionServer.updateConfiguration());
+
+    // Try to create a table. This should fail because the cluster is still in 
read-only mode
+    TableName tableName = TableName.valueOf("testBlockedTransitionTable");
+    TableDescriptor desc = TableDescriptorBuilder.newBuilder(tableName)
+      .setColumnFamily(ColumnFamilyDescriptorBuilder.of("cf")).build();
+    assertThrows(IOException.class, () -> 
TEST_UTIL.getAdmin().createTable(desc));
+
+    // Delete the foreign active cluster file
+    fs.delete(activeClusterFile, false);
+    assertFalse(activeClusterIdFileExists());

Review Comment:
   Whichever is already present in the active suffix file.
   
   @kgeisz would you like to improve docs as part of this patch?



-- 
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]

Reply via email to