Skip to content

Commit ed3a2df

Browse files
authored
refactor(store): drain TTL native resources on shutdown (#3304)
Shutdown waits for TTL workers and their native cleanup after RPC/Scan ownership drains. TTL closes its scan and borrowed session lease in order, attempts both releases, and retains cleanup failures as shutdown blockers. The distribution stop script reports timeouts explicitly. Native reference-count, close-failure, active-read and shutdown regressions ship here. A real nonempty TTL scan with interrupted Spring close verifies that the database stays alive while the reader is blocked, then checks iterator/session drain before bean destruction.
1 parent fd64f74 commit ed3a2df

14 files changed

Lines changed: 1594 additions & 24 deletions

File tree

‎.github/workflows/pd-store-ci.yml‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,19 @@ permissions:
88
contents: read
99

1010
jobs:
11+
store-stop-shell-test:
12+
permissions:
13+
contents: read
14+
runs-on: ubuntu-24.04
15+
steps:
16+
- name: Checkout
17+
uses: actions/checkout@v7
18+
with:
19+
persist-credentials: false
20+
21+
- name: Verify Store stop failure propagation and PID retention
22+
run: bash hugegraph-server/hugegraph-dist/src/assembly/travis/test-stop-hugegraph-store.sh
23+
1124
rocksdb-compatibility:
1225
permissions:
1326
contents: read
@@ -431,7 +444,7 @@ jobs:
431444
mvn test -pl hugegraph-store/hg-store-test -am \
432445
-P store-core-test -Djacoco.sessionId=store-core-test
433446
434-
- name: Run server lifecycle test
447+
- name: Run server lifecycle and shutdown tests
435448
run: |
436449
mvn test -pl hugegraph-store/hg-store-test -am \
437450
-P store-server-test -Djacoco.sessionId=store-server-test

‎docs/storage-lifecycle.md‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ atomic restore across all databases in a graph.
1212

1313
- [RPC and Scan ownership](#rpc-and-scan-ownership): final batches, half-close,
1414
cancellation, iterator cleanup and callback dispatch.
15-
- [Shutdown coordination](#shutdown-coordination): admission, worker drain and
16-
application callbacks before database teardown.
15+
- [Shutdown coordination](#shutdown-coordination): admission, worker drain, TTL cleanup
16+
and [Store node shutdown](#store-node-shutdown).
1717
- [Standalone snapshot recovery](#standalone-snapshot-recovery): pending records,
1818
checkpoint identity, WAL paths and safe retry.
1919
- [Storage provider selection and ToplingDB](#storage-provider-selection-and-toplingdb):
@@ -114,6 +114,38 @@ releases graph database and schema resources. Keep this ordering when modifying
114114
database teardown. Request drain and database release have different responsibilities;
115115
reuse their existing owners instead of adding a parallel registry.
116116

117+
### Store node shutdown
118+
119+
Spring coordinates Store shutdown: new RPCs are refused, active requests are cancelled,
120+
and their application callbacks and scan workers are drained before database teardown.
121+
The shutdown coordinator continues cancellation and independent cleanup even if one
122+
callback fails. It reuses the existing per-call owners.
123+
124+
TTL cleanup closes the scan before returning its borrowed RocksDB session lease. Both
125+
releases are attempted when either fails. The first cleanup failure is retained, later
126+
failures are attached, and completion is signalled after these release attempts.
127+
Executor termination alone does not establish that native cleanup succeeded.
128+
129+
Shutdown waits for TTL cleanup as well as ordinary request callbacks. Failed native
130+
releases remain diagnostic drain blockers. The engine stops heartbeat producers and
131+
joins partition Raft services before releasing native databases. Do not add a concurrent
132+
JVM shutdown hook that closes those databases independently of this drain.
133+
134+
Cancellation can fail in-flight writes. Before planned maintenance, stop new writes and
135+
check outstanding write outcomes.
136+
137+
For an unpacked distribution, invoke its normal stop script:
138+
139+
```bash
140+
bash bin/stop-hugegraph-store.sh
141+
```
142+
143+
The script waits up to 30 seconds for process exit. A timeout returns a nonzero status
144+
and retains `bin/pid` for diagnosis; it does not force-kill the process or remove data.
145+
A stuck callback or failed iterator close can keep shutdown pending. Inspect Store logs
146+
and a JVM thread dump to identify the remaining owner before retrying. Preserve the data,
147+
WAL and Raft directories, and do not bypass the resource drain to make the process exit.
148+
117149
## Standalone snapshot recovery
118150

119151
### Installation and pending metadata

‎hugegraph-server/hugegraph-dist/src/assembly/travis/run-native-runtime-smoke-test.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ cleanup() {
3939
local status=$?
4040
trap - EXIT
4141
if [[ "$SERVER_START_ATTEMPTED" == "true" ]]; then
42-
"$SERVER_DIR/bin/stop-hugegraph.sh" -m false >/dev/null 2>&1 || status=1
42+
"$SERVER_DIR/bin/stop-hugegraph.sh" -m false || status=1
4343
fi
4444
rm -rf "$WORK_DIR"
4545
exit "$status"
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
#!/bin/bash
2+
#
3+
# Licensed to the Apache Software Foundation (ASF) under one or more
4+
# contributor license agreements. See the NOTICE file distributed with
5+
# this work for additional information regarding copyright ownership.
6+
# The ASF licenses this file to You under the Apache License, Version 2.0
7+
# (the "License"); you may not use this file except in compliance with
8+
# the License. You may obtain a copy of the License at
9+
#
10+
# http://www.apache.org/licenses/LICENSE-2.0
11+
#
12+
# Unless required by applicable law or agreed to in writing, software
13+
# distributed under the License is distributed on an "AS IS" BASIS,
14+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
# See the License for the specific language governing permissions and
16+
# limitations under the License.
17+
#
18+
# Verify stop failure propagation and PID retention without signalling real processes.
19+
set -euo pipefail
20+
21+
ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../../../../.." && pwd)"
22+
SCRIPT="$ROOT/hugegraph-store/hg-store-dist/src/assembly/static/bin/stop-hugegraph-store.sh"
23+
FIXTURE=$(mktemp -d)
24+
trap 'rm -rf "$FIXTURE"' EXIT
25+
mkdir -p "$FIXTURE/bin"
26+
cp "$SCRIPT" "$FIXTURE/bin/stop-hugegraph-store.sh"
27+
cat > "$FIXTURE/bin/util.sh" <<'UTIL'
28+
kill_process_and_wait() {
29+
[[ "$1" == HugeGraphStoreServer && "$2" == 12345 && "$3" == 30 ]] || return 99
30+
return "$STOP_RESULT"
31+
}
32+
UTIL
33+
printf '12345\n' > "$FIXTURE/bin/pid"
34+
if STOP_RESULT=1 bash "$FIXTURE/bin/stop-hugegraph-store.sh"; then
35+
echo "Stop timeout incorrectly returned success" >&2
36+
exit 1
37+
fi
38+
[[ "$(cat "$FIXTURE/bin/pid")" == 12345 ]]
39+
STOP_RESULT=0 bash "$FIXTURE/bin/stop-hugegraph-store.sh"
40+
[[ ! -e "$FIXTURE/bin/pid" ]]
41+
STOP_RESULT=0 bash "$FIXTURE/bin/stop-hugegraph-store.sh"
42+
echo "store-stop-contract-ok"

‎hugegraph-store/README.md‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -400,10 +400,15 @@ For Docker and Kubernetes deployment details, see [Deployment Guide](docs/deploy
400400

401401
---
402402

403+
## Stopping a Store node
404+
405+
See [Store-wide shutdown](../docs/storage-lifecycle.md#store-node-shutdown) for stop commands,
406+
timeout handling, diagnostics and resource drain behavior.
407+
403408
## Documentation
404409

405-
See [RPC and Scan lifecycle](../docs/storage-lifecycle.md#rpc-and-scan-ownership) for request ownership,
406-
cancellation and feedback semantics.
410+
See the [storage lifecycle guide](../docs/storage-lifecycle.md) for request ownership,
411+
shutdown, recovery and provider configuration.
407412

408413
Comprehensive documentation for HugeGraph Store:
409414

‎hugegraph-store/hg-store-dist/src/assembly/static/bin/stop-hugegraph-store.sh‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ if [ ! -f ${PID_FILE} ]; then
4141
fi
4242

4343
PID=`cat $PID_FILE`
44-
kill_process_and_wait "HugeGraphStoreServer" "$PID" "$SERVER_SHUTDOWN_TIMEOUT_S"
45-
46-
if [ $? -eq 0 ]; then
44+
if kill_process_and_wait "HugeGraphStoreServer" "$PID" "$SERVER_SHUTDOWN_TIMEOUT_S"; then
4745
rm "$PID_FILE"
46+
else
47+
exit 1
4848
fi

‎hugegraph-store/hg-store-node/src/main/java/org/apache/hugegraph/store/node/listener/ContextClosedListener.java‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -69,13 +69,9 @@ public void onApplicationEvent(ContextClosedEvent event) {
6969
}
7070
this.grpcServers.forEach(Server::shutdownNow);
7171
if (cleaner != null) {
72-
// TTL failure ownership and worker drain are added by the native shutdown PR.
73-
if (cleaner.getExecutor() != null) {
74-
cleaner.getExecutor().shutdownNow();
75-
}
76-
if (cleaner.getScheduler() != null) {
77-
cleaner.getScheduler().shutdownNow();
78-
}
72+
// The scheduler can create the worker pool while a job is starting.
73+
stopAndWait(cleaner.getScheduler(), "TTL scheduler");
74+
stopAndWait(cleaner.getExecutor(), "TTL workers");
7975
}
8076
if (storeStream != null) {
8177
// Cancelled queued scans must run their finally blocks to release iterators.
@@ -108,7 +104,18 @@ public void onApplicationEvent(ContextClosedEvent event) {
108104
if (storeStream != null) {
109105
storeStream.awaitScanCleanup();
110106
}
111-
log.info("closed gRPC callbacks, scan and aggregate query workers");
107+
if (cleaner != null) {
108+
cleaner.awaitCleanup();
109+
}
110+
log.info("closed gRPC callbacks, scan, aggregate query and TTL workers");
111+
}
112+
113+
private static void stopAndWait(ExecutorService executor, String name) {
114+
if (executor == null) {
115+
return;
116+
}
117+
executor.shutdownNow();
118+
awaitWorkers(executor, name);
112119
}
113120

114121
private static void awaitWorkers(ExecutorService executor, String name) {

‎hugegraph-store/hg-store-node/src/main/java/org/apache/hugegraph/store/node/task/TTLCleaner.java‎

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import java.util.concurrent.TimeUnit;
3434
import java.util.concurrent.atomic.AtomicBoolean;
3535
import java.util.concurrent.atomic.AtomicLong;
36+
import java.util.concurrent.atomic.AtomicReference;
3637
import java.util.function.BiFunction;
3738

3839
import org.apache.commons.lang3.ArrayUtils;
@@ -97,6 +98,7 @@ public class TTLCleaner implements Runnable {
9798
@Autowired
9899
private HgStoreNodeService service;
99100
private final AtomicBoolean running = new AtomicBoolean(false);
101+
private final AtomicReference<Throwable> cleanupFailure = new AtomicReference<>();
100102

101103
private final AppConfig appConfig;
102104
private final AppConfig.JobConfig jobConfig;
@@ -285,11 +287,12 @@ private Runnable getTask(
285287
String table = t.getRight();
286288
TaskInfo taskInfo = counter.get(graph);
287289
ScanIterator scan = null;
290+
RocksDBSession session = null;
288291
try {
289292
Map<String, AtomicLong> graphCounter = taskInfo.getTableCounter();
290293
TaskSubmitter submitter = taskInfo.getTaskSubmitter();
291294
AtomicLong tableCounter = graphCounter.get(table);
292-
RocksDBSession session = handler.getSession(id);
295+
session = handler.getSession(id);
293296
InnerKeyCreator keyCreator = handler.getKeyCreator();
294297
SessionOperator op = session.sessionOp();
295298
BiFunction<byte[], byte[], Boolean> judge = getJudge(graph, table);
@@ -301,7 +304,7 @@ private Runnable getTask(
301304
LinkedList<ByteString> all = new LinkedList<>();
302305
AtomicBoolean state = new AtomicBoolean(true);
303306
AtomicLong partitionCounter = pc.get(id);
304-
while (filter.hasNext() && state.get()) {
307+
while (!Thread.currentThread().isInterrupted() && state.get() && filter.hasNext()) {
305308
RocksDBSession.BackendColumn current = filter.next();
306309
byte[] realKey =
307310
Arrays.copyOfRange(current.name, 0, current.name.length - Short.BYTES);
@@ -315,7 +318,7 @@ private Runnable getTask(
315318
all = new LinkedList<>();
316319
}
317320
}
318-
if (all.size() > 0 && state.get()) {
321+
if (!Thread.currentThread().isInterrupted() && all.size() > 0 && state.get()) {
319322
submitter.submitClean(id, graph, table, all, state, tableCounter,
320323
partitionCounter);
321324
}
@@ -326,14 +329,59 @@ private Runnable getTask(
326329
String msg = String.format(s, id, graph, table);
327330
log.error(msg, e);
328331
} finally {
329-
latch.countDown();
330-
if (scan != null) {
331-
scan.close();
332+
try {
333+
if (scan != null) {
334+
scan.close();
335+
}
336+
} catch (RuntimeException | Error failure) {
337+
recordCleanupFailure(failure);
338+
} finally {
339+
try {
340+
if (session != null) {
341+
session.close();
342+
}
343+
} catch (RuntimeException | Error failure) {
344+
recordCleanupFailure(failure);
345+
} finally {
346+
latch.countDown();
347+
}
332348
}
333349
}
334350
};
335351
}
336352

353+
private void recordCleanupFailure(Throwable failure) {
354+
this.cleanupFailure.compareAndSet(null, failure);
355+
Throwable first = this.cleanupFailure.get();
356+
if (first != failure) {
357+
first.addSuppressed(failure);
358+
}
359+
log.error("TTL native cleanup failed; database close stays blocked", failure);
360+
}
361+
362+
/** Call after workers terminate: a failed native release must remain a shutdown blocker. */
363+
public void awaitCleanup() {
364+
boolean interrupted = false;
365+
long nextLog = System.nanoTime() + TimeUnit.SECONDS.toNanos(5);
366+
synchronized (this.cleanupFailure) {
367+
while (this.cleanupFailure.get() != null) {
368+
try {
369+
this.cleanupFailure.wait(5000L);
370+
} catch (InterruptedException e) {
371+
interrupted = true;
372+
}
373+
if (System.nanoTime() - nextLog >= 0) {
374+
log.warn("TTL scan cleanup failed; database close stays blocked",
375+
this.cleanupFailure.get());
376+
nextLog = System.nanoTime() + TimeUnit.SECONDS.toNanos(5);
377+
}
378+
}
379+
}
380+
if (interrupted) {
381+
Thread.currentThread().interrupt();
382+
}
383+
}
384+
337385
public ScheduledFuture<?> getFuture() {
338386
return future;
339387
}

0 commit comments

Comments
 (0)