github-actions[bot] commented on code in PR #65373:
URL: https://github.com/apache/doris/pull/65373#discussion_r3584164154
##########
regression-test/suites/schema_change_p0/test_schema_change_with_empty_rowset.groovy:
##########
@@ -31,6 +31,28 @@ suite("test_schema_change_with_empty_rowset",
"p0,nonConcurrent") {
return jobStateResult[0][9]
}
+ def backendIdToHost = [:]
+ def backendIdToHttpPort = [:]
+ getBackendIpHttpPort(backendIdToHost, backendIdToHttpPort)
+
+ def triggerCumulativeAndWaitForVersionCount = { tbl, int
targetVersionCount ->
+ def tablets = sql_return_maparray """ SHOW TABLETS FROM ${tbl} """
+ for (def tablet in tablets) {
+ def host = backendIdToHost["${tablet.BackendId}"]
+ def port = backendIdToHttpPort["${tablet.BackendId}"]
+ def (code, out, err) = be_run_cumulative_compaction(host, port,
tablet.TabletId)
Review Comment:
This only checks whether the `curl` process exited successfully, not whether
the compaction API accepted the request. The BE action replies with HTTP 200
for non-OK compaction statuses by serializing `st.to_json()` into the body, and
the shared `trigger_and_wait_compaction` helper explicitly parses `stdout` and
rejects or classifies non-`Success` statuses after the same curl call. With the
new direct helper, a real BE-side submission failure can be ignored here and
turn into a 300-second `VersionCount` wait with the useful status hidden in
`stdout`. Please parse `out` and require `status == "Success"` (or explicitly
handle the intended benign statuses) before entering the wait.
##########
regression-test/suites/schema_change_p0/test_schema_change_with_empty_rowset.groovy:
##########
@@ -31,6 +31,28 @@ suite("test_schema_change_with_empty_rowset",
"p0,nonConcurrent") {
return jobStateResult[0][9]
}
+ def backendIdToHost = [:]
+ def backendIdToHttpPort = [:]
+ getBackendIpHttpPort(backendIdToHost, backendIdToHttpPort)
+
+ def triggerCumulativeAndWaitForVersionCount = { tbl, int
targetVersionCount ->
+ def tablets = sql_return_maparray """ SHOW TABLETS FROM ${tbl} """
+ for (def tablet in tablets) {
+ def host = backendIdToHost["${tablet.BackendId}"]
+ def port = backendIdToHttpPort["${tablet.BackendId}"]
+ def (code, out, err) = be_run_cumulative_compaction(host, port,
tablet.TabletId)
+ assert code == 0: "trigger cumulative compaction failed,
tablet=${tablet.TabletId}, " +
+ "code=${code}, stdout=${out}, stderr=${err}"
+ }
+
+ Awaitility.await().atMost(300, TimeUnit.SECONDS).pollInterval(1,
TimeUnit.SECONDS).until {
Review Comment:
This can still start the schema change before the BE actually has the rowset
headroom this test is trying to create. The value polled here comes from FE
`SHOW TABLETS`, which displays `Replica.getTotalVersionCount()` and is
refreshed from periodic tablet stats/reports, while the direct
`/api/compaction/run` path does not synchronously update that FE field. The
manual cumulative action can also return `Success` after the detached task
merely outlives its two-second wait, before `modify_rowsets()` has reduced the
BE-local rowset count. The following inserts are admitted by the live BE
`tablet()->version_count()` check, so a stale FE count can let this wait pass
and still hit `TOO_MANY_VERSION` during the schema-change workload. Please wait
on a BE-local completion/count signal for each triggered tablet, for example
`/api/compaction/show` rowsets or another terminal signal published after the
compaction modifies rowsets, instead of using FE `VersionCount` 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]
