bitflicker64 commented on code in PR #366:
URL:
https://github.com/apache/hugegraph-computer/pull/366#discussion_r4359434242
##########
vermeer/apps/worker/compute_bl.go:
##########
@@ -144,9 +157,16 @@ func (cb *ComputeBl) RunSuperStep(taskID int32,
computeValues map[string][]byte)
}
}()
computeTask := ComputeTaskMgr.GetTask(taskID)
+ if computeTask == nil {
+ return
+ }
+ computeTask.Locker.Lock()
+ computeTask.WaitingForStep = false
if !cb.CheckAction(computeTask) {
Review Comment:
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`.
##########
.asf.yaml:
##########
@@ -38,10 +38,7 @@ github:
# strict means "Require branches to be up-to-date before merging".
strict: false
# contexts are the names of checks that must pass
- contexts:
- #- computer-ci # always stuck in CI
- - check-license-header
- - Analyze (java)
+ contexts: []
Review Comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
