github-actions[bot] commented on code in PR #70208:
URL: https://github.com/apache/doris/pull/70208#discussion_r4545977674
##########
fe/fe-core/src/main/java/org/apache/doris/master/MasterImpl.java:
##########
@@ -546,6 +548,19 @@ private void finishClearAlterTask(AgentTask task,
TFinishTaskRequest request) {
}
private void finishPublishVersion(AgentTask task, TFinishTaskRequest
request) {
+ PublishVersionTask publishVersionTask = (PublishVersionTask) task;
+ synchronized (publishVersionTask) {
+ updatePublishVersionTask(publishVersionTask, request);
+ }
+ }
+
+ private void updatePublishVersionTask(PublishVersionTask task,
TFinishTaskRequest request) {
+ // INCOMPLETE is a progress report; only final reports finish/remove
the task.
+ if (request.getTaskStatus().getStatusCode() == TStatusCode.INCOMPLETE)
{
+ // A failed final report leaves isFinished true when the same task
is resent.
+ task.setSuccTablets(request.getSuccTablets());
Review Comment:
[P2] Preserve delta rows before partial successes make a transaction visible
This branch retains only successes, while `PublishVersionDaemon` collects
delta rows only from finished tasks. With three replicas and an FE publish
timeout of 1s, let BE0 report A successful/B still retrying, BE1 report B
successful/A still retrying, and BE2 finish A+B. Both tablets now have quorum
at the timeout, but only BE2 contributes rows; `AnalysisManager` divides those
rows by the configured replica count. Visibility then removes/prunes all
publish tasks, so later final reports cannot repair the undercount. If the gaps
resolve at 2s, the base waits for those final reports and includes the newly
published A/B contributions; this change has already discarded them. This loses
additional updated-row statistics used for analyze freshness. Carry and merge
deduplicated tablet row deltas with progress, and collect them before early
visibility; add a daemon-level accounting test.
##########
fe/fe-core/src/main/java/org/apache/doris/transaction/DatabaseTransactionMgr.java:
##########
@@ -1646,11 +1646,9 @@ private void
checkReplicaContinuousVersionSucc(List<Long> subTxnIds, long alterW
boolean success = true;
for (int i = 0; i < subTxnIds.size(); i++) {
PublishVersionTask task = replicaPublishTasks.get(i);
- // Defensive null guard: AgentTaskCleanupDaemon may force-finish a
PublishVersionTask
- // without populating succTablets; MasterImpl.finishPublishVersion
may also call
- // setSuccTablets(null) on a non-OK BE response.
+ // A task can report this tablet's success while retrying version
gaps on other tablets.
Map<Long, Long> succ = (task == null) ? null :
task.getSuccTablets();
- success = (task != null && task.isFinished() && succ != null &&
succ.containsKey(tabletId)) || (
+ success = (succ != null && succ.containsKey(tabletId)) || (
Review Comment:
[P2] Let partial progress reach the daemon's completion check
Removing `isFinished()` here is insufficient on the production path:
`PublishVersionDaemon.tryFinishOneTxn()` still skips `finishTransaction()`
while any live task is unfinished, unless the FE timeout or the
majority-finished/slow-queue exception applies. For example, BE0 reports tablet
A successful while retrying B, and BE1/BE2 finish A+B. Every tablet has quorum,
but with an ordinary queue (at most 1000 tasks), BE0 keeps `shouldFinishTxn`
false. With the default BE retry timeout of 8s and FE timeout of 30s, the
partial report therefore cannot shorten that wait; the daemon waits for the
same final report as before. The new quorum tests call `finishTransaction()`
directly and miss this gate. Allow useful partial progress to trigger the
daemon's quorum check, retaining the existing version and subtransaction
checks, and cover this through the daemon entry path.
--
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]
