github-actions[bot] commented on code in PR #65453:
URL: https://github.com/apache/doris/pull/65453#discussion_r3557185286
##########
regression-test/suites/inverted_index_p0/index_change/test_index_change_4.groovy:
##########
@@ -18,7 +18,7 @@
import org.codehaus.groovy.runtime.IOGroovyMethods
suite("test_index_change_4") {
- def timeout = 60000
Review Comment:
This gives the suite more time, but the non-cloud V1 path still uses a local
wait helper that can pass after timing out. Below,
`wait_for_build_index_on_partition_finish(tableName, timeout)` loops while `t
<= OpTimeout`, assigns `useTime = t` for unfinished rows, and finally asserts
`useTime <= OpTimeout`. On the last poll `useTime` is exactly `OpTimeout`, so
the assertion succeeds even if one or more build-index jobs never reached
`FINISHED`. Please apply the same explicit `finished` flag and timeout failure
message to this local helper, or reuse an equivalent fixed helper, otherwise
this suite can still mask the timeout condition this PR is trying to catch.
##########
regression-test/plugins/plugin_index_change.groovy:
##########
@@ -24,21 +24,25 @@ import java.util.regex.Pattern;
def delta_time = 1000
Suite.metaClass.wait_for_last_build_index_finish = {table_name, OpTimeout ->
- def useTime = 0
- for(int t = delta_time; t <= OpTimeout; t += delta_time){
- def alter_res = sql """SHOW BUILD INDEX WHERE TableName =
"${table_name}" ORDER BY CreateTime DESC LIMIT 1;"""
+ def finished = false
+ def alter_res = ""
+ for(int t = 0; t <= OpTimeout; t += delta_time){
+ alter_res = sql """SHOW BUILD INDEX WHERE TableName = "${table_name}"
ORDER BY CreateTime DESC LIMIT 1;"""
alter_res = alter_res.toString()
if(alter_res.contains("FINISHED")) {
logger.info(table_name + " latest alter job finished, detail: " +
alter_res)
+ finished = true
break
} else if (alter_res.contains("CANCELLED")) {
logger.info(table_name + " latest alter job failed, detail: " +
alter_res)
assertTrue(false)
}
- useTime = t
+ if (t >= OpTimeout) {
+ break
+ }
sleep(delta_time)
}
- assertTrue(useTime <= OpTimeout, "wait for last build index finish
timeout")
Review Comment:
This fixes the build-index wait, but the same shared plugin still has the
old timeout-success pattern in `wait_for_last_col_change_finish`. That helper
also polls through `t <= OpTimeout`, records `useTime = t` while `FINISHED` has
not been seen, and then asserts `useTime <= OpTimeout`, so it succeeds on the
final poll even when the column alter job never finished. `test_index_change_4`
calls that helper before later build-index waits, so a stalled column change
can still be hidden. Please update the column-change helper with the same
explicit `finished` flag and latest-result timeout failure.
--
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]
