github-actions[bot] commented on code in PR #65453:
URL: https://github.com/apache/doris/pull/65453#discussion_r3587600097
##########
regression-test/suites/inverted_index_p0/index_change/test_index_change_4.groovy:
##########
@@ -36,19 +36,33 @@ suite("test_index_change_4") {
++finished_num;
}
}
- if (finished_num == expected_finished_num) {
Review Comment:
The V1 branch still does not snapshot before its explicit `BUILD INDEX`.
Earlier on the same `test_index_change_4_v1` table, the test drops two inverted
indexes and waits for those delete jobs to finish. This helper then succeeds
whenever `SHOW BUILD INDEX WHERE TableName = ...` is nonempty and every
returned row is `FINISHED`, so if the later `build_index_on_table("idx_note",
tableName)` path ever fails to create a new job row, the already-finished drop
rows are enough for the wait to pass. Please mirror the V2 branch by capturing
`get_build_index_job_ids(tableName)` immediately before the build and using the
snapshot-filtered shared wait.
##########
regression-test/suites/inverted_index_p0/index_change/test_index_change_on_renamed_column.groovy:
##########
@@ -101,7 +78,7 @@ suite("test_index_change_on_renamed_column") {
// drop inverted index on renamed column
sql """ alter table ${tableName} drop index idx_s; """
Review Comment:
Keeping only the column wait is right for the cloud false-mode case from the
earlier thread, but this suite also runs the drop in non-cloud mode after
building `idx_s`. In FE, full-table `INVERTED` drops in non-cloud still take
the light-index path: `SchemaChangeHandler` sets `supportLightIndexChange` for
`INVERTED`/`ANN` when `Config.isNotCloudMode()`, and
`modifyTableLightSchemaChange` calls `buildOrDeleteTableInvertedIndices` for
drops. That enqueues delete `IndexChangeJob`s after the metadata/column job, so
the non-cloud run can finish this suite without waiting for the physical delete
job it just submitted. Please keep this column-only for cloud false-mode, but
also snapshot and wait for the new build-index/delete rows when the full-table
drop is a non-cloud `INVERTED` drop.
##########
regression-test/suites/inverted_index_p0/index_change/test_drop_index_on_partition.groovy:
##########
@@ -26,29 +26,24 @@ suite("test_drop_index_on_partition", "inverted_index") {
}
def wait_for_build_index_on_partition_finish = { table_name,
expected_job_count, 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}";"""
- if (alter_res.size() < expected_job_count) {
- useTime = t
- sleep(delta_time)
- continue
+ 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}";"""
Review Comment:
This still scopes the wait to every retained `SHOW BUILD INDEX` row with the
same table name. `SHOW BUILD INDEX` exposes `JobId`, `TableName`, partition and
state, but not the table id, and finished/cancelled `IndexChangeJob`s are kept
for `history_job_keep_max_second`, 7 days by default. Since this suite reuses
fixed names after `DROP TABLE IF EXISTS`, a stale `CANCELLED` or still-running
row from a previous run can make this helper fail or time out even after the
current `DROP INDEX ... PARTITION` jobs finish. Please snapshot the job ids
before each partition drop and wait only for rows not in that snapshot.
##########
regression-test/suites/inverted_index_p0/test_create_index_2.groovy:
##########
@@ -107,10 +107,18 @@ suite("test_create_index_2", "inverted_index"){
assertEquals(show_result[1][2], "name_idx_2")
// drop index
+ def previous_job_ids = isCloudMode() ?
get_build_index_job_ids(indexTbName1) : null
sql "drop index name_idx_1 on ${indexTbName1}"
- wait_for_last_build_index_finish(indexTbName1, timeout)
+ wait_for_last_col_change_finish(indexTbName1, timeout)
+ if (isCloudMode()) {
+ wait_for_last_build_index_finish(indexTbName1, timeout,
previous_job_ids)
+ previous_job_ids = get_build_index_job_ids(indexTbName1)
+ }
sql "drop index name_idx_2 on ${indexTbName1}"
- wait_for_last_build_index_finish(indexTbName1, timeout)
+ wait_for_last_col_change_finish(indexTbName1, timeout)
+ if (isCloudMode()) {
Review Comment:
This guard also needs to cover the non-cloud `INVERTED` drop. `name_idx_1`
is `NGRAM_BF`, so the non-cloud first drop is schema-change only, but
`name_idx_2` is `INVERTED`; in non-cloud FE sets `supportLightIndexChange` for
full-table `INVERTED` drops and `modifyTableLightSchemaChange` enqueues delete
`IndexChangeJob`s. With the wait inside `if (isCloudMode())`, the non-cloud run
can continue to the later same-table `CREATE INDEX` cases while the
`name_idx_2` delete job is still running. Please snapshot and wait for the new
build-index/delete rows for this second drop in non-cloud mode too.
--
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]
