github-actions[bot] commented on code in PR #65373:
URL: https://github.com/apache/doris/pull/65373#discussion_r3551979305
##########
regression-test/plugins/plugin_compaction.groovy:
##########
@@ -114,9 +126,9 @@ Suite.metaClass.trigger_and_wait_compaction = { String
table_name, String compac
triggered_tablets.add(tablet) // compaction already in queue,
treat it as successfully triggered
} else if (!auto_compaction_disabled) {
// ignore the error if auto compaction enabled
- } else if (status_lower.contains("e-2000") ||
status_lower.contains("e-2010")) {
+ } else if (isNoopCompactionStatus(trigger_status.status) ||
status_lower.contains("e-2010")) {
// ignore this tablet compaction.
Review Comment:
The `E-2010` path here still skips adding the tablet to `triggered_tablets`,
so the wait-phase handoff checks added below never run when the manual
cumulative trigger returns `CUMULATIVE_MEET_DELETE_VERSION` synchronously. That
synchronous path is possible because the manual endpoint waits for the callback
result for up to two seconds, and
`CumulativeCompaction::pick_rowsets_to_compact()` returns `E-2010` after
advancing the cumulative point so base compaction can handle the delete-version
rowset. In that case this helper reports completion immediately instead of
waiting for either base success or a real base failure. Please treat
trigger-time `E-2010` as a handoff that still needs the same base verification,
rather than dropping the tablet before the wait loop.
##########
regression-test/plugins/plugin_compaction.groovy:
##########
@@ -177,9 +189,30 @@ 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(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 baseFailureNonFatal = baseFailureTimeChanged &&
+ (isNoopCompactionStatus(tabletStatus["last base
status"]) ||
+ 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 && !baseFailureNonFatal) {
+ 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 || baseFailureNonFatal ||
+ (!handedOffToBaseCompactionAfterDeleteVersion &&
!success_time_unchanged)
Review Comment:
In the `E-2010` handoff case, `baseFailureNonFatal` should not make the
helper finish. The cumulative path advanced the point because base compaction
is supposed to handle the delete-version rowsets, and the comment above this
block explicitly says to accept the handoff through a base/cumulative success
signal, not through failure time alone. A base `E-808`/no-suitable status can
mean base did not select input rowsets or did not satisfy the base policy, so
returning success here can hide exactly the missing base compaction that this
handoff check was added to wait for. Please keep no-suitable statuses non-fatal
for ordinary requested compactions, but continue waiting or fail when the
`E-2010` handoff only produces a base failure timestamp.
--
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]
