yujun777 commented on code in PR #68390:
URL: https://github.com/apache/doris/pull/68390#discussion_r4091535818
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/mtmv/MTMVTask.java:
##########
@@ -259,7 +260,25 @@ private PartitionPlanningException(String message,
Throwable cause) {
// Written by the executing (Disruptor worker) thread via the
executeCommand consumer
// callback and read by the cancel (command) thread, so it must be
volatile.
private volatile StmtExecutor executor;
- private Map<String, MTMVRefreshPartitionSnapshot> partitionSnapshots;
+ // What this task has committed, per MV partition: the snapshot each
partition's rows were read at.
+ // One accumulator for the whole task rather than one per phase, because
that is what the MV publishes
+ // at the end of it -- a phase that started from empty would publish its
own work and drop the work of
+ // the phases before it, leaving partitions a preceding rebuild replaced
looking unsynced.
+ private Map<String, MTMVRefreshPartitionSnapshot> partitionSnapshots =
Maps.newConcurrentMap();
+ // The requirement each refreshed partition was read under, captured
before the base tables were read
+ // and recorded only once that batch's data committed (see
commitCapturedEpochs). In memory only: the
+ // journal carries the resulting states, and a replay applies those
instead of recomputing anything.
+ private transient Map<String, Long> ivmCapturedEpochs = Maps.newHashMap();
Review Comment:
Not fixed here, and I would rather say so than claim a fix. The half the
finding names -- publishing an accumulator the worker can still mutate -- is
fixed in this branch: `getIvmCapturedEpochs` hands out a detached copy, and the
snapshot write-back filters to the partitions that are still clean.
What remains is the lifecycle underneath it: STOP reaches `cancel(false)`
through `AbstractJob`, so a cancelled task publishes its result and releases
its resources before the worker stops. The publish is a prefix of the committed
work -- the accumulators only grow per committed batch -- so each entry is
truthful; what it costs is a second result when the worker does finish, and a
worker that can be interrupted through resources the cancel path has already
released. Unwinding that means deciding what a cancelled refresh publishes and
whether STOP waits for quiescence, for every job type that shares the
framework, which is a change of its own and not this PR's. I have left the
thread open.
--
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]
