This is an automated email from the ASF dual-hosted git repository.
smiklosovic pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/trunk by this push:
new f91655df06 When a custom disk error handler fails to initiate, fail
the startup of a node instead of using the no-op handler
f91655df06 is described below
commit f91655df061bea988e12a2d9f2e438b7d825ce13
Author: Stefan Miklosovic <smikloso...@apache.org>
AuthorDate: Sun May 4 22:39:30 2025 +0200
When a custom disk error handler fails to initiate, fail the startup of a
node instead of using the no-op handler
patch by Stefan Miklosovic; reviewed by Caleb Rackliffe for CASSANDRA-20614
---
CHANGES.txt | 1 +
.../cassandra/service/DiskErrorsHandler.java | 5 +-
.../service/DiskErrorsHandlerService.java | 4 +-
.../cassandra/service/DiskErrorsHandlerTest.java | 135 +++++++++------------
4 files changed, 64 insertions(+), 81 deletions(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index 80a9d636f6..77d5b4eb71 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
5.1
+ * When a custom disk error handler fails to initiate, fail the startup of a
node instead of using the no-op handler (CASSANDRA-20614)
* Rewrite constraint framework to remove column specification from constraint
definition, introduce SQL-like NOT NULL (CASSANDRA-20563)
* Fix a bug in AutoRepair duration metric calculation if schedule finishes
quickly (CASSANDRA-20622)
* Fix AutoRepair flaky InJvm dtest (CASSANDRA-20620)
diff --git a/src/java/org/apache/cassandra/service/DiskErrorsHandler.java
b/src/java/org/apache/cassandra/service/DiskErrorsHandler.java
index b4fe9d67db..14add63fee 100644
--- a/src/java/org/apache/cassandra/service/DiskErrorsHandler.java
+++ b/src/java/org/apache/cassandra/service/DiskErrorsHandler.java
@@ -18,8 +18,6 @@
package org.apache.cassandra.service;
-import com.google.common.annotations.VisibleForTesting;
-
import org.apache.cassandra.io.FSError;
import org.apache.cassandra.io.sstable.CorruptSSTableException;
@@ -43,8 +41,7 @@ public interface DiskErrorsHandler extends AutoCloseable
{
public static final DiskErrorsHandler NO_OP = new
NoOpDiskErrorHandler();
- @VisibleForTesting
- NoOpDiskErrorHandler() {}
+ private NoOpDiskErrorHandler() {}
@Override
public void inspectCommitLogError(Throwable t) {}
diff --git
a/src/java/org/apache/cassandra/service/DiskErrorsHandlerService.java
b/src/java/org/apache/cassandra/service/DiskErrorsHandlerService.java
index 97e7ecde5f..98fb7ee609 100644
--- a/src/java/org/apache/cassandra/service/DiskErrorsHandlerService.java
+++ b/src/java/org/apache/cassandra/service/DiskErrorsHandlerService.java
@@ -35,7 +35,7 @@ public class DiskErrorsHandlerService
private static volatile DiskErrorsHandler instance = NO_OP;
@VisibleForTesting
- public static synchronized void set(DiskErrorsHandler newInstance)
+ public static synchronized void set(DiskErrorsHandler newInstance) throws
ConfigurationException
{
if (newInstance == null)
return;
@@ -58,7 +58,7 @@ public class DiskErrorsHandlerService
}
catch (Throwable t)
{
- logger.warn("Exception occured while initializing disk error
handler of class " + newInstance.getClass().getName(), t);
+ throw new ConfigurationException("Exception occured while
initializing disk error handler of class " + newInstance.getClass().getName(),
t);
}
}
diff --git a/test/unit/org/apache/cassandra/service/DiskErrorsHandlerTest.java
b/test/unit/org/apache/cassandra/service/DiskErrorsHandlerTest.java
index 6465164fe0..c8638754af 100644
--- a/test/unit/org/apache/cassandra/service/DiskErrorsHandlerTest.java
+++ b/test/unit/org/apache/cassandra/service/DiskErrorsHandlerTest.java
@@ -21,11 +21,13 @@ package org.apache.cassandra.service;
import org.junit.Test;
import org.apache.cassandra.distributed.shared.WithProperties;
+import org.apache.cassandra.exceptions.ConfigurationException;
import org.apache.cassandra.io.FSError;
import org.apache.cassandra.io.sstable.CorruptSSTableException;
import static
org.apache.cassandra.config.CassandraRelevantProperties.CUSTOM_DISK_ERROR_HANDLER;
import static org.apache.cassandra.service.DiskErrorsHandlerService.get;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
@@ -35,17 +37,16 @@ public class DiskErrorsHandlerTest
@Test
public void testSetting() throws Throwable
{
+ DiskErrorsHandler handlerA;
+ DiskErrorsHandler handlerB;
try (WithProperties ignore = new
WithProperties().set(CUSTOM_DISK_ERROR_HANDLER,
HandlerA.class.getName()))
{
DiskErrorsHandlerService.configure();
-
- assertSame(HandlerA.class, get().getClass());
-
- assertTrue(HandlerA.initialized);
- assertFalse(HandlerA.closed);
- assertFalse(HandlerB.initialized);
- assertFalse(HandlerB.closed);
+ handlerA = get();
+ assertSame(HandlerA.class, handlerA.getClass());
+ assertInitialized(HandlerA.class, handlerA);
+ assertNotClosed(HandlerA.class, handlerA);
}
try (WithProperties ignore = new
WithProperties().set(CUSTOM_DISK_ERROR_HANDLER,
@@ -53,96 +54,73 @@ public class DiskErrorsHandlerTest
{
DiskErrorsHandlerService.configure();
- assertTrue(HandlerA.initialized);
- assertTrue(HandlerA.closed);
+ handlerB = get();
+ assertSame(HandlerB.class, handlerB.getClass());
- assertTrue(HandlerB.initialized);
- assertFalse(HandlerB.closed);
+ assertInitialized(HandlerA.class, handlerA);
+ assertClosed(HandlerA.class, handlerA);
- assertSame(HandlerB.class, get().getClass());
+ assertInitialized(HandlerB.class, handlerB);
+ assertNotClosed(HandlerB.class, handlerB);
- get().close();
+ handlerB.close();
- assertTrue(HandlerB.closed);
+ assertClosed(HandlerB.class, handlerB);
}
}
@Test
public void testFailures()
{
- // failed closing
+ DiskErrorsHandler handlerC;
try (WithProperties ignore = new
WithProperties().set(CUSTOM_DISK_ERROR_HANDLER,
HandlerC.class.getName()))
{
DiskErrorsHandlerService.configure();
- assertTrue(HandlerC.initialized);
- assertSame(HandlerC.class, get().getClass());
+ handlerC = get();
+ assertInitialized(HandlerC.class, handlerC);
}
- // this will call close() on C handler
+ DiskErrorsHandler handlerA;
+ // this will call _not_ close() on C handler
try (WithProperties ignore = new
WithProperties().set(CUSTOM_DISK_ERROR_HANDLER,
-
HandlerE.class.getName()))
+
HandlerA.class.getName()))
{
DiskErrorsHandlerService.configure();
- assertTrue(HandlerE.initialized);
- assertSame(HandlerE.class, get().getClass());
+ handlerA = get();
+ assertInitialized(HandlerA.class, handlerA);
+ assertNotClosed(HandlerC.class, handlerC);
}
try (WithProperties ignore = new
WithProperties().set(CUSTOM_DISK_ERROR_HANDLER,
HandlerD.class.getName()))
{
- DiskErrorsHandlerService.configure();
- // still handler E as handler D failed to init
- assertSame(HandlerE.class, get().getClass());
- }
- }
+ assertThatThrownBy(DiskErrorsHandlerService::configure)
+ .isInstanceOf(ConfigurationException.class);
- public static class HandlerA extends DummyErrorHandler
- {
- public static boolean initialized = false;
- public static boolean closed = false;
-
- @Override
- public void init()
- {
- initialized = true;
+ assertSame(HandlerA.class, get().getClass());
+ // still handler A as handler D failed to init
+ assertInitialized(HandlerA.class, handlerA);
+ assertNotClosed(HandlerA.class, handlerA);
}
- @Override
- public void close() throws Exception
+ // what if a user tries to set no-op handler or handler which can not
be constructed (constructor is private)
+ try (WithProperties ignore = new
WithProperties().set(CUSTOM_DISK_ERROR_HANDLER,
+
DiskErrorsHandler.NoOpDiskErrorHandler.class.getName()))
{
- closed = true;
+ assertThatThrownBy(DiskErrorsHandlerService::configure)
+ .isInstanceOf(ConfigurationException.class)
+ .hasMessageContaining("Default constructor for disk error handler
class " +
+ '\'' +
DiskErrorsHandler.NoOpDiskErrorHandler.class.getName() + "' is inaccessible.");
}
}
- public static class HandlerB extends DummyErrorHandler
- {
- public static boolean initialized = false;
- public static boolean closed = false;
-
- @Override
- public void init()
- {
- initialized = true;
- }
+ public static class HandlerA extends DummyErrorHandler {}
- @Override
- public void close() throws Exception
- {
- closed = true;
- }
- }
+ public static class HandlerB extends DummyErrorHandler {}
public static class HandlerC extends DummyErrorHandler
{
- public static boolean initialized = false;
-
- @Override
- public void init()
- {
- initialized = true;
- }
-
@Override
public void close() throws Exception
{
@@ -152,25 +130,35 @@ public class DiskErrorsHandlerTest
public static class HandlerD extends DummyErrorHandler
{
- public static boolean closed = false;
-
@Override
public void init()
{
throw new RuntimeException("failed to init");
}
+ }
- @Override
- public void close() throws Exception
- {
- closed = true;
- }
+ public void assertClosed(Class<?> handlerClass, DiskErrorsHandler
diskErrorsHandler)
+ {
+ assertSame(handlerClass, diskErrorsHandler.getClass());
+ assertTrue(((DummyErrorHandler) diskErrorsHandler).closed);
+ }
+
+ public void assertNotClosed(Class<?> handlerClass, DiskErrorsHandler
diskErrorsHandler)
+ {
+ assertSame(handlerClass, diskErrorsHandler.getClass());
+ assertFalse(((DummyErrorHandler) diskErrorsHandler).closed);
}
- public static class HandlerE extends DummyErrorHandler
+ public void assertInitialized(Class<?> handlerClass, DiskErrorsHandler
diskErrorsHandler)
{
- public static boolean initialized = false;
- public static boolean closed = false;
+ assertSame(handlerClass, diskErrorsHandler.getClass());
+ assertTrue(((DummyErrorHandler) diskErrorsHandler).initialized);
+ }
+
+ private static abstract class DummyErrorHandler implements
DiskErrorsHandler
+ {
+ public boolean initialized = false;
+ public boolean closed = false;
@Override
public void init()
@@ -183,10 +171,7 @@ public class DiskErrorsHandlerTest
{
closed = true;
}
- }
- private static abstract class DummyErrorHandler implements
DiskErrorsHandler
- {
@Override
public void handleCorruptSSTable(CorruptSSTableException e)
{
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
