github-actions[bot] commented on code in PR #65233:
URL: https://github.com/apache/doris/pull/65233#discussion_r3551487509
##########
regression-test/plugins/plugin_compaction.groovy:
##########
@@ -146,8 +156,37 @@ Suite.metaClass.trigger_and_wait_compaction = { String
table_name, String compac
}
def tabletStatus = parseJson(stdout.trim())
def oldStatus =
be_tablet_compaction_status.get("${be_host}-${tablet.TabletId}")
- // last compaction success time isn't updated, indicates
compaction is not started(so we treat it as running and wait)
- running = running || (oldStatus["last ${compaction_type}
success time"] == tabletStatus["last ${compaction_type} success time"])
+ // last compaction success/failure time isn't updated,
indicates compaction is not started(so we treat it as running and wait)
+ def handedOffToBaseCompactionAfterDeleteVersion = false
+ def completedByBaseCompactionAfterDeleteVersion = false
+ if (compaction_type == "cumulative") {
+ def oldCumulativePoint =
toLongOrNull(oldStatus["cumulative point"])
+ def newCumulativePoint =
toLongOrNull(tabletStatus["cumulative point"])
+ def lastCumulativeStatus = "${tabletStatus["last
cumulative status"]}".toLowerCase()
+ def baseSuccessTimeChanged = oldStatus["last base success
time"] != tabletStatus["last base success time"]
+ def cumulativeSuccessTimeChanged =
+ oldStatus["last cumulative success time"] !=
tabletStatus["last cumulative success time"]
+ // E-2010 advances the cumulative point and lets base
compaction handle delete-version rowsets.
+ // In some timing windows, base success is already visible
in the cached old status while
+ // cumulative success advances later, so accept either
success signal but not failure time alone.
+ handedOffToBaseCompactionAfterDeleteVersion =
lastCumulativeStatus.contains("e-2010") &&
+ oldCumulativePoint != null && newCumulativePoint
!= null &&
+ newCumulativePoint > oldCumulativePoint
+ completedByBaseCompactionAfterDeleteVersion =
+ handedOffToBaseCompactionAfterDeleteVersion &&
+ (baseSuccessTimeChanged ||
cumulativeSuccessTimeChanged)
+ }
+ def success_time_unchanged = (oldStatus["last
${compaction_type} success time"] == tabletStatus["last ${compaction_type}
success time"])
+ def failure_time_unchanged = (oldStatus["last
${compaction_type} failure time"] == tabletStatus["last ${compaction_type}
failure time"])
+ if (!running && !handedOffToBaseCompactionAfterDeleteVersion &&
+ !completedByBaseCompactionAfterDeleteVersion &&
+ success_time_unchanged && !failure_time_unchanged) {
+ throw new Exception("compaction failed, be host:
${be_host}, tablet id: ${tablet.TabletId}, " +
+ "run status: ${compactionStatus.run_status}, old
status: ${oldStatus}, new status: ${tabletStatus}")
+ }
+ def compactionFinished =
completedByBaseCompactionAfterDeleteVersion ||
Review Comment:
This E-2010 handoff branch still treats delegated base failures as keep
waiting. Once `handedOffToBaseCompactionAfterDeleteVersion` is true, the
failure check above is skipped, and `compactionFinished` only accepts
base/cumulative success timestamp changes. If the base compaction that is
supposed to handle the delete-version rowsets fails, BE/cloud record that in
`last base failure time` / `last base status` without advancing either accepted
success timestamp, so after `/api/compaction/run_status` goes false this loop
just returns `false` until the Awaitility timeout. The handoff path should also
compare the base failure status/time against the cached status and throw when
it advances before an accepted success signal.
##########
regression-test/plugins/plugin_compaction.groovy:
##########
@@ -146,8 +156,37 @@ Suite.metaClass.trigger_and_wait_compaction = { String
table_name, String compac
}
def tabletStatus = parseJson(stdout.trim())
def oldStatus =
be_tablet_compaction_status.get("${be_host}-${tablet.TabletId}")
- // last compaction success time isn't updated, indicates
compaction is not started(so we treat it as running and wait)
- running = running || (oldStatus["last ${compaction_type}
success time"] == tabletStatus["last ${compaction_type} success time"])
+ // last compaction success/failure time isn't updated,
indicates compaction is not started(so we treat it as running and wait)
+ def handedOffToBaseCompactionAfterDeleteVersion = false
+ def completedByBaseCompactionAfterDeleteVersion = false
+ if (compaction_type == "cumulative") {
+ def oldCumulativePoint =
toLongOrNull(oldStatus["cumulative point"])
+ def newCumulativePoint =
toLongOrNull(tabletStatus["cumulative point"])
+ def lastCumulativeStatus = "${tabletStatus["last
cumulative status"]}".toLowerCase()
+ def baseSuccessTimeChanged = oldStatus["last base success
time"] != tabletStatus["last base success time"]
+ def cumulativeSuccessTimeChanged =
+ oldStatus["last cumulative success time"] !=
tabletStatus["last cumulative success time"]
+ // E-2010 advances the cumulative point and lets base
compaction handle delete-version rowsets.
+ // In some timing windows, base success is already visible
in the cached old status while
+ // cumulative success advances later, so accept either
success signal but not failure time alone.
+ handedOffToBaseCompactionAfterDeleteVersion =
lastCumulativeStatus.contains("e-2010") &&
+ oldCumulativePoint != null && newCumulativePoint
!= null &&
+ newCumulativePoint > oldCumulativePoint
+ completedByBaseCompactionAfterDeleteVersion =
+ handedOffToBaseCompactionAfterDeleteVersion &&
+ (baseSuccessTimeChanged ||
cumulativeSuccessTimeChanged)
+ }
+ def success_time_unchanged = (oldStatus["last
${compaction_type} success time"] == tabletStatus["last ${compaction_type}
success time"])
+ def failure_time_unchanged = (oldStatus["last
${compaction_type} failure time"] == tabletStatus["last ${compaction_type}
failure time"])
+ if (!running && !handedOffToBaseCompactionAfterDeleteVersion &&
Review Comment:
This wait-phase failure check bypasses `ignored_errors`. The helper only
applies that list to the immediate trigger response, but existing
fault-injection callers pass `new String[]{"e-6010"}` for full compaction, and
in cloud mode `/api/compaction/run` can return success after submitting the
task while the async compaction later records `last full failure time` / `last
full status`. When that ignored E-6010 failure reaches this branch, the helper
throws before checking the status against `ignored_errors`, so the suite fails
even though the caller explicitly asked to ignore that compaction error. Please
honor `ignored_errors` for the wait-phase status before throwing, or make the
contract trigger-only and update the callers.
--
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]
