From faa5dd6392e59218a2494371b225801b749f721c Mon Sep 17 00:00:00 2001 From: garyschulte Date: Thu, 5 Oct 2023 14:18:00 -0700 Subject: [PATCH] use status.code for identifying transient exceptions rather than message parsing Signed-off-by: garyschulte --- .../storage/rocksdb/RocksDBTransaction.java | 14 +++++++------ .../rocksdb/RocksDBTransactionTest.java | 21 ++++++++++++------- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBTransaction.java b/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBTransaction.java index 03f9a5cdb2c..56e1c47d25c 100644 --- a/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBTransaction.java +++ b/plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBTransaction.java @@ -21,10 +21,12 @@ import org.hyperledger.besu.plugin.services.storage.SegmentIdentifier; import org.hyperledger.besu.plugin.services.storage.SegmentedKeyValueStorageTransaction; +import java.util.EnumSet; import java.util.function.Function; import org.rocksdb.ColumnFamilyHandle; import org.rocksdb.RocksDBException; +import org.rocksdb.Status; import org.rocksdb.Transaction; import org.rocksdb.WriteOptions; import org.slf4j.Logger; @@ -34,8 +36,6 @@ public class RocksDBTransaction implements SegmentedKeyValueStorageTransaction { private static final Logger logger = LoggerFactory.getLogger(RocksDBTransaction.class); private static final String ERR_NO_SPACE_LEFT_ON_DEVICE = "No space left on device"; - private static final String ERR_BUSY = "Busy"; - private static final String ERR_LOCK_TIMED_OUT = "TimedOut(LockTimeout)"; private static final int DEFAULT_MAX_RETRIES = 3; private final RocksDBMetrics metrics; @@ -120,6 +120,9 @@ private void close() { interface RetryableRocksDBAction { void retry() throws RocksDBException; + EnumSet RETRYABLE_STATUS_CODES = + EnumSet.of(Status.Code.TimedOut, Status.Code.TryAgain, Status.Code.Busy); + static void maybeRetryRocksDBAction( final RocksDBException ex, final int attemptNumber, @@ -131,13 +134,12 @@ static void maybeRetryRocksDBAction( System.exit(0); } if (attemptNumber <= retryLimit) { - if (ex.getMessage().contains(ERR_BUSY) || ex.getMessage().contains(ERR_LOCK_TIMED_OUT)) { + if (RETRYABLE_STATUS_CODES.contains(ex.getStatus().getCode())) { logger.warn( - "RocksDB Transient exception caught on attempt {} of {}, retrying.\n" - + "\tmessage: {}", + "RocksDB Transient exception caught on attempt {} of {}, status: {}, retrying.", attemptNumber, retryLimit, - ex.getMessage()); + ex.getStatus().getCodeString()); try { retryAction.retry(); } catch (RocksDBException ex2) { diff --git a/plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBTransactionTest.java b/plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBTransactionTest.java index 11829362946..c6e01468ab5 100644 --- a/plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBTransactionTest.java +++ b/plugins/rocksdb/src/test/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBTransactionTest.java @@ -33,11 +33,16 @@ import org.rocksdb.OptimisticTransactionDB; import org.rocksdb.Options; import org.rocksdb.RocksDBException; +import org.rocksdb.Status; import org.rocksdb.Transaction; import org.rocksdb.WriteOptions; @ExtendWith(MockitoExtension.class) public class RocksDBTransactionTest { + static final Status BUSY = new Status(Status.Code.Busy, Status.SubCode.None, "Busy"); + static final Status TIMED_OUT = + new Status(Status.Code.TimedOut, Status.SubCode.LockTimeout, "TimedOut(LockTimeout)"); + @TempDir public Path folder; @Mock(answer = RETURNS_DEEP_STUBS) @@ -60,8 +65,8 @@ public void assertNominalBehavior() throws Exception { @Test public void assertDefaultBusyRetryBehavior() throws Exception { - doThrow(new RocksDBException("Busy")) - .doThrow(new RocksDBException("Busy")) + doThrow(new RocksDBException("Busy", BUSY)) + .doThrow(new RocksDBException("Busy", BUSY)) .doNothing() .when(mockTransaction) .commit(); @@ -71,9 +76,9 @@ public void assertDefaultBusyRetryBehavior() throws Exception { @Test public void assertLockTimeoutBusyRetryBehavior() throws Exception { - doThrow(new RocksDBException("Busy")) - .doThrow(new RocksDBException("TimedOut(LockTimeout)")) - .doThrow(new RocksDBException("TimedOut(LockTimeout)")) + doThrow(new RocksDBException("Busy", BUSY)) + .doThrow(new RocksDBException("TimedOut(LockTimeout)", TIMED_OUT)) + .doThrow(new RocksDBException("TimedOut(LockTimeout)", TIMED_OUT)) .doNothing() .when(mockTransaction) .commit(); @@ -83,7 +88,7 @@ public void assertLockTimeoutBusyRetryBehavior() throws Exception { @Test public void assertBusyRetryFailBehavior() throws Exception { - doThrow(new RocksDBException("Busy")).when(mockTransaction).commit(); + doThrow(new RocksDBException("Busy", BUSY)).when(mockTransaction).commit(); assertThatThrownBy(tx::commit) .isInstanceOf(StorageException.class) @@ -100,8 +105,8 @@ public void assertRocksTxCloseOnRetryDoesNotThrow() throws Exception { tx = spy(new RocksDBTransaction(__ -> null, innerTx, writeOptions, mockMetrics)); - doThrow(new RocksDBException("Busy")) - .doThrow(new RocksDBException("Busy")) + doThrow(new RocksDBException("Busy", BUSY)) + .doThrow(new RocksDBException("Busy", BUSY)) .doCallRealMethod() .when(innerTx) .commit();