[imbajin]

Vermeer could not load graphs when PD required a deployment secret or Server required authentication for Schema reads. PD requests now resolve a protected password file through VERMEER_PD_PASSWORD_FILE / pd_password_file. Worker Schema reads separately resolve VERMEER_HUGEGRAPH_CREDENTIALS_FILE / hugegraph_credentials_file, without persisting deployment credentials in task parameters. Explicit invalid files fail closed; unconfigured deployments retain legacy behavior.

Storage initialization errors now propagate before scans start, release initialization waiters before reporting to Master, and leave failed graphs safe to unload and retry. A blocked status RPC cannot prevent local receivers from leaving initialization. The barrier is held before publishing a task, receivers inspect its published initialization result, and vertex frames register before waiting. Initialization retains its original barrier pointer so cancellation cleanup cannot cause a second panic after recovery.

A small authenticated GET /api/v1/workers/availability reports one readiness boolean for the credential's space allocations/common pool. It accepts no caller-selected space, requires no admin token and allocates no resources. Readiness follows explicit allocation precedence and requires every registered member of a candidate group to be Ready. Graph-specific candidates use exact keys from the authenticated space's actual graph names, without inferring ownership by prefixes. Worker connection publication shares the readiness lock and follows session construction. Both availability and scheduler admission also require the current Worker instance's three live task streams. Reverse gRPC readiness during hello is insufficient; canceled streams immediately disable readiness, cleanup preserves replacements, and missing initial dispatch streams return errors.

Validation: targeted common, initialization/Schema, receiver and scoped-readiness regressions pass with -race; the dev build passes. Local PD gRPC + HTTP Basic Schema fixtures verify credential scope. A separate Schema observer loaded a fresh 1001-vertex graph on the retained real stack, with 1000 + 1 distinct results and real Chromium pagination. Independent cross reviews checked these changes. The cleanup regression fails against the previous source with a nil WaitGroup panic and passes with the fix. A real hello regression holds the ordinary memory-limit RPC: on 554daab it falsely advertises readiness and admits dispatch before streams exist; the new checked-in regression fails there and passes with the repair under -race. On 090bf27, a fresh retained-stack run established all real task streams, reloaded the source graph and returned six finite PageRank values. Existing structure Save/Load fixtures fail with an empty DataDir on both original and modified source, so a full structure-suite pass is not claimed.

CI runs the affected regressions and loads its Buildx dry-run image before metadata checks.

PD currently uses plaintext gRPC; the trusted-network requirement is documented and its TLS thread remains open for coordinated PD/client support.

Hubble adapter, reproducible helpers and browser evidence: hugegraph/hugegraph-toolchain#84.

---

Upstream submission notes

Source community PR: hugegraph#19. This upstream PR reuses the same source repository and branch. Validation statements above are reports from the source task; they were not independently rerun during upstream submission. At submission preflight, source-head hosted checks: 7 success. Upstream exact-head CI and current review feedback require separate confirmation; opening this PR does not establish merge or release readiness.

图解

凭据隔离、初始化失败处理，以及统一的 Worker 就绪判定与调度准入条件。可用性不保证任意图立即可调度；此图不代表集群验收结果。

[imbajin]

Independent narrow workflow-delta review at cea93ff: both Docker action pins are present in the current ASF approved patterns. This also changes major action versions, rather than only pinning existing versions. Declared inputs and Node24 runner requirements were checked against the official action metadata; no concrete incompatibility was found. No runtime Go change is present in this delta. This was static verification, not a successful hosted execution; current-head CI and the wider runtime review remain separate. No whole-PR score is assigned.

[bitflicker64]

Blocking: yes. Score: 6/10 (one blocking item; 10 - 2 - 3 x 0.5 = 6.5, held at 6).

This review covers b5c1693. A security concern was shared with the maintainers privately.

The nil LoadWg crash found when this change was reviewed on hugegraph#19 is fixed: initialization keeps its own barrier pointer, and TestLoadReceiversBarrierSurvivesTaskCleanup fails if Done goes back to the task's LoadWg. The unreachable InitializationFailed checks from that review are now documented as guards. The plaintext PD transport is documented in the README as a trusted-network requirement, so I am not raising it again here.

go build and go vet on the changed packages are clean, and the five go test -race commands from vermeer-ci.yml pass locally (with -tags dev). CI is still pending.

Three Minor items are inline.

[bitflicker64]

Minor: When a later worker has no load stream, this returns after earlier workers in the loop already received LoadStep_Vertex, and nothing tells them to stop.

workerClients comes from GroupWorkers (map order), so the worker without a stream can come after one that was already sent the request. The dispatched worker runs StartLoadGraph, but once the task is in Error its FetchLoadPart calls get no partition (load_graph.go:370-372), its LoadTaskStatus reports are ignored (load_task.go:81), and no LoadStep_Error reaches it, so its load task and graph stay in memory. The SendLoadReq error branch below has the same gap.

Suggestion: before returning, send LoadStep_Error to the workers already dispatched, skipping any whose LoadServer is nil. Checking every stream before the first send is shorter but covers only this branch.

[bitflicker64]

Minor: Availability ignores a group that an admin allocated to a graph that is not loaded yet, while the scheduler uses that group for the graph's first load.

Example: groupMapper {"current newgraph": "graph-group"}, one ready worker in graph-group, no space-level group and no workers in "$". ApplyGroup("current", "newgraph") returns graph-group and its member is ready, but HasAvailableWorkers("current", nil) returns false, because graphNames only comes from GraphMgr. AdminWorkerGroupGraphAllocHandler does not require the graph to exist, so this layout is reachable, and a client that waits for this flag would not start that first load.

The PR rules out prefix matching on purpose (the "other space with shared prefix" test case), so the simplest fix is one README sentence next to the endpoint saying such a group is not counted until the graph exists.

[bitflicker64]

Minor: This test registers its own route and sets the credential itself, so nothing checks that /workers/availability sits behind the auth filter, which is the access rule the README states for this endpoint.

Through SetRouters(sen, auth.TokenFilter), the endpoint returns 401 without a token on both /api/v1/workers/availability and /workers/availability, and 200 with a signed Bearer token. So the wiring is correct today, but removing authFilters from the new regVerAPI call in router.go would still pass this test.

Suggestion: add a case that goes through SetRouters with auth.TokenFilter and expects 401 without a token. Sentinel keeps its gin engine unexported, so the test needs a small accessor or has to start the server on a loopback port.

[bitflicker64]

Blocking: yes. Score: 6/10 (10 - 2 = 8, held at 6 while the security item is open). Covers 81e1277.

The three Minor items from the last review are addressed: earlier workers now get LoadStep_Error when a later dispatch fails, the README documents the preallocated-group case, and a new test goes through SetRouters with auth.TokenFilter. A security concern was shared with the maintainers privately.

• Minor, task_bl.go partial dispatch: Fixed in 81e1277. Only workers that accepted the request are stopped, so a nil LoadServer is never used, a failed stop is logged without replacing the dispatch error, and the new subtests cover missing stream, send failure, cleanup failure and success.
• Minor, worker_manager.go preallocated group: Fixed in 81e1277 (README sentence, as suggested).
• Minor, router auth wiring untested: Fixed in 81e1277. TestWorkerAvailabilityRequiresAuthentication expects 401 without a token on both paths.
• Security concern: Still open.

Note, not scored: if an already-dispatched worker reports a load error while this loop is still sending, and a later dispatch then fails, LoadTaskStatus and the new cleanup can both send LoadStep_Error on the same stream from two goroutines. The window is small, and LoadTaskStatus could already send on these streams while this loop ran. A send mutex in LoadGraphServer.AsyncLoad would close it for every caller; the duplicate stop only makes the worker decrement TaskRunningCnt twice.

Evidence: go build ./apps/... . and go vet on the changed packages are clean; the five go test -race commands from vermeer-ci.yml pass locally as CI runs them, and both new tests are matched by the CI -run patterns. Each new test fails when its fix is reverted (rollback loop skipped, authFilters removed from the availability route). CI checks are pending.

[bitflicker64]

Blocking: no. Score: 7.5/10 (10 - 2 - 0.5). Covers 819b34d.

819b34d closes the blocking item from the last review: with a credential file set, PD discovery uses only the deployment pd_peers list and fails before any dial without it, and Schema requests no longer follow redirects. Without credential files or pd_peers, behaviour is unchanged. The send mutex in AsyncLoad fixes the concurrent LoadStep_Error sends noted last time.

• Security concern: host selection fixed. A narrower part is still open (Important, -2) and was shared with the maintainers privately.
• Partial dispatch, preallocated group, router auth test: fixed in 81e1277, unchanged.
• Not scored: findValidPD writes pdIPAddr from several goroutines (hugegraph_tools.go:100), a data race when two PDs answer. It predates the PR; worth a follow-up issue.

Evidence: build, vet and the CI race tests pass; removing the mutex, the redirect block or requireTrustedPD fails its new test. CI is pending.

[bitflicker64]

Minor: When pd_peers is set, task peers from load.hg_pd_peers and output.hg_pd_peers are dropped here without a log line or error. The stored task parameters still show a PD that was never contacted, so a setup that used per-task peers for several clusters would quietly load a same-named graph from the deployment cluster instead.

The smallest fix is a logrus.Warnf here when a task peer is not in peers (logrus needs importing). Checking membership rather than equality keeps requireTrustedPD quiet, since it always passes a configured peer. Rejecting such tasks would be stricter, but would also fail tasks that spell the same PD differently.

[bitflicker64]

Blocking: no. Score: 7.5/10 (10 - 2 - 0.5). Covers 73a6f64.

73a6f64 publishes the load and compute task streams under the same ServerManager lock that installs the dispatch entry, and moves SuperStep publication into PutSuperStepServer, so a replacement registration can no longer overtake an entry that is not yet dispatchable. The lock order is always ServerManager then workerManager, and nothing in the workers package calls back into ServerManager, so the change does not add a deadlock path. The new superstep_publication_order case fails when the PutSuperStepServer change is reverted.

• Security concern (Important, -2, shared privately): still open; the delta does not touch credential handling.
• Minor, task peers dropped silently when pd_peers is set (pd_peers.go:58): still open, no reply on the thread.
• Earlier items fixed in 81e1277 and 819b34d: unchanged.

Evidence: go vet on apps/master/bl and apps/master/workers is clean, and the five CI go test -race commands pass on a copy of 73a6f64. CI is pending.

[bitflicker64]

Blocking: no. Score: 10/10 (no open items). Covers 273e1ec.

273e1ec closes the two items left from the last review. resolvePDPeers now logs a warning when configured pd_peers replace different task peers.

• Security concern shared privately: Fixed in 273e1ec.
• Minor, task peers dropped without notice: Fixed in 273e1ec (warning).
• Earlier items fixed in 81e1277, 819b34d and 73a6f64: unchanged.
• Behaviour change, documented in the README: non-admin HugeGraph loads under token auth are now rejected, including tasks already queued. That is the intended cost of the fix, so it is not scored.
• Not scored, predates the PR: findValidPD still writes pdIPAddr from several goroutines (hugegraph_tools.go:100).

Evidence: on a copy of 273e1ec, go build and go vet are clean and the five CI go test -race commands pass, including the three new TestHugegraphLoadAuthorization tests. Removing the create-time check or the start-time recheck makes those tests fail. CI is still pending, so this stays a comment.

[bitflicker64]

Blocking: yes. Score: 6/10 (10 - 2 - 0.5 = 7.5, held at 6 while the .asf.yaml change is in). Covers 0d09ba0.

a2eec0c makes a partial compute dispatch roll back: Master records the error, then sends a bounded cancel to each worker that accepted, retrying NotFound while the worker has not created the task yet. Workers now consume a cancel that arrives while they wait for the first superstep, and CancelOnce keeps the running-task gauge from dropping twice. That logic looks right. The blocking item is the unrelated .asf.yaml change (inline).

• Earlier items fixed in 81e1277, 819b34d, 73a6f64 and 273e1ec: unchanged.
• Note, not scored: ControlTask now returns NotFound for an unknown task instead of success. On the Master cancel path (task_canceler.go:155) that makes cancel of a task some worker never created fall through to setCancelDone, which looks like the better outcome, but it also logs a "cancel task error" for that worker.
• Not scored, predates the PR: findValidPD still writes pdIPAddr from several goroutines (hugegraph_tools.go:100).
• c2c18a5 adds Hubble integration notes and three screenshots (about 312 KB) under vermeer/docs; fine as evidence, no finding.

Evidence: on a copy of 0d09ba0, go build and go vet (bl, worker) are clean and the five CI go test -race commands pass, including TestComputeDispatchCancellation and the new compute_dispatch subtests. Removing the rollback append or the idle cancel handoff makes those tests fail. GitHub runs for this head are still queued.

[bitflicker64]

Blocking: this empties the required status checks for master, so check-license-header and Analyze (java) stop gating every future PR in this repository.

The checks themselves work. On master the last license checker run passed (2026-10-11), and on this PR the license checker passed at 04:21 and CodeQL passed at 06:41, 08:19 and 09:07 today. What keeps this PR's checks pending is that each push cancels the previous license checker and Vermeer CI runs, and the new runs are still queued. Removing the requirement lets the PR merge without those results; it does not make them run.

Please drop this commit from #366. If the required list should change (for example because Analyze (java) never reports on Go-only PRs), that fits better in its own PR, and the license header check should stay required either way.

[bitflicker64]

Minor: CheckAction runs with computeTask.Locker held, so if the task is paused here, pauseAction sleeps in 10-second steps with the lock held until a resume or cancel arrives. A cancel through ControlTask then waits up to 10 s for the lock before it answers Master, and a StepEnd that passed its own CheckAction before the pause blocks at line 420. Nothing deadlocks, because SetAction runs before the lock in ControlTask.

Here the lock only needs to cover clearing WaitingForStep: once it is false, ControlTask leaves the cancel to this goroutine, and CancelOnce covers a double consume, so CheckAction can move after the Unlock. In StartCompute the check has to stay under the lock; on pause, unlock, call pauseAction, then re-lock and check again before setting WaitingForStep.