github-actions[bot] commented on code in PR #65453:
URL: https://github.com/apache/doris/pull/65453#discussion_r3567754812
##########
regression-test/plugins/plugin_index_change.groovy:
##########
@@ -51,25 +60,28 @@ Suite.metaClass.build_index_on_table = {index_name,
table_name ->
}
Suite.metaClass.wait_for_last_col_change_finish = { table_name, OpTimeout ->
- def useTime = 0
+ def finished = false
+ def alter_res = ""
- 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;"""
+ 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;"""
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)
+ finished = true
+ break
+ }
+ if (t >= OpTimeout) {
break
}
- useTime = t
sleep(delta_time)
}
- assertTrue(useTime <= OpTimeout, "wait_for_last_col_change_finish timeout")
+ assertTrue(finished, "wait_for_last_col_change_finish timeout, latest
result: ${alter_res}")
}
Suite.metaClass.wait_for_last_schema_change_finish = {table_name, OpTimeout ->
wait_for_last_col_change_finish(table_name, OpTimeout)
- wait_for_last_build_index_finish(table_name, OpTimeout)
Review Comment:
Making this wrapper column-only fixes the schema-only callers, but it also
skips cloud full-table drop-index cleanup jobs. In cloud mode, dropping an
inverted index with `enable_add_index_for_new_data` goes through
`modifyTableLightSchemaChange` and then submits per-partition index-change
delete jobs via `buildOrDeleteTableInvertedIndices`; `SHOW ALTER TABLE COLUMN`
can finish before those jobs are done. Callers like
`variant_p0/with_index/load.groovy` line 52 and `test_add_drop_index.groovy`
line 147 then create/build another index on the same table, which can race the
unfinished cleanup and hit the existing
`hasIndexChangeJobOnPartition`/`hasIndexChangeJobOnTable` checks. Please split
the wrapper semantics at these drop-index call sites, or add a separate helper
so cloud drop-index waits include the relevant build-index/delete-index jobs
while schema-only create-index waits remain column-only.
##########
regression-test/plugins/plugin_index_change.groovy:
##########
@@ -51,25 +60,28 @@ Suite.metaClass.build_index_on_table = {index_name,
table_name ->
}
Suite.metaClass.wait_for_last_col_change_finish = { table_name, OpTimeout ->
- def useTime = 0
+ def finished = false
+ def alter_res = ""
- 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;"""
+ 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;"""
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)
+ finished = true
+ break
+ }
+ if (t >= OpTimeout) {
break
}
- useTime = t
sleep(delta_time)
}
- assertTrue(useTime <= OpTimeout, "wait_for_last_col_change_finish timeout")
+ assertTrue(finished, "wait_for_last_col_change_finish timeout, latest
result: ${alter_res}")
}
Suite.metaClass.wait_for_last_schema_change_finish = {table_name, OpTimeout ->
wait_for_last_col_change_finish(table_name, OpTimeout)
Review Comment:
Making this wrapper column-only fixes the schema-only callers, but it also
skips cloud full-table drop-index cleanup jobs. In cloud mode, dropping an
inverted index with `enable_add_index_for_new_data` goes through
`modifyTableLightSchemaChange` and then submits per-partition index-change
delete jobs via `buildOrDeleteTableInvertedIndices`; `SHOW ALTER TABLE COLUMN`
can finish before those jobs are done. Callers like
`variant_p0/with_index/load.groovy` line 52 and `test_add_drop_index.groovy`
line 147 then create/build another index on the same table, which can race the
unfinished cleanup and hit the existing
`hasIndexChangeJobOnPartition`/`hasIndexChangeJobOnTable` checks. Please split
the wrapper semantics at these drop-index call sites, or add a separate helper
so cloud drop-index waits include the relevant build-index/delete-index jobs
while schema-only create-index waits remain column-only.
--
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]
