From ef198ac1fb064266739fdb8fdb655f683b14e28c Mon Sep 17 00:00:00 2001 From: ayanx <98254953+byteayan@users.noreply.github.com> Date: Tue, 20 Oct 2026 16:42:04 +0000 Subject: [PATCH 1/5] fix(pd): wait for a held metadata RocksDB lock at startup A restarted PD can start before the previous JVM has released the metadata RocksDB LOCK, and then fails startup straight away. Retry the open for up to 30 seconds while the failure is a lock conflict, then fail startup as before. Other open failures are not retried. Fixes #3226 --- hugegraph-pd/README.md | 1 + hugegraph-pd/docs/api-reference.md | 5 +- hugegraph-pd/docs/configuration.md | 3 + .../hugegraph/pd/store/HgKVStoreImpl.java | 57 ++++++++++++- .../apache/hugegraph/pd/rest/StoreAPI.java | 3 +- .../pd/core/store/HgKVStoreImplTest.java | 85 +++++++++++++++++++ 6 files changed, 147 insertions(+), 7 deletions(-) diff --git a/hugegraph-pd/README.md b/hugegraph-pd/README.md index a5dc5d6f13..f8f20c0c4f 100644 --- a/hugegraph-pd/README.md +++ b/hugegraph-pd/README.md @@ -291,6 +291,7 @@ docker/docker-compose-3pd-3store-3server.yml - PD uses Raft consensus for leader election and data replication - Cluster can tolerate up to `(N-1)/2` node failures (e.g., 1 failure in 3-node cluster) - Leader handles all write operations; followers handle read operations +- After a restart, PD waits up to 30 seconds for a previous process to release the metadata RocksDB `LOCK`, then exits non-zero; never delete `LOCK` files to work around this (see [`pd.data-path`](docs/configuration.md#production-recommendations-pd-data-path)) ### Network Requirements diff --git a/hugegraph-pd/docs/api-reference.md b/hugegraph-pd/docs/api-reference.md index 35d70fadd4..5945ebee25 100644 --- a/hugegraph-pd/docs/api-reference.md +++ b/hugegraph-pd/docs/api-reference.md @@ -813,8 +813,9 @@ Two unauthenticated endpoints are meant for probes and startup gates: | `GET /v1/ready` | Readiness: the raft node is active and sees a leader, so this PD is inside a quorum. | `200` when ready, `503` otherwise | The health probe reads Raft state only; it does not verify metadata-store initialization. -Metadata open failures propagate through startup; bounded LOCK retry remains tracked in -[#3226](https://github.com/apache/hugegraph/issues/3226). +Metadata open failures fail PD startup instead. While the metadata RocksDB LOCK is still held, +PD retries for up to 30 seconds first; see +[`pd.data-path`](configuration.md#production-recommendations-pd-data-path). ```bash curl -i http://localhost:8620/v1/ready diff --git a/hugegraph-pd/docs/configuration.md b/hugegraph-pd/docs/configuration.md index c2d6eecc9f..2dcfe3b246 100644 --- a/hugegraph-pd/docs/configuration.md +++ b/hugegraph-pd/docs/configuration.md @@ -192,8 +192,11 @@ pd: | `pd.initial-store-count` | Integer | `1` | Minimum number of Store nodes required for cluster to be operational. Set to expected initial store count. | | `pd.initial-store-list` | String | `127.0.0.1:8500` | Comma-separated list of Store gRPC addresses to auto-activate on startup. Useful for bootstrapping. | + **Production Recommendations**: - `pd.data-path`: Use dedicated SSD with at least 50GB free space + - PD opens its metadata RocksDB at `/rocksdb`. If its `LOCK` is still held, usually by a previous PD process that is still stopping, PD retries for up to 30 seconds and then fails startup with a non-zero exit so the supervisor can restart it. + - Never delete `LOCK` files under `pd.data-path` (`rocksdb/` or the Raft directories) to get past this: while the previous process is alive, two processes then write the same database. Stop or kill the old process instead. - `pd.patrol-interval`: - Development: `300` (5 minutes) for fast testing - Production: `1800` (30 minutes) to reduce overhead diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/store/HgKVStoreImpl.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/store/HgKVStoreImpl.java index f33648e0eb..21a3ff3ac3 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/store/HgKVStoreImpl.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/store/HgKVStoreImpl.java @@ -43,6 +43,7 @@ import org.rocksdb.RocksDBException; import org.rocksdb.RocksIterator; import org.rocksdb.Slice; +import org.rocksdb.Status; import com.alipay.sofa.jraft.util.Utils; import com.google.common.cache.CacheBuilder; @@ -55,10 +56,17 @@ public class HgKVStoreImpl implements HgKVStore { private static final ConcurrentHashMap> CACHE = new ConcurrentHashMap(); + /* + * A restarted PD can start before the previous JVM has released the RocksDB LOCK. + * Wait about as long as stop-hugegraph-pd.sh waits for that JVM, then fail startup. + */ + private static final long LOCK_WAIT_TIMEOUT_MS = 30_000L; + private static final long LOCK_RETRY_INTERVAL_MS = 500L; private final ReadWriteLock readWriteLock = new ReentrantReadWriteLock(); private RocksDB db; private String dbPath; private Options dbOptions; + private long lockWaitTimeoutMs = LOCK_WAIT_TIMEOUT_MS; @Override public void init(PDConfig config) { @@ -75,10 +83,8 @@ public void init(PDConfig config) { log.warn("Failed to create data file,{}", e); } } - openRocksDB(dbPath); + openRocksDBWaitingForLock(this.dbPath); } catch (PDException | RuntimeException | Error e) { - // TODO: retry opening a held RocksDB LOCK before failing PD startup. - // https://github.com/apache/hugegraph/issues/3226 if (this.dbOptions != null) { this.dbOptions.close(); this.dbOptions = null; @@ -347,6 +353,51 @@ private void closeRocksDB() { } } + private void openRocksDBWaitingForLock(String dbPath) throws PDException { + long deadline = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(this.lockWaitTimeoutMs); + boolean waited = false; + while (true) { + try { + this.db = RocksDB.open(this.dbOptions, dbPath); + if (waited) { + log.info("Opened RocksDB at {} after its LOCK was released", dbPath); + } + return; + } catch (RocksDBException e) { + if (!isLockConflict(e) || System.nanoTime() - deadline >= 0) { + log.error("Failed to open RocksDB from {}", dbPath, e); + throw new PDException(Pdpb.ErrorType.ROCKSDB_LOAD_SNAPSHOT_ERROR_VALUE, e); + } + if (!waited) { + log.warn("RocksDB LOCK at {} is held, probably by a previous PD that is " + + "still stopping; retrying for up to {} ms: {}", + dbPath, this.lockWaitTimeoutMs, e.getMessage()); + waited = true; + } + try { + Thread.sleep(LOCK_RETRY_INTERVAL_MS); + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + e.addSuppressed(ie); + throw new PDException(Pdpb.ErrorType.ROCKSDB_LOAD_SNAPSHOT_ERROR_VALUE, e); + } + } + } + } + + /** + * The LOCK is held by another process ("While lock file") or by another handle in this + * JVM ("lock hold by current process"). Other open failures, including a LOCK file that + * cannot be created, are not transient and are not retried. + */ + private static boolean isLockConflict(RocksDBException e) { + Status status = e.getStatus(); + String message = e.getMessage(); + return status != null && status.getCode() == Status.Code.IOError && message != null && + (message.contains("While lock file") || + message.contains("lock hold by current process")); + } + private void openRocksDB(String dbPath) throws PDException { try { this.db = RocksDB.open(dbOptions, dbPath); diff --git a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/StoreAPI.java b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/StoreAPI.java index e4fea9d16e..a7ad2a33f8 100644 --- a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/StoreAPI.java +++ b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/StoreAPI.java @@ -395,8 +395,7 @@ class StoreStatistics { * @return Returns a string indicating the service's health status. Typically, an empty * string indicates the service is healthy. */ - // TODO: bounded metadata-store LOCK retry is tracked separately in - // https://github.com/apache/hugegraph/issues/3226; this probe reads only Raft state. + // This probe reads only Raft state; metadata-store open failures fail PD startup instead. @GetMapping(value = "/health", produces = MediaType.TEXT_PLAIN_VALUE) public ResponseEntity checkHealthy() { String state = RaftEngine.getInstance().getRaftStatus().getState(); diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/store/HgKVStoreImplTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/store/HgKVStoreImplTest.java index 36ef499038..ec3e8c7f28 100644 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/store/HgKVStoreImplTest.java +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/store/HgKVStoreImplTest.java @@ -19,7 +19,9 @@ import java.io.File; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.nio.file.Paths; +import java.util.concurrent.TimeUnit; import org.apache.commons.io.FileUtils; import org.apache.hugegraph.pd.common.PDException; @@ -28,9 +30,13 @@ import org.apache.hugegraph.pd.meta.MetadataKeyHelper; import org.apache.hugegraph.pd.store.HgKVStore; import org.apache.hugegraph.pd.store.HgKVStoreImpl; +import org.apache.hugegraph.testutil.Whitebox; import org.junit.Assert; import org.junit.BeforeClass; import org.junit.Test; +import org.rocksdb.Options; +import org.rocksdb.RocksDB; +import org.rocksdb.RocksDBException; public class HgKVStoreImplTest { @@ -135,4 +141,83 @@ public void TestSnapshot() throws PDException { kvStore.close(); } + + @Test + public void testInitWaitsForHeldLock() throws Exception { + PDConfig config = newConfig("tmp/test-lock-released"); + String dbPath = config.getDataPath() + "/rocksdb"; + FileUtils.forceMkdir(new File(dbPath)); + HgKVStore kvStore = new HgKVStoreImpl(); + try (Options options = new Options().setCreateIfMissing(true)) { + RocksDB holder = RocksDB.open(options, dbPath); + // Release the LOCK the way a previous PD JVM does while it is still stopping + Thread releaser = new Thread(() -> { + try { + Thread.sleep(1000L); + } catch (InterruptedException ignored) { + Thread.currentThread().interrupt(); + } + holder.close(); + }); + releaser.start(); + try { + kvStore.init(config); + byte[] key = "lock".getBytes(StandardCharsets.UTF_8); + kvStore.put(key, key); + Assert.assertArrayEquals(key, kvStore.get(key)); + } finally { + releaser.join(); + kvStore.close(); + } + } + } + + @Test + public void testInitFailsWhenLockIsNeverReleased() throws Exception { + PDConfig config = newConfig("tmp/test-lock-held"); + String dbPath = config.getDataPath() + "/rocksdb"; + FileUtils.forceMkdir(new File(dbPath)); + HgKVStoreImpl kvStore = new HgKVStoreImpl(); + Whitebox.setInternalState(kvStore, "lockWaitTimeoutMs", 1000L); + try (Options options = new Options().setCreateIfMissing(true); + RocksDB ignored = RocksDB.open(options, dbPath)) { + long start = System.nanoTime(); + IllegalStateException e = Assert.assertThrows(IllegalStateException.class, + () -> kvStore.init(config)); + long elapsed = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start); + Assert.assertTrue("gave up after " + elapsed + " ms", elapsed >= 1000L); + Assert.assertTrue(e.getCause() instanceof PDException); + Assert.assertTrue(e.getCause().getCause() instanceof RocksDBException); + } finally { + kvStore.close(); + } + } + + @Test + public void testInitDoesNotRetryOtherOpenFailures() throws Exception { + PDConfig config = newConfig("tmp/test-lock-not-a-dir"); + // A regular file where the RocksDB directory belongs is not a lock conflict + File dbFile = new File(config.getDataPath() + "/rocksdb"); + FileUtils.forceMkdir(dbFile.getParentFile()); + FileUtils.touch(dbFile); + HgKVStore kvStore = new HgKVStoreImpl(); + long start = System.nanoTime(); + try { + Assert.assertThrows(IllegalStateException.class, () -> kvStore.init(config)); + } finally { + kvStore.close(); + } + long elapsed = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start); + Assert.assertTrue("retried for " + elapsed + " ms", elapsed < 5000L); + } + + private static PDConfig newConfig(String path) throws IOException { + File dir = new File(path); + if (dir.exists()) { + FileUtils.deleteDirectory(dir); + } + return new PDConfig() {{ + setDataPath(path); + }}; + } } From 1c09528813fda889758fabf147ef8da3fb19eb3f Mon Sep 17 00:00:00 2001 From: ayanx <98254953+byteayan@users.noreply.github.com> Date: Tue, 20 Oct 2026 16:56:44 +0000 Subject: [PATCH 2/5] fix(pd): back off between metadata RocksDB lock retries Each RocksDB open attempt rotates the info LOG, so back off from 500 ms to 4 s instead of retrying every 500 ms, and never sleep past the deadline. Share the open-and-wrap path with snapshot reload, tighten the tests, and correct the docs about how a failed start exits. --- hugegraph-pd/README.md | 2 +- hugegraph-pd/docs/api-reference.md | 5 +-- hugegraph-pd/docs/configuration.md | 5 +-- .../hugegraph/pd/store/HgKVStoreImpl.java | 37 ++++++++++--------- .../pd/core/store/HgKVStoreImplTest.java | 14 ++++++- 5 files changed, 37 insertions(+), 26 deletions(-) diff --git a/hugegraph-pd/README.md b/hugegraph-pd/README.md index f8f20c0c4f..5d62d76659 100644 --- a/hugegraph-pd/README.md +++ b/hugegraph-pd/README.md @@ -291,7 +291,7 @@ docker/docker-compose-3pd-3store-3server.yml - PD uses Raft consensus for leader election and data replication - Cluster can tolerate up to `(N-1)/2` node failures (e.g., 1 failure in 3-node cluster) - Leader handles all write operations; followers handle read operations -- After a restart, PD waits up to 30 seconds for a previous process to release the metadata RocksDB `LOCK`, then exits non-zero; never delete `LOCK` files to work around this (see [`pd.data-path`](docs/configuration.md#production-recommendations-pd-data-path)) +- Never delete RocksDB `LOCK` files under `pd.data-path` to get a restarted PD going; see [PD Core Settings](docs/configuration.md#pd-core-settings) ### Network Requirements diff --git a/hugegraph-pd/docs/api-reference.md b/hugegraph-pd/docs/api-reference.md index 5945ebee25..122ae6c8b4 100644 --- a/hugegraph-pd/docs/api-reference.md +++ b/hugegraph-pd/docs/api-reference.md @@ -813,9 +813,8 @@ Two unauthenticated endpoints are meant for probes and startup gates: | `GET /v1/ready` | Readiness: the raft node is active and sees a leader, so this PD is inside a quorum. | `200` when ready, `503` otherwise | The health probe reads Raft state only; it does not verify metadata-store initialization. -Metadata open failures fail PD startup instead. While the metadata RocksDB LOCK is still held, -PD retries for up to 30 seconds first; see -[`pd.data-path`](configuration.md#production-recommendations-pd-data-path). +If the metadata store cannot be opened, PD startup fails; while its RocksDB `LOCK` is still +held, PD first retries for up to 30 seconds (see [PD Core Settings](configuration.md#pd-core-settings)). ```bash curl -i http://localhost:8620/v1/ready diff --git a/hugegraph-pd/docs/configuration.md b/hugegraph-pd/docs/configuration.md index 2dcfe3b246..18cd0dd884 100644 --- a/hugegraph-pd/docs/configuration.md +++ b/hugegraph-pd/docs/configuration.md @@ -192,11 +192,10 @@ pd: | `pd.initial-store-count` | Integer | `1` | Minimum number of Store nodes required for cluster to be operational. Set to expected initial store count. | | `pd.initial-store-list` | String | `127.0.0.1:8500` | Comma-separated list of Store gRPC addresses to auto-activate on startup. Useful for bootstrapping. | - **Production Recommendations**: - `pd.data-path`: Use dedicated SSD with at least 50GB free space - - PD opens its metadata RocksDB at `/rocksdb`. If its `LOCK` is still held, usually by a previous PD process that is still stopping, PD retries for up to 30 seconds and then fails startup with a non-zero exit so the supervisor can restart it. - - Never delete `LOCK` files under `pd.data-path` (`rocksdb/` or the Raft directories) to get past this: while the previous process is alive, two processes then write the same database. Stop or kill the old process instead. + - PD opens its metadata RocksDB at `/rocksdb`. If its `LOCK` is still held, usually by a previous PD process that is still stopping, PD retries for up to 30 seconds and then fails startup. Run PD in the foreground (`-d false`) under a supervisor so that a failed start exits and is restarted. + - Never delete the `LOCK` files in `/rocksdb/` or `/pd_raft/log/` to get past this: while the previous process is alive, two processes then write the same database. Stop or kill the old process instead. - `pd.patrol-interval`: - Development: `300` (5 minutes) for fast testing - Production: `1800` (30 minutes) to reduce overhead diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/store/HgKVStoreImpl.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/store/HgKVStoreImpl.java index 21a3ff3ac3..dec86dfb28 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/store/HgKVStoreImpl.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/store/HgKVStoreImpl.java @@ -59,13 +59,17 @@ public class HgKVStoreImpl implements HgKVStore { /* * A restarted PD can start before the previous JVM has released the RocksDB LOCK. * Wait about as long as stop-hugegraph-pd.sh waits for that JVM, then fail startup. + * Every attempt rotates the RocksDB info LOG, so back off between attempts. This + * covers the metadata store only; PD opens it before the raft log store. */ private static final long LOCK_WAIT_TIMEOUT_MS = 30_000L; - private static final long LOCK_RETRY_INTERVAL_MS = 500L; + private static final long LOCK_RETRY_MIN_INTERVAL_MS = 500L; + private static final long LOCK_RETRY_MAX_INTERVAL_MS = 4_000L; private final ReadWriteLock readWriteLock = new ReentrantReadWriteLock(); private RocksDB db; private String dbPath; private Options dbOptions; + // Tests shorten this private long lockWaitTimeoutMs = LOCK_WAIT_TIMEOUT_MS; @Override @@ -83,7 +87,7 @@ public void init(PDConfig config) { log.warn("Failed to create data file,{}", e); } } - openRocksDBWaitingForLock(this.dbPath); + openRocksDB(this.dbPath, this.lockWaitTimeoutMs); } catch (PDException | RuntimeException | Error e) { if (this.dbOptions != null) { this.dbOptions.close(); @@ -353,8 +357,13 @@ private void closeRocksDB() { } } - private void openRocksDBWaitingForLock(String dbPath) throws PDException { - long deadline = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(this.lockWaitTimeoutMs); + private void openRocksDB(String dbPath) throws PDException { + openRocksDB(dbPath, 0L); + } + + private void openRocksDB(String dbPath, long lockWaitTimeoutMs) throws PDException { + long deadline = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(lockWaitTimeoutMs); + long interval = LOCK_RETRY_MIN_INTERVAL_MS; boolean waited = false; while (true) { try { @@ -371,11 +380,13 @@ private void openRocksDBWaitingForLock(String dbPath) throws PDException { if (!waited) { log.warn("RocksDB LOCK at {} is held, probably by a previous PD that is " + "still stopping; retrying for up to {} ms: {}", - dbPath, this.lockWaitTimeoutMs, e.getMessage()); + dbPath, lockWaitTimeoutMs, e.getMessage()); waited = true; } try { - Thread.sleep(LOCK_RETRY_INTERVAL_MS); + long remainingMs = TimeUnit.NANOSECONDS.toMillis(deadline - System.nanoTime()); + Thread.sleep(Math.max(1L, Math.min(interval, remainingMs))); + interval = Math.min(interval * 2, LOCK_RETRY_MAX_INTERVAL_MS); } catch (InterruptedException ie) { Thread.currentThread().interrupt(); e.addSuppressed(ie); @@ -387,8 +398,9 @@ private void openRocksDBWaitingForLock(String dbPath) throws PDException { /** * The LOCK is held by another process ("While lock file") or by another handle in this - * JVM ("lock hold by current process"). Other open failures, including a LOCK file that - * cannot be created, are not transient and are not retried. + * JVM ("lock hold by current process"). "While lock file" also covers a file system that + * cannot lock at all; that fails once the wait runs out. Other open failures, including a + * LOCK file that cannot be created, are not retried. */ private static boolean isLockConflict(RocksDBException e) { Status status = e.getStatus(); @@ -397,13 +409,4 @@ private static boolean isLockConflict(RocksDBException e) { (message.contains("While lock file") || message.contains("lock hold by current process")); } - - private void openRocksDB(String dbPath) throws PDException { - try { - this.db = RocksDB.open(dbOptions, dbPath); - } catch (RocksDBException e) { - log.error("Failed to open RocksDB from {}", dbPath, e); - throw new PDException(Pdpb.ErrorType.ROCKSDB_LOAD_SNAPSHOT_ERROR_VALUE, e); - } - } } diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/store/HgKVStoreImplTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/store/HgKVStoreImplTest.java index ec3e8c7f28..69c27728c8 100644 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/store/HgKVStoreImplTest.java +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/store/HgKVStoreImplTest.java @@ -158,16 +158,21 @@ public void testInitWaitsForHeldLock() throws Exception { Thread.currentThread().interrupt(); } holder.close(); - }); + }, "test-lock-releaser"); + releaser.setDaemon(true); releaser.start(); try { + long start = System.nanoTime(); kvStore.init(config); + long elapsed = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start); + Assert.assertTrue("opened after " + elapsed + " ms", elapsed >= 500L); byte[] key = "lock".getBytes(StandardCharsets.UTF_8); kvStore.put(key, key); Assert.assertArrayEquals(key, kvStore.get(key)); } finally { releaser.join(); kvStore.close(); + FileUtils.deleteQuietly(new File(config.getDataPath())); } } } @@ -190,6 +195,7 @@ public void testInitFailsWhenLockIsNeverReleased() throws Exception { Assert.assertTrue(e.getCause().getCause() instanceof RocksDBException); } finally { kvStore.close(); + FileUtils.deleteQuietly(new File(config.getDataPath())); } } @@ -203,9 +209,13 @@ public void testInitDoesNotRetryOtherOpenFailures() throws Exception { HgKVStore kvStore = new HgKVStoreImpl(); long start = System.nanoTime(); try { - Assert.assertThrows(IllegalStateException.class, () -> kvStore.init(config)); + IllegalStateException e = Assert.assertThrows(IllegalStateException.class, + () -> kvStore.init(config)); + String message = e.getCause().getCause().getMessage(); + Assert.assertTrue(message, message.contains("is not a directory")); } finally { kvStore.close(); + FileUtils.deleteQuietly(new File(config.getDataPath())); } long elapsed = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start); Assert.assertTrue("retried for " + elapsed + " ms", elapsed < 5000L); From 89aa82ec1f2a9783ea229cac4e1577033ec9029c Mon Sep 17 00:00:00 2001 From: ayanx <98254953+byteayan@users.noreply.github.com> Date: Wed, 21 Oct 2026 10:31:49 +0000 Subject: [PATCH 3/5] fix(pd): open PD metadata before Raft in every startup callback DiscoveryService, KvServiceGrpcImpl and PDService each start the shared RaftEngine from @PostConstruct, in no fixed order. Only PDService opened the metadata store first, so a held raft log LOCK could abort startup before the metadata lock wait ran. Start Raft through RaftStartup, which opens metadata first; a stopping PD releases the raft log LOCK before the metadata LOCK. Also test the lock wait against a LOCK held by another process, the restart case, for both wait-then-open and timeout. --- .../hugegraph/pd/store/HgKVStoreImpl.java | 5 +- .../pd/service/DiscoveryService.java | 2 +- .../pd/service/KvServiceGrpcImpl.java | 2 +- .../hugegraph/pd/service/PDService.java | 2 +- .../hugegraph/pd/service/RaftStartup.java | 42 +++++++++++ .../hugegraph/pd/core/PDCoreSuiteTest.java | 1 + .../hugegraph/pd/core/RaftStartupTest.java | 70 +++++++++++++++++++ .../pd/core/store/HgKVStoreImplTest.java | 68 ++++++++++++++++++ .../pd/core/store/RocksDBLockHolder.java | 46 ++++++++++++ 9 files changed, 233 insertions(+), 5 deletions(-) create mode 100644 hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/RaftStartup.java create mode 100644 hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/RaftStartupTest.java create mode 100644 hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/store/RocksDBLockHolder.java diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/store/HgKVStoreImpl.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/store/HgKVStoreImpl.java index dec86dfb28..105f54736d 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/store/HgKVStoreImpl.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/store/HgKVStoreImpl.java @@ -59,8 +59,9 @@ public class HgKVStoreImpl implements HgKVStore { /* * A restarted PD can start before the previous JVM has released the RocksDB LOCK. * Wait about as long as stop-hugegraph-pd.sh waits for that JVM, then fail startup. - * Every attempt rotates the RocksDB info LOG, so back off between attempts. This - * covers the metadata store only; PD opens it before the raft log store. + * Every attempt rotates the RocksDB info LOG, so back off between attempts. Only the + * metadata store waits: RaftStartup opens it before the raft log store, and a stopping + * PD releases the raft log LOCK before this one. */ private static final long LOCK_WAIT_TIMEOUT_MS = 30_000L; private static final long LOCK_RETRY_MIN_INTERVAL_MS = 500L; diff --git a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/DiscoveryService.java b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/DiscoveryService.java index 00a652577f..e74b529c91 100644 --- a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/DiscoveryService.java +++ b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/DiscoveryService.java @@ -59,7 +59,7 @@ public class DiscoveryService extends DiscoveryServiceGrpc.DiscoveryServiceImplB @PostConstruct public void init() throws PDException { log.info("PDService init………… {}", pdConfig); - RaftEngine.getInstance().init(pdConfig.getRaft()); + RaftStartup.init(pdConfig); RaftEngine.getInstance().addStateListener(this); register = new RegistryService(pdConfig); licenseVerifierService = new LicenseVerifierService(pdConfig); diff --git a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/KvServiceGrpcImpl.java b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/KvServiceGrpcImpl.java index 94b41a0f0a..8d9438d391 100644 --- a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/KvServiceGrpcImpl.java +++ b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/KvServiceGrpcImpl.java @@ -75,7 +75,7 @@ public class KvServiceGrpcImpl extends KvServiceGrpc.KvServiceImplBase implement @PostConstruct public void init() { - RaftEngine.getInstance().init(pdConfig.getRaft()); + RaftStartup.init(pdConfig); RaftEngine.getInstance().addStateListener(this); kvService = new KvService(pdConfig); subjects = new KvWatchSubject(pdConfig); diff --git a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/PDService.java b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/PDService.java index f40b2f5801..500bd41959 100644 --- a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/PDService.java +++ b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/PDService.java @@ -199,7 +199,7 @@ private void initService() throws PDException { RaftEngine.getInstance().addStateListener(this); RaftEngine.getInstance().addStateListener(configService); - RaftEngine.getInstance().init(pdConfig.getRaft()); + RaftStartup.init(pdConfig); //pdConfig = configService.loadConfig(); onLeaderChanged storeNodeService = new StoreNodeService(pdConfig); partitionService = new PartitionService(pdConfig, storeNodeService); diff --git a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/RaftStartup.java b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/RaftStartup.java new file mode 100644 index 0000000000..0f43f14dea --- /dev/null +++ b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/RaftStartup.java @@ -0,0 +1,42 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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. + */ + +package org.apache.hugegraph.pd.service; + +import org.apache.hugegraph.pd.config.PDConfig; +import org.apache.hugegraph.pd.meta.MetadataFactory; +import org.apache.hugegraph.pd.raft.RaftEngine; + +/** + * Starts the shared {@link RaftEngine} from any service's startup callback. + */ +public final class RaftStartup { + + private RaftStartup() { + } + + /** + * Open the metadata store before Raft. Its open waits for a LOCK still held by a stopping + * PD, and that PD closes Raft before metadata, so the raft log LOCK is free once the + * metadata store is open. Spring runs the service callbacks in no fixed order, so each of + * them starts Raft through here. + */ + public static void init(PDConfig pdConfig) { + MetadataFactory.getStore(pdConfig); + RaftEngine.getInstance().init(pdConfig.getRaft()); + } +} diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java index c0c15975e8..ca0b94adae 100644 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java @@ -44,6 +44,7 @@ IdMetaStoreReadIndexTest.class, HgKVStoreImplTest.class, PDNativeOwnerTest.class, + RaftStartupTest.class, PDConfigTest.class, ConfigServiceTest.class, IdServiceTest.class, diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/RaftStartupTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/RaftStartupTest.java new file mode 100644 index 0000000000..4d9c4eea18 --- /dev/null +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/RaftStartupTest.java @@ -0,0 +1,70 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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. + */ + +package org.apache.hugegraph.pd.core; + +import java.io.File; +import java.lang.reflect.Field; + +import org.apache.commons.io.FileUtils; +import org.apache.hugegraph.pd.config.PDConfig; +import org.apache.hugegraph.pd.meta.MetadataFactory; +import org.apache.hugegraph.pd.raft.RaftEngine; +import org.apache.hugegraph.pd.service.RaftStartup; +import org.junit.Assert; +import org.junit.Test; + +public class RaftStartupTest { + + @Test + public void testRaftDoesNotStartBeforeMetadataOpens() throws Exception { + Assert.assertNull(RaftEngine.getInstance().getRaftNode()); + String dataPath = "tmp/test-raft-startup"; + FileUtils.deleteQuietly(new File(dataPath)); + // A regular file where the metadata RocksDB directory belongs makes its open fail + FileUtils.forceMkdir(new File(dataPath)); + FileUtils.touch(new File(dataPath, "rocksdb")); + PDConfig config = new PDConfig(); + config.setDataPath(dataPath); + PDConfig.Raft raft = config.new Raft(); + raft.setDataPath(dataPath + "/raft"); + config.setRaft(raft); + + Field storeField = MetadataFactory.class.getDeclaredField("store"); + Field shutdownField = MetadataFactory.class.getDeclaredField("shutdown"); + storeField.setAccessible(true); + shutdownField.setAccessible(true); + synchronized (MetadataFactory.class) { + Object previous = storeField.get(null); + boolean shutdown = shutdownField.getBoolean(null); + try { + storeField.set(null, null); + shutdownField.setBoolean(null, false); + IllegalStateException e = Assert.assertThrows(IllegalStateException.class, + () -> RaftStartup.init(config)); + Assert.assertTrue(e.getMessage(), + e.getMessage().startsWith("Failed to open PD metadata")); + Assert.assertFalse(new File(dataPath, "raft").exists()); + Assert.assertNull(RaftEngine.getInstance().getRaftNode()); + } finally { + storeField.set(null, previous); + shutdownField.setBoolean(null, shutdown); + FileUtils.deleteQuietly(new File(dataPath)); + } + } + } +} diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/store/HgKVStoreImplTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/store/HgKVStoreImplTest.java index 69c27728c8..772f7a7909 100644 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/store/HgKVStoreImplTest.java +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/store/HgKVStoreImplTest.java @@ -17,8 +17,10 @@ package org.apache.hugegraph.pd.core.store; +import java.io.BufferedReader; import java.io.File; import java.io.IOException; +import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; import java.nio.file.Paths; import java.util.concurrent.TimeUnit; @@ -221,6 +223,72 @@ public void testInitDoesNotRetryOtherOpenFailures() throws Exception { Assert.assertTrue("retried for " + elapsed + " ms", elapsed < 5000L); } + @Test(timeout = 60_000L) + public void testInitWaitsForLockHeldByAnotherProcess() throws Exception { + PDConfig config = newConfig("tmp/test-process-lock-released"); + String dbPath = config.getDataPath() + "/rocksdb"; + FileUtils.forceMkdir(new File(dbPath)); + HgKVStore kvStore = new HgKVStoreImpl(); + Process holder = startLockHolder(dbPath, 1000L); + try { + long start = System.nanoTime(); + kvStore.init(config); + long elapsed = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start); + Assert.assertTrue("opened after " + elapsed + " ms", elapsed >= 500L); + byte[] key = "lock".getBytes(StandardCharsets.UTF_8); + kvStore.put(key, key); + Assert.assertArrayEquals(key, kvStore.get(key)); + Assert.assertEquals(0, holder.waitFor()); + } finally { + holder.destroyForcibly().waitFor(); + kvStore.close(); + FileUtils.deleteQuietly(new File(config.getDataPath())); + } + } + + @Test(timeout = 60_000L) + public void testInitFailsWhenAnotherProcessKeepsLock() throws Exception { + PDConfig config = newConfig("tmp/test-process-lock-held"); + String dbPath = config.getDataPath() + "/rocksdb"; + FileUtils.forceMkdir(new File(dbPath)); + HgKVStoreImpl kvStore = new HgKVStoreImpl(); + Whitebox.setInternalState(kvStore, "lockWaitTimeoutMs", 1000L); + Process holder = startLockHolder(dbPath, -1L); + try { + long start = System.nanoTime(); + IllegalStateException e = Assert.assertThrows(IllegalStateException.class, + () -> kvStore.init(config)); + long elapsed = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start); + Assert.assertTrue("gave up after " + elapsed + " ms", elapsed >= 1000L); + Throwable cause = e.getCause().getCause(); + Assert.assertTrue(cause instanceof RocksDBException); + Assert.assertTrue(cause.getMessage(), cause.getMessage().contains("While lock file")); + } finally { + holder.destroyForcibly().waitFor(); + kvStore.close(); + FileUtils.deleteQuietly(new File(config.getDataPath())); + } + } + + private static Process startLockHolder(String dbPath, long holdMs) throws Exception { + String java = Paths.get(System.getProperty("java.home"), "bin", "java").toString(); + Process process = new ProcessBuilder(java, "-cp", System.getProperty("java.class.path"), + RocksDBLockHolder.class.getName(), dbPath, + String.valueOf(holdMs)) + .redirectErrorStream(true) + .start(); + BufferedReader output = new BufferedReader( + new InputStreamReader(process.getInputStream(), StandardCharsets.UTF_8)); + String line; + do { + line = output.readLine(); + } while (line != null && !RocksDBLockHolder.LOCKED.equals(line)); + if (line == null) { + Assert.fail("Lock holder exited with " + process.waitFor()); + } + return process; + } + private static PDConfig newConfig(String path) throws IOException { File dir = new File(path); if (dir.exists()) { diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/store/RocksDBLockHolder.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/store/RocksDBLockHolder.java new file mode 100644 index 0000000000..9cdec9cedb --- /dev/null +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/store/RocksDBLockHolder.java @@ -0,0 +1,46 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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. + */ + +package org.apache.hugegraph.pd.core.store; + +import org.rocksdb.Options; +import org.rocksdb.RocksDB; + +/** + * Holds a RocksDB LOCK from a separate JVM, the way a previous PD process does while it is + * still stopping. Prints {@link #LOCKED} once the database is open, then releases it after + * the given number of milliseconds, or holds it until killed when the number is negative. + */ +public final class RocksDBLockHolder { + + public static final String LOCKED = "LOCKED"; + + private RocksDBLockHolder() { + } + + public static void main(String[] args) throws Exception { + String dbPath = args[0]; + long holdMs = Long.parseLong(args[1]); + RocksDB.loadLibrary(); + try (Options options = new Options().setCreateIfMissing(true); + RocksDB ignored = RocksDB.open(options, dbPath)) { + System.out.println(LOCKED); + System.out.flush(); + Thread.sleep(holdMs < 0 ? Long.MAX_VALUE : holdMs); + } + } +} From 68a18856780fc1e2ed751cb931a8d72c244c6660 Mon Sep 17 00:00:00 2001 From: ayanx <98254953+byteayan@users.noreply.github.com> Date: Wed, 21 Oct 2026 10:42:33 +0000 Subject: [PATCH 4/5] fix(pd): move RaftStartup into hg-pd-core hg-pd-service is repackaged as a Spring Boot jar, so hg-pd-test cannot compile against its classes in a packaged build. Keep the package so the service callbacks are unchanged. --- .../main/java/org/apache/hugegraph/pd/service/RaftStartup.java | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename hugegraph-pd/{hg-pd-service => hg-pd-core}/src/main/java/org/apache/hugegraph/pd/service/RaftStartup.java (100%) diff --git a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/RaftStartup.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/service/RaftStartup.java similarity index 100% rename from hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/RaftStartup.java rename to hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/service/RaftStartup.java From 38087dbd29ba54b7ef010aacc5a64cf70fee885e Mon Sep 17 00:00:00 2001 From: ayanx <98254953+byteayan@users.noreply.github.com> Date: Wed, 21 Oct 2026 11:33:39 +0000 Subject: [PATCH 5/5] fix(pd): release metadata opened by a failed Raft startup When DiscoveryService or KvServiceGrpcImpl starts first and Raft then fails, nothing closed the metadata store RaftStartup had just opened, so its LOCK stayed held until the JVM exited. Close it again after joining Raft, but only when this call opened it; a store opened by PDService or another caller stays with its owner. --- .../hugegraph/pd/meta/MetadataFactory.java | 20 ++++ .../hugegraph/pd/service/RaftStartup.java | 26 +++++- .../hugegraph/pd/core/RaftStartupTest.java | 93 ++++++++++++++++--- 3 files changed, 122 insertions(+), 17 deletions(-) diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/meta/MetadataFactory.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/meta/MetadataFactory.java index 954a9f9e2e..e7c64d0222 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/meta/MetadataFactory.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/meta/MetadataFactory.java @@ -56,6 +56,26 @@ public static synchronized HgKVStore getStore(PDConfig pdConfig) { return store; } + /** + * Open the store like {@link #getStore}, and return it only when this call opened it. + */ + public static synchronized HgKVStore openStore(PDConfig pdConfig) { + HgKVStore previous = store; + HgKVStore current = getStore(pdConfig); + return previous == null ? current : null; + } + + /** + * Close a store that a failed startup opened, so its LOCK does not outlive the attempt. + * A store another caller has replaced it with stays open. + */ + public static synchronized void discardStore(HgKVStore opened) { + if (opened != null && store == opened) { + store = null; + opened.close(); + } + } + public static synchronized void closeStore() { if (store != null) { store.close(); diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/service/RaftStartup.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/service/RaftStartup.java index 0f43f14dea..53b27c8a14 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/service/RaftStartup.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/service/RaftStartup.java @@ -20,6 +20,7 @@ import org.apache.hugegraph.pd.config.PDConfig; import org.apache.hugegraph.pd.meta.MetadataFactory; import org.apache.hugegraph.pd.raft.RaftEngine; +import org.apache.hugegraph.pd.store.HgKVStore; /** * Starts the shared {@link RaftEngine} from any service's startup callback. @@ -33,10 +34,29 @@ private RaftStartup() { * Open the metadata store before Raft. Its open waits for a LOCK still held by a stopping * PD, and that PD closes Raft before metadata, so the raft log LOCK is free once the * metadata store is open. Spring runs the service callbacks in no fixed order, so each of - * them starts Raft through here. + * them starts Raft through here. If Raft then fails to start, a metadata store this call + * opened is closed again, since a callback other than PDService has no owner to close it. */ public static void init(PDConfig pdConfig) { - MetadataFactory.getStore(pdConfig); - RaftEngine.getInstance().init(pdConfig.getRaft()); + HgKVStore opened = MetadataFactory.openStore(pdConfig); + try { + RaftEngine.getInstance().init(pdConfig.getRaft()); + } catch (RuntimeException | Error failure) { + // Only a store this call opened is ours to release; PDService and the + // shutdown hook own the rest + if (opened != null) { + try { + // Join Raft before closing metadata, as on shutdown; a failed join + // keeps the database open + RaftEngine.getInstance().shutDown(); + MetadataFactory.discardStore(opened); + } catch (RuntimeException | Error cleanup) { + if (cleanup != failure) { + failure.addSuppressed(cleanup); + } + } + } + throw failure; + } } } diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/RaftStartupTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/RaftStartupTest.java index 4d9c4eea18..8e6a441cae 100644 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/RaftStartupTest.java +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/RaftStartupTest.java @@ -17,6 +17,10 @@ package org.apache.hugegraph.pd.core; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + import java.io.File; import java.lang.reflect.Field; @@ -25,46 +29,107 @@ import org.apache.hugegraph.pd.meta.MetadataFactory; import org.apache.hugegraph.pd.raft.RaftEngine; import org.apache.hugegraph.pd.service.RaftStartup; +import org.apache.hugegraph.pd.store.HgKVStore; import org.junit.Assert; import org.junit.Test; +import org.rocksdb.Options; +import org.rocksdb.RocksDB; public class RaftStartupTest { @Test public void testRaftDoesNotStartBeforeMetadataOpens() throws Exception { - Assert.assertNull(RaftEngine.getInstance().getRaftNode()); String dataPath = "tmp/test-raft-startup"; - FileUtils.deleteQuietly(new File(dataPath)); + PDConfig config = newConfig(dataPath, "127.0.0.1:0"); // A regular file where the metadata RocksDB directory belongs makes its open fail FileUtils.forceMkdir(new File(dataPath)); FileUtils.touch(new File(dataPath, "rocksdb")); + withFactoryStore(null, () -> { + IllegalStateException e = Assert.assertThrows(IllegalStateException.class, + () -> RaftStartup.init(config)); + Assert.assertTrue(e.getMessage(), + e.getMessage().startsWith("Failed to open PD metadata")); + Assert.assertFalse(new File(dataPath, "raft").exists()); + Assert.assertNull(RaftEngine.getInstance().getRaftNode()); + }); + } + + @Test + public void testRaftFailureReleasesMetadataItOpened() throws Exception { + String dataPath = "tmp/test-raft-startup-owned"; + // An address jraft cannot parse makes Raft fail after metadata has opened + PDConfig config = newConfig(dataPath, "not-a-peer"); + withFactoryStore(null, () -> { + Assert.assertThrows(IllegalArgumentException.class, () -> RaftStartup.init(config)); + Assert.assertNull(factoryStore()); + Assert.assertNull(RaftEngine.getInstance().getRaftNode()); + // The metadata LOCK is free again + try (Options options = new Options(); + RocksDB ignored = RocksDB.open(options, dataPath + "/rocksdb")) { + Assert.assertNotNull(ignored); + } + }); + } + + @Test + public void testRaftFailureKeepsMetadataOpenedByAnotherOwner() throws Exception { + PDConfig config = newConfig("tmp/test-raft-startup-shared", "not-a-peer"); + HgKVStore shared = mock(HgKVStore.class); + withFactoryStore(shared, () -> { + Assert.assertThrows(IllegalArgumentException.class, () -> RaftStartup.init(config)); + Assert.assertSame(shared, factoryStore()); + verify(shared, never()).close(); + }); + } + + private static PDConfig newConfig(String dataPath, String raftAddress) throws Exception { + FileUtils.deleteQuietly(new File(dataPath)); PDConfig config = new PDConfig(); config.setDataPath(dataPath); PDConfig.Raft raft = config.new Raft(); raft.setDataPath(dataPath + "/raft"); + raft.setAddress(raftAddress); config.setRaft(raft); + return config; + } - Field storeField = MetadataFactory.class.getDeclaredField("store"); - Field shutdownField = MetadataFactory.class.getDeclaredField("shutdown"); - storeField.setAccessible(true); - shutdownField.setAccessible(true); + private static Object factoryStore() throws Exception { + return factoryField("store").get(null); + } + + private static Field factoryField(String name) throws Exception { + Field field = MetadataFactory.class.getDeclaredField(name); + field.setAccessible(true); + return field; + } + + private static void withFactoryStore(HgKVStore initial, Body body) throws Exception { + Assert.assertNull(RaftEngine.getInstance().getRaftNode()); + Field storeField = factoryField("store"); + Field shutdownField = factoryField("shutdown"); synchronized (MetadataFactory.class) { Object previous = storeField.get(null); boolean shutdown = shutdownField.getBoolean(null); try { - storeField.set(null, null); + storeField.set(null, initial); shutdownField.setBoolean(null, false); - IllegalStateException e = Assert.assertThrows(IllegalStateException.class, - () -> RaftStartup.init(config)); - Assert.assertTrue(e.getMessage(), - e.getMessage().startsWith("Failed to open PD metadata")); - Assert.assertFalse(new File(dataPath, "raft").exists()); - Assert.assertNull(RaftEngine.getInstance().getRaftNode()); + body.run(); } finally { + Object left = storeField.get(null); + if (left != null && left != initial) { + ((HgKVStore) left).close(); + } storeField.set(null, previous); shutdownField.setBoolean(null, shutdown); - FileUtils.deleteQuietly(new File(dataPath)); + FileUtils.deleteQuietly(new File("tmp/test-raft-startup")); + FileUtils.deleteQuietly(new File("tmp/test-raft-startup-owned")); + FileUtils.deleteQuietly(new File("tmp/test-raft-startup-shared")); } } } + + private interface Body { + + void run() throws Exception; + } }