Repository navigation
refactor(hubble): close the async executor - #818
Conversation
- require confirmed loading mode before cleanup - keep cleanup state local and reset it on clone - cover failed startup and cleanup error paths - document network uncertainty and shared mode limits
- log the graph when a mode write loses its response - preserve the original checked transport exception - retain cleanup ownership and avoid blind mode resets - verify the diagnostic and explain manual recovery
- include the latest mode acquisition guard - preserve current ASF launcher and fixture fixes - keep Hubble changes available for review
- retain loader ownership through cleanup and recovery - preserve typed file progress during cancellation - serialize job admission with metadata deletion - verify transaction rollback and stale callbacks
- align the Hubble cleanup branch with current master - preserve the validated Loader and Hubble source bytes - retain lifecycle and checkpoint regression coverage
- Require graph-instance loading owner capability. - Reuse one owner tuple for uncertain request outcomes. - Release ownership after all loader workers terminate. - Document server compatibility and orphan recovery.
- Lock Loader SDK and runtime to the protocol-1 Server. - Retain all import profiles on the matching candidate. - Verify official Server 1.7 rejects import before mutation. - Record per-consumer source inputs and document coverage.
- route Ingest and graph cleanup through scoped job deletion - preserve permission and remote-drop failure boundaries - retry rejected dispatch after SQL and commit failures - isolate polling writes and retire owners after commit - document and verify deletion and recovery contracts
- propagate Spring transaction and savepoint failures - retain earlier owners when the polling transaction aborts - verify real deadlocks and ordinary SQL failure isolation - document the polling rollback boundary
- Confirm protocol support before every import mode. - Clean RESTORING only after a successful enum change. - Match Hubble and Loader image import fixtures to Server. - Verify legacy refusal while preserving traversal and UI checks.
- Preserve loading ownership and restoring cleanup. - Retain released SDK helpers in source assemblies. - Include the upstream JSON list parser and its coverage.
- inherit merged source release helper migration - retain upstream JSON list support - preserve reviewed Hubble deletion and recovery changes
Merge the published Hubble cleanup dependency normally. Preserve the reviewed loading-owner and CI files. Keep current Apache master and source-package helpers.
- use deletion-specific stopping while keeping stop requests strict - preserve completed states and wait for existing owner cleanup - cover H2 completion races, closure refusal and completion waits - document deletion of tasks that finish after enumeration
- Pin Loader and Hubble imports and Loader image smoke to the paired Server fix. - Keep existing SDK defaults and official Server 1.7 refusal controls. - Verify workflow syntax, source resolution and frozen source/class bindings.
Keep fixed owner handles for acquire and cleanup Preserve legacy imports and RESTORING behavior Remove unrelated Hubble and checkpoint changes
- close task and example loaders after loading - close unused loaders on save, dispatch or transaction failure - remove owned shutdown hooks and preserve unused checkpoints - defer metadata and concurrency recovery to separate work
Limit manual NONE recovery to legacy Servers Explain same-tuple and same-principal REST release Document that repeated shutdown does not retry release
- limit unused-loader cleanup to initial task creation - retain task-exit and built-in example guidance - leave resume and retry failures in the deferred scope
Keep the original owner tuple in the primary release error Attach context cleanup failures as suppressed exceptions Cover lost replies, logger errors and successful release
Keep owner tokens out of diagnostics. Document privileged recovery with epoch and revision checks. Exercise legacy Server imports in release CI. Verify owner requests tolerate revision capabilities.
Close loaders when resume or retry dispatch fails. Preserve stopped states when shutdown rethrows the load failure. Report independent cleanup errors without replacing the primary cause. Cover native loader exception and dispatch boundaries.
Register the executor with Spring lifecycle management. Verify real async workers stop with their context. Document executor ownership and register suite coverage.
imbajin
left a comment
imbajin
left a comment
There was a problem hiding this comment.
P2 — Pause load tasks before waiting for the managed executor to stop
Reviewed commit: 550bb42.
Registering this executor as a bean in AsyncConfig.java:33–40 adds a shutdown wait before Hubble's existing cooperative-pause hook. In Spring 6.2.19, the executor's SmartLifecycle stop callback waits for its executing tasks to finish, and application-context shutdown completes that lifecycle phase before invoking DisposableBean.destroy(). Hubble only calls pauseAllTasks() from HubbleDisposableBean.destroy(), so a long-running import is still running while Spring waits for it.
The defaults conflict here: Boot 3.5.16 allows 30 seconds for the lifecycle phase, while the packaged stop-hubble.sh also waits 30 seconds before SIGKILL. A long-running import can therefore consume the entire graceful-stop budget before Hubble requests its pause and persists the resulting state. The added test drains every future before context.close(), so it cannot exercise this ordering.
Could the shutdown flow request cooperative pause/drain before waiting for this managed executor, and add a real Hubble-context regression with both a long-running import and queued work under the packaged stop deadline? Merely increasing a timeout would leave the ordering issue in place.
Sources at the reviewed version:
- HubbleDisposableBean.java:34–38 and LoadTaskService.java:312–339: cooperative pause during destruction
- hugegraph-hubble/hubble-dist/assembly/static/bin/stop-hubble.sh:23,63–78: packaged stop deadline
- https://github.com/spring-projects/spring-framework/blob/v6.2.19/spring-context/src/main/java/org/springframework/context/support/AbstractApplicationContext.java#L1193-L1204
- https://github.com/spring-projects/spring-framework/blob/v6.2.19/spring-context/src/main/java/org/springframework/scheduling/concurrent/ExecutorLifecycleDelegate.java#L84-L98
- https://github.com/spring-projects/spring-boot/blob/v3.5.16/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/context/LifecycleProperties.java#L31-L43
Evidence boundary: independently cross-checked source and framework lifecycle semantics; not a reproduced full Hubble shutdown test. This finding concerns the active-import shutdown ordering and deadline. It does not claim that every shutdown hangs, that queued execution always freezes, or that persisted RUNNING tasks lack recovery.
- retain independent mode reset errors during shutdown - preserve direct fatal cleanup errors and identity - prevent cyclic cleanup exception diagnostics - cover actual loader resource cleanup and reset failures
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #818 +/- ##
============================================
+ Coverage 47.06% 49.48% +2.42%
- Complexity 4518 4874 +356
============================================
Files 602 603 +1
Lines 29452 30174 +722
Branches 3456 3636 +180
============================================
+ Hits 13862 14933 +1071
+ Misses 14308 13843 -465
- Partials 1282 1398 +116 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
bitflicker64
left a comment
bitflicker64
left a comment
There was a problem hiding this comment.
Blocking: yes. Score: 6/10 (10 - 2 queued-task shutdown hang - 2 already-raised lifecycle ordering = 6; cap 6 for blocking). Summary: Registering the executor as a bean does let idle workers exit, but the Spring lifecycle stop and shutdownNow() now run before Hubble's own load-task pause, so a queued import makes shutdown hang and a long import can lose its PAUSED state. Evidence: static trace of AsyncConfig, LoadTaskExecutor, LoadTaskService.pause/pauseAllTasks, LoadTask.run/stop and HubbleDisposableBean; a focused probe on Spring 6.2.19 with AsyncConfig at this head, a DisposableBean and 8 blocking plus 1 queued @async task: lifecycle stop timed out on getAsyncExecutor, the running workers were interrupted, the DisposableBean ran after the executor shutdown, and the queued task never executed. CI is green.
Already raised by @imbajin: the executor lifecycle stop waits for active imports before HubbleDisposableBean.destroy() requests the pause, which can use the whole 30 s stop budget.
- identify the checkpoint fixture as a legacy Server - preserve strict ownership endpoint fallback in the SDK - keep native parser and resume regressions reachable
- inherit the SDK layer legacy endpoint fixture response - retain the verified production cleanup implementation - keep the async-specific layer limited to six files
- hold active mock runs until every stop is requested - reuse the existing latch to preserve queued-first ordering - retain cleanup assertions and production behavior
- inherit the shared stop fanout latch in the fixture - preserve SDK ownership and checkpoint behavior - retain the original stacked dependency ancestry
- inherit the ordered admission fixture from the SDK - keep async shutdown and cleanup production code unchanged - preserve the six-file async layer and stack ancestry
- adopt merged CDC validation and Go dependency selectors - retain the current Hubble cleanup implementation - create a native synchronization event for the updated base
- inherit the verified Hubble base synchronization - retain the existing SDK ownership implementation - preserve the twelve-file SDK layer above its dependency
- inherit the SDK and current master synchronization - retain the verified async shutdown implementation - preserve the six-file async layer above the SDK
- use standard suppression with a direct identity guard - remove recursive throwable graph scanning - retain failure identity and native cleanup coverage
- inherit the Loader suppression simplification - retain the existing Hubble shutdown lifecycle - keep the SDK dependency history connected
- document unbounded import and metadata task draining - point operators to deployment stop timeouts - describe forced exit cleanup and metadata recovery
- reuse existing uploading directories for later chunks - retain first reservation alias checks and upload quotas - verify real chunks, alias rejection and operation counts
- inherit upload reservation reuse across file chunks - retain standard Loader cleanup failure suppression - preserve the existing SDK ownership projection
- inherit the verified upload reservation reuse - preserve Loader cleanup failure simplification - retain the Hubble shutdown contract and docs
- sync the merged ASF CDC fixture helper - retain the verified upload reservation simplification - inherit matching CI selection regression coverage
- inherit the exact ASF master helper integration - retain SDK ownership and failure suppression - preserve the verified upload optimization
- inherit the merged ASF CDC fixture helper - preserve async shutdown and operational docs - retain the verified Loader and upload projections
- await the rollback task pause before releasing cleanup - keep its accepted ticket present in the shutdown snapshot - retain the existing status and native cleanup assertions
Inherit current master and merged Hubble task cleanup. Preserve owner-specific legacy endpoint test fixtures. Retain merged cleanup cycle guards and regression cases.
- merge the resolved loader ownership baseline - retain the six shutdown-specific files unchanged - preserve merged cleanup cycle protection and tests
imbajin
changed the title
imbajin
left a comment
imbajin
left a comment
There was a problem hiding this comment.
Historical independent lifecycle review of the reviewed revision, merged on 2026-10-16 at 16:34 UTC: 9/10, a provisional bounded assessment, not a merge recommendation. Two independent full source/lifecycle reviews found no new confirmed defect.
A fresh real-Spring 6.2.19 probe against the current AsyncConfig passed five controlled scenarios: eight active tasks plus one queued task; a blocked final callback with datasource lifetime preserved; interruption before/during drain; hook/worker failure; and native shutdown throwing after initiation. The probe used JDK 21 with Java 17 source/target and controlled lifecycle beans. It did not run the full 36 JUnit tests, packaged Java 17 Hubble/Server/H2 acceptance or a throughput benchmark. The documented indefinite-drain contract remains; this is not a universal shutdown-time guarantee.
Current-head CI failed in scope selection with StaleInputError after the live PR had already merged. No shutdown regression was observed in that failure. Hubble, Loader and Spark jobs were skipped, so they provide no green runtime validation for this head. This comment records historical review evidence only; deployed-stack, crash and same-RC acceptance remain separate.
4 participants
Purpose of the PR
Hubble's manually initialized async pool could outlive context close. Managing it with Spring's default policy can instead interrupt active imports or discard queued imports before Hubble pauses them.
Dependency and comparison scope
#814, #815, and the paired Server #3331 are merged. This PR now compares only its six shutdown-specific files against master. Refreshing the merged #815 baseline changes commit ancestry only: the complete source tree is identical to the preceding tested revision.
Main Changes
Keep the existing pool and queue sizing. Pause accepted imports and drain native Loader cleanup plus final metadata callbacks before executor destruction; preserve queued work and keep the datasource available until the managed executor finishes draining.
This inherits #814's checkpoint, deletion and recovery contract and #815's fixed ownership handles. Deleting metadata after an abnormal restart does not reset Server graph mode or release stranded Server ownership; unresolved live-process cleanup remains visible.
While ownership handles are active, the Server rejects every enum graph-mode PUT for that graph, including NONE, LOADING and RESTORING. Avoid running the Spark connector, older Loaders or restore imports concurrently with owned imports; stop and drain the owned imports, release or recover ownership, then retry through the existing client workflow.
Verifying these changes
The final stack's Spring controls cover active and queued tasks, pause-before-pool-stop ordering, pending admission/transactions, callback failures and datasource lifetime. Native Loader/H2 controls exercise actual cleanup and final callback boundaries. After refreshing the merged baseline, Java 17 compilation and the focused shutdown, executor lifecycle, checkpoint and task-service regressions pass; #815 also passes its focused Client/Loader unit regressions.
Before this baseline refresh, the packaged application passed the approved runtime gates with the paired Server. The six shutdown-specific files, including the rollback synchronization fixture, are unchanged by the refresh. The existing legacy same-job path starts nine CSV mappings: eight active imports and one queued task. The original
stop-hubble.shexits naturally in about 4.1 seconds within its default 30-second deadline, without SIGKILL. Independent post-exit H2 reads find every task PAUSED, while native-resource and Server reads confirm completed cleanup, expected data and final NONE mode.Chrome separately completes three independent single-CSV jobs with direct Server data readback. This UI control does not claim same-job multi-file submission or observed UI overlap. Distributed and abrupt-crash scenarios remain outside this validation. New-head CI remains to be checked after this source update.
CI diagnosis and performance scope
The previous revision's only failed check was Spark Loader / release-1.7. During the locked candidate SDK build, Maven Central returned HTTP 502 for
net.bytebuddy:byte-buddy:1.17.6. The job stopped before starting the release Server or running Spark tests. The same revision passed Hubble on both candidate and release-1.7 baselines, as well as candidate Spark Loader. This is a dependency-download failure, with no Hubble async failure in that job. The original failed job has been retried once at its original source revision (run attempt 2), to verify the download recovery. The refreshed revision's CI must finish separately; skipped checks do not establish that the failed Spark check passed.The core/max thread counts (8/16), queue capacity (1024), and normal submission/execution path are unchanged. Spring manages initialization and destruction of the existing executor. The added wait runs during destruction, after import admissions stop and native cleanup/final callbacks drain; it does not add a per-row lock or wait. This supports expecting no material steady-state throughput change, but no throughput/latency benchmark is claimed. Shutdown can take longer, and can wait indefinitely if a native worker remains stuck; the README documents the external stop deadline and forced-exit limitation.
Does this PR potentially affect the following parts?
Documentation Status
Doc - TODODoc - Done: Hubble README.Doc - No NeedPaired PR: hugegraph#70
Conceptual overview
Conceptual diagram; not runtime test evidence. Shutdown and UI observations are author-reported; they do not establish independent acceptance or a universal 30-second guarantee.