github-actions[bot] commented on code in PR #65233:
URL: https://github.com/apache/doris/pull/65233#discussion_r3533135147
##########
regression-test/plugins/plugin_compaction.groovy:
##########
@@ -146,8 +156,32 @@ 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"])
Review Comment:
This now treats a changed failure timestamp as a successful completion
signal for every non-E-2010 case. Once `/api/compaction/run_status` is false,
`currentCompactionTimestampChanged` becomes true even when only `last
${compaction_type} failure time` advanced, so `compactionFinished` becomes true
and the helper returns without surfacing the failed compaction. The BE/cloud
status APIs keep success and failure timestamps/statuses separate, and normal
base/cumulative/full failure paths update the failure timestamp without
updating the success timestamp. That means shared callers of
`trigger_and_wait_compaction` can continue after a failed manual compaction
instead of timing out or failing. The E-2010 handoff special case makes sense,
but the generic path should still require the relevant success timestamp, or
explicitly fail when the failure status/timestamp advances without an accepted
handoff.
--
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]
