github-actions[bot] commented on code in PR #68007:
URL: https://github.com/apache/doris/pull/68007#discussion_r4022377256
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/insert/streaming/StreamingInsertJob.java:
##########
@@ -1052,6 +1076,9 @@ public void replayOnUpdated(StreamingInsertJob replayJob)
{
Env.getCurrentGlobalTransactionMgr().getCallbackFactory().removeCallback(getJobId());
}
}
+ if (replayJob.getOffsetProviderPersist() != null) {
Review Comment:
[P1] Do not replay the configured start offset over committed progress
A local lexical S3 job can retain `properties.offset=A` after transactions
have committed through `B`. A later PAUSED job-update record therefore carries
both the full map with `A` and authoritative `offsetProviderPersist=B`. With
this new ordering, replay stores `B` first and then `modifyPropertiesInternal`
reapplies `A`, overwriting the persisted field too; if that paused follower
checkpoints/restarts before resume, it now restores `A` and reprocesses
committed files (the previous order left the serialized field at `B`). Please
keep replay-time property refresh from treating the original start offset as an
ALTER, or make the authoritative persisted offset win after the merge, and add
a create-at-A -> commit-B -> PAUSE replay -> image round-trip test.
##########
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:
[P3] Reconstruct the displayed end offset on terminal recovery
`maxEndFile` supplies the jobs TVF `EndOffset`, but it is transient and
neither `updateOffset()` nor `restoreFromPersistInfo()` rebuilds it. After an
image restore, local commit replay, or cloud attachment replay, the persisted
`lastBatch=true` makes this method immediately report completion, so no later
metadata fetch repopulates the value; a completed job that showed its final key
before failover shows a blank `EndOffset` forever afterward. Please reconstruct
`maxEndFile` from the committed `endFile` for a terminal ONCE offset (or fall
back to it when displaying), and assert `EndOffset` across local/image/cloud
recovery.
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/insert/streaming/StreamingInsertJob.java:
##########
@@ -972,6 +995,7 @@ private void
updateCloudJobStatisticAndOffset(StreamingTaskTxnCommitAttachment a
this.jobStatistic.setFileSize(attachment.getFileBytes());
this.jobStatistic.setFilteredRows(attachment.getFilteredRows());
offsetProvider.updateOffset(offsetProvider.deserializeOffset(attachment.getOffset()));
+ this.offsetProviderPersist = offsetProvider.getPersistInfo();
Review Comment:
[P2] Restore success metadata before cloud recovery finishes the job
The final cloud commit can be durable in Meta Service while the FE crashes
before `onStreamTaskSuccess` increments `SucceedTaskCount` and
`LastTaskSuccessTime`. This replay now restores the final offset and lets the
PENDING job immediately journal FINISHED, but `StreamingTaskCommitAttachmentPB`
carries neither a cumulative task count nor a success timestamp, so the
terminal `jobs()` row remains permanently one task behind (or zero) with an
empty/stale success time. This is distinct from the existing persisted-offset
thread: that fix preserves the cursor but cannot reconstruct these fields.
Please persist an idempotent cumulative count/time (or task identity) in the
cloud aggregate and assert them in the final-commit crash/recovery test.
--
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]
