github-actions[bot] commented on code in PR #65453:
URL: https://github.com/apache/doris/pull/65453#discussion_r3595825371
##########
regression-test/plugins/plugin_index_change.groovy:
##########
@@ -51,25 +73,34 @@ Suite.metaClass.build_index_on_table = {index_name,
table_name ->
}
Suite.metaClass.wait_for_last_col_change_finish = { table_name, OpTimeout ->
- def useTime = 0
-
- for (int t = delta_time; t <= OpTimeout; t += delta_time) {
- def alter_res = sql """SHOW ALTER TABLE COLUMN WHERE TableName =
"${table_name}" ORDER BY CreateTime DESC LIMIT 1;"""
- alter_res = alter_res.toString()
- if (alter_res.contains("FINISHED")) {
- sleep(3000) // wait change table state to normal
- logger.info(table_name + " latest alter job finished, detail: " +
alter_res)
+ def finished = false
+ def alter_res = []
+
+ for (int t = 0; t <= OpTimeout; t += delta_time) {
+ alter_res = sql """SHOW ALTER TABLE COLUMN WHERE TableName =
"${table_name}" ORDER BY CreateTime DESC LIMIT 1;"""
Review Comment:
This latest-row lookup is still ambiguous when consecutive same-table jobs
get the same millisecond `CreateTime`. FE supplies these rows in ascending
JobId order, and `SchemaChangeProcDir` stably sorts only by `CreateTime`, so a
tie leaves the older/lower-JobId `FINISHED` row first and `LIMIT 1` can return
it while the new job is still running. That is reachable in the Cloud
`test_index_change_1` path, which submits a light finished add immediately
before a parser-bearing schema-change add. Please add `JobId DESC` as a
tie-breaker (or snapshot column-job IDs) so this waiter identifies the
submitted operation.
##########
regression-test/suites/inverted_index_p0/index_change/test_add_drop_index_with_data.groovy:
##########
@@ -261,8 +242,9 @@ suite("test_add_drop_index_with_data", "inverted_index"){
logger.info("show index from " + indexTbName1 + " result: " + show_result)
assertEquals(show_result.size(), 3)
- build_index_on_table("idx_name", indexTbName1)
- wait_for_build_index_on_partition_finish(indexTbName1, timeout)
+ run_index_change_job_and_wait(indexTbName1, timeout) {
Review Comment:
The strict migration stops before the final multi-index drop below (current
lines 265-270). With `enable_add_index_for_new_data=true`, dropping these
INVERTED indexes enqueues asynchronous delete `IndexChangeJob`s in both local
and Cloud mode, but the following `SHOW INDEX` and MATCH checks only observe
metadata/fallback and can still pass if cleanup is running or later
`CANCELLED`. Please wrap that final `ALTER TABLE ... DROP INDEX` in
`run_index_change_job_and_wait` as well so this suite verifies the physical
delete jobs finish.
--
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]
