bitflicker64 commented on code in PR #814:
URL:
https://github.com/apache/hugegraph-toolchain/pull/814#discussion_r4269196606
##########
hugegraph-hubble/hubble-be/src/main/java/org/apache/hugegraph/service/load/LoadTaskService.java:
##########
@@ -350,9 +398,7 @@ public LoadTask resume(int taskId, String token) {
task.getOptions().incrementalMode = true;
task.reconnect(token);
task.setStatus(LoadStatus.RUNNING);
- this.update(task);
- this.taskExecutor.execute(task, () -> this.update(task));
- this.runningTaskContainer.put(taskId, task);
+ this.updateAndExecute(task);
Review Comment:
Important: with this PR, `resume` can skip rows that were never inserted.
Every task exit now goes through `loader.shutdown()` -> `drainThenClose()` ->
`LoadContext.close(true)`, which writes a `load-progress` checkpoint
(`LoadContext.java:293`) unless `failLoading` recorded a failure. At the base
commit, no Hubble code called `shutdown()` or `close()` on a Loader, so a
running Hubble never wrote a checkpoint, and `resume` (which sets
`incrementalMode = true` on line 398) read the whole file again. Now it starts
from the checkpoint. The checkpoint marks lines as done once they are parsed
and submitted, not once they are inserted: the `InputItemProgress` comment
(lines 24-29) says a file whose inserts partly failed is marked loaded and
skipped next time, and `FileLoadTest.testLoadIncrementalModeAndLoadFailure`
depends on that.
Two normal paths lose rows:
1. Pause, then resume. When inserts are slower than parsing, the parse
thread usually waits in `batchSemaphore.acquire()` (`TaskManager.java:243`, 5
permits with Hubble's 4 batch threads). `pause` calls `stopLoading()`. When the
waiting batch gets its permit, it reaches `if (this.context.stopped())` (line
256) and goes to the failure log as `Loading stopped before batch submission`,
so it is never sent. `loadStruct` then calls `markLoaded` for those lines
(`HugeGraphLoader.java:804`), and `close()` saves that offset. `resume` skips
them: up to `batchSize` (100) lines per mapping for each paused parse thread.
2. Resume after FAILED. Single-insert failures
(`SingleInsertTask.handleInsertFailure`), parse failures and read failures only
call `occurredError()`, so `close()` still writes the checkpoint, with the file
marked loaded or at its last parsed offset. `resume` skips those lines and the
task ends `SUCCEED` without them. Hubble has no `--failure-mode` path to load
the failure files again.
Requested change: keep the base checkpoint behaviour for Hubble task exits
in this PR. For example, let Hubble shut the Loader down without writing
progress (the new `LoadContext.close(boolean saveProgress)` already supports
this) until a checkpoint covers only rows that were inserted. Alternatively,
stop `markLoaded` from covering batches recorded as `stopped before batch
submission`, and make resuming a FAILED task read from the start. Please add a
pause-then-resume test that pauses while the parse thread waits on the batch
semaphore and checks that every row is present after resume.
--
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]
