Harden RocksDB retry detection and error accounting

Signed-off-by: Solanasol233 <solanasolisss@gmail.com>

Author: Solanasol233

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/pathbased/common/storage/PathBasedWorldStateKeyValueStorage.java

@@ -386,7 +386,7 @@ private void moveDBEntry(
         tx.commit();
         break;
       } catch (StorageException se) {
-        if (!retried && se.getMessage().contains("RocksDBException: Busy")) {
+        if (!retried && RocksDbStorageExceptionHelper.isBusyException(se)) {
           retried = true;
         } else {
           break;

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/pathbased/common/storage/RocksDbStorageExceptionHelper.java

@@ -0,0 +1,68 @@
+/*
+ * Copyright contributors to Hyperledger Besu.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.hyperledger.besu.ethereum.trie.pathbased.common.storage;
+
+import org.hyperledger.besu.plugin.services.exception.StorageException;
+
+import java.lang.reflect.Method;
+import java.util.Optional;
+
+public final class RocksDbStorageExceptionHelper {
+
+  private static final String ROCKS_DB_EXCEPTION_CLASS_NAME = "org.rocksdb.RocksDBException";
+  private static final String BUSY_STATUS_CODE_NAME = "Busy";
+
+  private RocksDbStorageExceptionHelper() {}
+
+  public static boolean isBusyException(final StorageException storageException) {
+    return hasStatusCode(storageException, BUSY_STATUS_CODE_NAME);
+  }
+
+  public static boolean hasStatusCode(
+      final StorageException storageException, final String statusCodeName) {
+    return getStatusCodeName(storageException).filter(statusCodeName::equals).isPresent();
+  }
+
+  public static Optional<String> getStatusCodeName(final StorageException storageException) {
+    return Optional.ofNullable(storageException.getCause())
+        .filter(cause -> ROCKS_DB_EXCEPTION_CLASS_NAME.equals(cause.getClass().getName()))
+        .flatMap(RocksDbStorageExceptionHelper::extractStatusCodeName);
+  }
+
+  private static Optional<String> extractStatusCodeName(final Throwable rocksDbException) {
+    try {
+      // ethereum/core does not depend directly on rocksdbjni, so inspect the status via
+      // reflection instead of relying on exception message text.
+      final Method getStatusMethod = rocksDbException.getClass().getMethod("getStatus");
+      final Object status = getStatusMethod.invoke(rocksDbException);
+      if (status == null) {
+        return Optional.empty();
+      }
+
+      final Method getCodeMethod = status.getClass().getMethod("getCode");
+      final Object statusCode = getCodeMethod.invoke(status);
+      if (statusCode == null) {
+        return Optional.empty();
+      }
+
+      if (statusCode instanceof Enum<?> enumStatusCode) {
+        return Optional.of(enumStatusCode.name());
+      }
+      return Optional.of(statusCode.toString());
+    } catch (final ReflectiveOperationException ignored) {
+      return Optional.empty();
+    }
+  }
+}

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/StorageExceptionManager.java

@@ -18,6 +18,7 @@
 
 import java.util.EnumSet;
 import java.util.Optional;
+import java.util.concurrent.atomic.AtomicLong;
 
 import org.rocksdb.RocksDBException;
 import org.rocksdb.Status;
@@ -29,7 +30,7 @@ public final class StorageExceptionManager {
 
   private static final long ERROR_THRESHOLD = 1000;
 
-  private static long retryableErrorCounter;
+  private static final AtomicLong retryableErrorCounter = new AtomicLong();
 
   /**
    * Determines if an operation can be retried based on the error received. This method checks if
@@ -41,25 +42,25 @@ public final class StorageExceptionManager {
    * @return true if the operation can be retried, false otherwise
    */
   public static boolean canRetryOnError(final StorageException e) {
-    return Optional.of(e.getCause())
+    return Optional.ofNullable(e.getCause())
         .filter(z -> z instanceof RocksDBException)
         .map(RocksDBException.class::cast)
         .map(RocksDBException::getStatus)
         .map(Status::getCode)
-        .map(RETRYABLE_STATUS_CODES::contains)
+        .filter(RETRYABLE_STATUS_CODES::contains)
         .map(
-            result -> {
-              retryableErrorCounter++;
-              return result;
+            statusCode -> {
+              retryableErrorCounter.incrementAndGet();
+              return true;
             })
         .orElse(false);
   }
 
   public static long getRetryableErrorCounter() {
-    return retryableErrorCounter;
+    return retryableErrorCounter.get();
   }
 
   public static boolean errorCountAtThreshold() {
-    return retryableErrorCounter % ERROR_THRESHOLD == 1;
+    return retryableErrorCounter.get() % ERROR_THRESHOLD == 1;
   }
 }

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/StorageExceptionManagerTest.java

@@ -0,0 +1,66 @@
+/*
+ * Copyright contributors to Hyperledger Besu.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.hyperledger.besu.ethereum.eth.sync;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.hyperledger.besu.plugin.services.exception.StorageException;
+
+import java.lang.reflect.Field;
+import java.util.concurrent.atomic.AtomicLong;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.rocksdb.RocksDBException;
+import org.rocksdb.Status;
+
+public class StorageExceptionManagerTest {
+
+  @BeforeEach
+  public void resetRetryableErrorCounter() throws ReflectiveOperationException {
+    final Field retryableErrorCounterField =
+        StorageExceptionManager.class.getDeclaredField("retryableErrorCounter");
+    retryableErrorCounterField.setAccessible(true);
+    ((AtomicLong) retryableErrorCounterField.get(null)).set(0L);
+  }
+
+  @Test
+  public void shouldNotThrowWhenStorageExceptionCauseIsNull() {
+    assertThat(StorageExceptionManager.canRetryOnError(new StorageException("missing cause")))
+        .isFalse();
+    assertThat(StorageExceptionManager.getRetryableErrorCounter()).isZero();
+  }
+
+  @Test
+  public void shouldNotCountNonRetryableRocksDbErrors() {
+    final RocksDBException rocksDbException =
+        new RocksDBException(new Status(Status.Code.Aborted, Status.SubCode.None, "aborted"));
+
+    assertThat(StorageExceptionManager.canRetryOnError(new StorageException(rocksDbException)))
+        .isFalse();
+    assertThat(StorageExceptionManager.getRetryableErrorCounter()).isZero();
+  }
+
+  @Test
+  public void shouldCountRetryableRocksDbErrors() {
+    final RocksDBException rocksDbException =
+        new RocksDBException(new Status(Status.Code.Busy, Status.SubCode.None, "busy"));
+
+    assertThat(StorageExceptionManager.canRetryOnError(new StorageException(rocksDbException)))
+        .isTrue();
+    assertThat(StorageExceptionManager.getRetryableErrorCounter()).isEqualTo(1L);
+    assertThat(StorageExceptionManager.errorCountAtThreshold()).isTrue();
+  }
+}

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/trie/pathbased/common/storage/RocksDbStorageExceptionHelperTest.java

@@ -0,0 +1,51 @@
+/*
+ * Copyright contributors to Hyperledger Besu.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.hyperledger.besu.ethereum.trie.pathbased.common.storage;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.hyperledger.besu.plugin.services.exception.StorageException;
+
+import org.junit.jupiter.api.Test;
+import org.rocksdb.RocksDBException;
+import org.rocksdb.Status;
+
+public class RocksDbStorageExceptionHelperTest {
+
+  @Test
+  public void shouldReturnBusyStatusCodeNameFromRocksDbException() {
+    final StorageException storageException =
+        new StorageException(
+            new RocksDBException(new Status(Status.Code.Busy, Status.SubCode.None, "busy")));
+
+    assertThat(RocksDbStorageExceptionHelper.getStatusCodeName(storageException))
+        .contains("Busy");
+    assertThat(RocksDbStorageExceptionHelper.isBusyException(storageException)).isTrue();
+  }
+
+  @Test
+  public void shouldReturnEmptyWhenStorageExceptionHasNoCause() {
+    assertThat(RocksDbStorageExceptionHelper.getStatusCodeName(new StorageException("missing")))
+        .isEmpty();
+  }
+
+  @Test
+  public void shouldReturnEmptyWhenCauseIsNotRocksDbException() {
+    assertThat(
+            RocksDbStorageExceptionHelper.getStatusCodeName(
+                new StorageException(new IllegalStateException("not rocksdb"))))
+        .isEmpty();
+  }
+}