github-actions[bot] commented on code in PR #68007:
URL: https://github.com/apache/doris/pull/68007#discussion_r4022674298
##########
fe/fe-core/src/main/java/org/apache/doris/job/offset/s3/S3SourceOffsetProvider.java:
##########
@@ -176,6 +186,9 @@ public void fetchRemoteMeta(Map<String, String> properties)
throws Exception {
@Override
public boolean hasMoreDataToConsume() {
+ if (hasReachedEnd()) {
Review Comment:
[P2] Compare cursors using the filesystem's UTF-8 order
After this terminal guard, readiness is decided by
`currentOffset.endFile.compareTo(maxEndFile)`, but S3 pagination orders raw
keys by unsigned UTF-8 bytes. The existing filesystem test provides a
counterexample: U+E000 sorts before U+1F600 in UTF-8, while Java compares
U+E000 after the emoji's leading UTF-16 surrogate. With `s3.max_batch_files=1`,
the first key commits with `lastBatch=false`, yet the successor is delayed
forever; every metadata probe recreates the same reversed pair, so the second
key is never ingested and the job never finishes. Please derive readiness from
explicit cursor/exhaustion state or use the filesystem's UTF-8 comparator, and
cover this sequence through scheduling.
##########
fe/fe-core/src/main/java/org/apache/doris/job/offset/s3/S3SourceOffsetProvider.java:
##########
@@ -186,6 +199,11 @@ public boolean hasMoreDataToConsume() {
return false;
}
+ @Override
+ public boolean hasReachedEnd() {
+ return onceMode && currentOffset != null &&
currentOffset.isLastBatch();
Review Comment:
[P2] Finish recovered nonterminal jobs when their tail disappears
This check only recognizes a terminal bit written by a committed task. If
`a` committed with `lastBatch=false`, the FE restarts, and the remaining `b` is
deleted, recovery restores `a` but not transient `maxEndFile`; the empty
metadata probe updates neither field. `hasMoreDataToConsume()` and this method
then both stay false, so the placeholder task is delayed forever even though
the PR promises recovered jobs finish when no files remain. This is distinct
from the existing initial-empty failure and finished-placeholder threads: here
a valid committed offset never reaches FINISHED. Please persist an exhausted
state when a post-cursor ONCE probe is empty, and cover nonterminal commit ->
tail deletion -> image recovery.
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/insert/streaming/StreamingJobSchedulerTask.java:
##########
@@ -74,9 +74,7 @@ private void handlePendingState() throws JobException {
}
if (streamingInsertJob.hasReachedEnd()) {
// Source already fully consumed (e.g. snapshot-only mode
recovered after FE restart).
- // Transition directly to FINISHED without creating a new task.
- streamingInsertJob.updateJobStatus(JobStatus.FINISHED);
- streamingInsertJob.logUpdateOperation();
+ streamingInsertJob.tryFinishJob();
Review Comment:
[P2] Restore the local final task's success time before finishing
A local FE can crash after the final COMMITTED transaction is journaled but
before `onStreamTaskSuccess` records `lastTaskSuccessTime`. Replay restores the
terminal offset and increments `SucceedTaskCount` in `replayOnCommitted`, but
leaves the time blank or stale; this new branch then journals FINISHED, so no
later task can repair `jobs()` or
`streaming_job_per_job_last_task_success_time_seconds`. This is distinct from
the existing cloud-aggregate thread: local replay already has the individual
`TransactionState` and its persisted `commitTime`. Please advance the timestamp
idempotently during local commit replay (or persist an equivalent value) and
cover COMMITTED replay -> recovery finish -> image round trip.
--
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]
