github-actions[bot] commented on code in PR #65373:
URL: https://github.com/apache/doris/pull/65373#discussion_r3557008297
##########
regression-test/plugins/plugin_compaction.groovy:
##########
@@ -177,21 +202,94 @@ Suite.metaClass.trigger_and_wait_compaction = { String
table_name, String compac
}
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"])
- def currentCompactionTimestampChanged =
!success_time_unchanged || !failure_time_unchanged
+ def compactionFailureNonFatal = !failure_time_unchanged &&
+ (isNoopCompactionStatus(compaction_type,
tabletStatus["last ${compaction_type} status"]) ||
+ isIgnoredCompactionStatus(tabletStatus["last
${compaction_type} status"]))
+ def baseFailureTimeChanged =
handedOffToBaseCompactionAfterDeleteVersion &&
+ oldStatus["last base failure time"] !=
tabletStatus["last base failure time"]
+ def baseFailureIgnored = baseFailureTimeChanged &&
isIgnoredCompactionStatus(tabletStatus["last base status"])
+ if (!running && !handedOffToBaseCompactionAfterDeleteVersion &&
+ !completedByBaseCompactionAfterDeleteVersion &&
+ success_time_unchanged && !failure_time_unchanged &&
!compactionFailureNonFatal) {
+ throw new Exception("compaction failed, be host:
${be_host}, tablet id: ${tablet.TabletId}, " +
+ "run status: ${compactionStatus.run_status}, old
status: ${oldStatus}, new status: ${tabletStatus}")
+ }
+ if (!running && handedOffToBaseCompactionAfterDeleteVersion &&
+ !completedByBaseCompactionAfterDeleteVersion &&
+ baseFailureTimeChanged && !baseFailureIgnored) {
+ throw new Exception("base compaction failed after
cumulative E-2010 handoff, be host: ${be_host}, " +
+ "tablet id: ${tablet.TabletId}, run status:
${compactionStatus.run_status}, " +
+ "old status: ${oldStatus}, new status:
${tabletStatus}")
+ }
def compactionFinished =
completedByBaseCompactionAfterDeleteVersion ||
- (!handedOffToBaseCompactionAfterDeleteVersion &&
currentCompactionTimestampChanged)
+ compactionFailureNonFatal || baseFailureIgnored ||
+ (!handedOffToBaseCompactionAfterDeleteVersion &&
!success_time_unchanged)
running = running || !compactionFinished
if (running) {
logger.info("compaction is still running, be host:
${be_host}, tablet id: ${tablet.TabletId}, run status:
${compactionStatus.run_status}, old status: ${oldStatus}, new status:
${tabletStatus}")
return false
}
} else {
// time series compaction sometimes doesn't update compaction
success time
- // so we solely check run_status for it
+ // so we use run_status as the success signal, but still
surface recorded failures.
if (running) {
logger.info("compaction is still running, be host:
${be_host}, tablet id: ${tablet.TabletId}")
return false
}
+ (exit_code, stdout, stderr) = be_show_tablet_status(be_host,
be_port, tablet.TabletId)
+ assert exit_code == 0: "get tablet status failed, exit code:
${exit_code}, stdout: ${stdout}, stderr: ${stderr}"
+ def tabletStatus = parseJson(stdout.trim())
+ def oldStatus =
be_tablet_compaction_status.get("${be_host}-${tablet.TabletId}")
+ def handedOffToBaseCompactionAfterDeleteVersion = false
+ def completedByBaseCompactionAfterDeleteVersion = false
+ def cumulativePointChanged = 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"]
+ cumulativePointChanged = oldCumulativePoint != null &&
newCumulativePoint != null &&
+ newCumulativePoint > oldCumulativePoint
+ handedOffToBaseCompactionAfterDeleteVersion =
+ lastCumulativeStatus.contains("e-2010") &&
cumulativePointChanged
+ 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"])
+ def status_unchanged = (oldStatus["last ${compaction_type}
status"] == tabletStatus["last ${compaction_type} status"])
+ def compactionFailureNonFatal = !failure_time_unchanged &&
+ (isNoopCompactionStatus(compaction_type,
tabletStatus["last ${compaction_type} status"]) ||
+ isIgnoredCompactionStatus(tabletStatus["last
${compaction_type} status"]))
+ def baseFailureTimeChanged =
handedOffToBaseCompactionAfterDeleteVersion &&
+ oldStatus["last base failure time"] !=
tabletStatus["last base failure time"]
+ def baseFailureIgnored = baseFailureTimeChanged &&
isIgnoredCompactionStatus(tabletStatus["last base status"])
+ if (!handedOffToBaseCompactionAfterDeleteVersion &&
+ !failure_time_unchanged && !compactionFailureNonFatal)
{
+ throw new Exception("compaction failed, be host:
${be_host}, tablet id: ${tablet.TabletId}, " +
+ "run status: ${compactionStatus.run_status}, old
status: ${oldStatus}, new status: ${tabletStatus}")
+ }
+ if (handedOffToBaseCompactionAfterDeleteVersion &&
+ !completedByBaseCompactionAfterDeleteVersion &&
+ baseFailureTimeChanged && !baseFailureIgnored) {
+ throw new Exception("base compaction failed after
cumulative E-2010 handoff, be host: ${be_host}, " +
+ "tablet id: ${tablet.TabletId}, run status:
${compactionStatus.run_status}, " +
+ "old status: ${oldStatus}, new status:
${tabletStatus}")
+ }
+ def statusChangedToOk = !status_unchanged &&
+ "${tabletStatus["last ${compaction_type}
status"]}".toLowerCase().contains("[ok]")
+ def compactionFinished =
completedByBaseCompactionAfterDeleteVersion ||
Review Comment:
This still has a false-timeout case for successful time-series compaction.
For a cumulative time-series compaction that only merges empty rowsets,
`CumulativeCompaction::execute_compact()` records `[OK]` but intentionally
skips `last cumulative success time`, and
`TimeSeriesCumulativeCompactionPolicy::update_cumulative_point()` also returns
without advancing the cumulative point when the output rowset has zero
segments. If the previous tablet status was already `[OK]`, this branch sees
`/run_status` become false but `success_time_unchanged`, `status_unchanged`,
and `cumulativePointChanged == false`, so `compactionFinished` stays false and
the helper polls until timeout even though the requested compaction succeeded.
Please add a terminal signal for this zero-segment success case, or otherwise
accept the explicit not-running `[OK]` result without requiring the status
string/cumulative point to change.
--
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]
