github-actions[bot] commented on code in PR #65373:
URL: https://github.com/apache/doris/pull/65373#discussion_r3568854668
##########
regression-test/plugins/plugin_compaction.groovy:
##########
@@ -87,6 +87,28 @@ Suite.metaClass.trigger_and_wait_compaction = { String
table_name, String compac
def tabletStatus = parseJson(stdout.trim())
be_tablet_compaction_status.put("${be_host}-${tablet.TabletId}",
tabletStatus)
}
+ def isIgnoredCompactionStatus = { status ->
+ def status_lower = "${status}".toLowerCase()
+ return ignored_errors.any { error ->
status_lower.contains(error.toLowerCase()) }
+ }
+ def isNoopCompactionStatus = { type, status ->
+ def status_lower = "${status}".toLowerCase()
+ switch (type) {
+ case "full":
+ return status_lower.contains("no suitable version") ||
+ status_lower.contains("e-808") ||
+ status_lower.contains("e-2008")
+ case "cumulative":
+ return status_lower.contains("e-2000") &&
+ !status_lower.contains("job tablet busy")
+ case "binlog":
+ return status_lower.contains("e-2012")
Review Comment:
Adding `E-2012` here makes trigger-time binlog no-input results look like
tolerated no-ops, but the async manual path still has timeout cases. If
`/api/compaction/run` returns `Success` after its two-second wait, the detached
callback can later return `BINLOG_COMPACTION_NO_SUITABLE_VERSION` either before
constructing `BinlogCompaction` when no level is available, or from
`BinlogCompaction::prepare_compact()`. The first path updates neither `last
binlog status` nor `last binlog failure time`; the second updates only `last
binlog status`, and there is no binlog completion counter. The wait loop
therefore keeps `terminalStatusChanged == false`, never applies this no-op
classification, and can poll until timeout. Please publish a per-attempt
terminal signal for every async binlog no-suitable exit, or only classify
`E-2012` as benign when the helper can prove it belongs to the current attempt.
##########
regression-test/plugins/plugin_compaction.groovy:
##########
@@ -177,21 +202,107 @@ 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 compactionFinished =
completedByBaseCompactionAfterDeleteVersion ||
- (!handedOffToBaseCompactionAfterDeleteVersion &&
currentCompactionTimestampChanged)
+ def cumulative_completion_count_unchanged = compaction_type !=
"cumulative" ||
+ oldStatus["cumulative compaction completed count"] ==
+ tabletStatus["cumulative compaction completed
count"]
+ def currentCompactionStatus = tabletStatus["last
${compaction_type} status"]
+ def statusOk =
"${currentCompactionStatus}".toLowerCase().contains("[ok]")
+ def terminalStatusChanged = !failure_time_unchanged ||
+ !cumulative_completion_count_unchanged
+ def compactionFailureNonFatal = terminalStatusChanged &&
+ (isNoopCompactionStatus(compaction_type,
currentCompactionStatus) ||
+
isIgnoredCompactionStatus(currentCompactionStatus))
+ def baseFailureTimeChanged =
handedOffToBaseCompactionAfterDeleteVersion &&
+ oldStatus["last base failure time"] !=
tabletStatus["last base failure time"]
+ def baseFailureIgnored = baseFailureTimeChanged &&
isIgnoredCompactionStatus(tabletStatus["last base status"])
+ if (!running && !handedOffToBaseCompactionAfterDeleteVersion &&
+ terminalStatusChanged && !statusOk &&
!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 &&
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 =
handedOffToBaseCompactionAfterDeleteVersion ||
+ completedByBaseCompactionAfterDeleteVersion ||
+ compactionFailureNonFatal || baseFailureIgnored ||
+ !success_time_unchanged ||
+ (!cumulative_completion_count_unchanged && statusOk)
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 use the completed count as the terminal signal after
run_status becomes false.
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 cumulative_completion_count_unchanged = compaction_type !=
"cumulative" ||
+ oldStatus["cumulative compaction completed count"] ==
+ tabletStatus["cumulative compaction completed
count"]
+ def currentCompactionStatus = tabletStatus["last
${compaction_type} status"]
+ def statusOk =
"${currentCompactionStatus}".toLowerCase().contains("[ok]")
+ def terminalStatusChanged = !failure_time_unchanged ||
+ !cumulative_completion_count_unchanged
+ def compactionFailureNonFatal = terminalStatusChanged &&
+ (isNoopCompactionStatus(compaction_type,
currentCompactionStatus) ||
+
isIgnoredCompactionStatus(currentCompactionStatus))
+ def baseFailureTimeChanged =
handedOffToBaseCompactionAfterDeleteVersion &&
+ oldStatus["last base failure time"] !=
tabletStatus["last base failure time"]
+ def baseFailureIgnored = baseFailureTimeChanged &&
isIgnoredCompactionStatus(tabletStatus["last base status"])
+ if (!handedOffToBaseCompactionAfterDeleteVersion &&
+ terminalStatusChanged && !statusOk &&
!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 &&
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 =
handedOffToBaseCompactionAfterDeleteVersion ||
+ completedByBaseCompactionAfterDeleteVersion ||
+ compactionFailureNonFatal || baseFailureIgnored ||
+ (!handedOffToBaseCompactionAfterDeleteVersion &&
+ (!success_time_unchanged ||
+
(!cumulative_completion_count_unchanged && statusOk) ||
+ cumulativePointChanged))
Review Comment:
The time-series branch can still finish before the requested cumulative
compaction reaches `execute_compact()`. On the shared-nothing manual path
`_execute_compaction_callback()` runs `prepare_compact()` first, and
`prepare_compact()` calls `calculate_cumulative_point()`, so a poll in the gap
before `execute_compact()` reacquires the cumulative lock can see `/run_status
== false` plus a higher cumulative point while `last cumulative success/failure
time` and `cumulative compaction completed count` are still unchanged. This
line then treats `cumulativePointChanged` as completion even though no rowsets
have been modified yet. Please require the new completion counter with `[OK]`,
or another signal published after `execute_compact()`, instead of accepting
point movement alone.
--
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]
