use status.code for identifying transient exceptions rather than mess…

…age parsing Signed-off-by: garyschulte <garyschulte@gmail.com>
garyschulte · Oct 5, 2023 · faa5dd6 · faa5dd6
1 parent 31ed321
commit faa5dd6
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 14 deletions.
diff --git a/...rc/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBTransaction.java b/...rc/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<Status.Code> 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/...est/java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBTransactionTest.java b/...est/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();