fix: preserve vector DDL errors after embedding retries - #1353
Conversation
0ebc604 to
65f0dd3
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Task matching remains overly broad, a completion race can misclassify valid schema, and cleanup coverage is nondeterministic.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Corrects vector refresh/rebuild error classification while HNSW auxiliary tables are being created.
Changes:
- Checks active vector DDL tasks when the hidden index-id table is absent.
- Returns transient or schema errors based on task state.
- Adds race and cancellation mysqltests.
File summaries
| File | Description |
|---|---|
ob_vector_refresh_index_executor.cpp |
Adds DDL-task-based error classification. |
ob_vector_refresh_index_executor.h |
Declares the task-check helper. |
vector_index_refresh_while_creating.test |
Tests refresh/rebuild during creation. |
vector_index_refresh_while_creating.result |
Adds expected race-test output. |
vector_index_create_failure_cleanup.test |
Tests cancellation cleanup and recreation. |
vector_index_create_failure_cleanup.result |
Adds expected cleanup-test output. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| "SELECT COUNT(*) AS CNT FROM oceanbase.%s WHERE object_id = %lu " | ||
| "AND ddl_type IN (%d, %d)", | ||
| OB_ALL_DDL_TASK_STATUS_TNAME, data_table_id, | ||
| share::ObDDLType::DDL_CREATE_VEC_INDEX, | ||
| share::ObDDLType::DDL_REBUILD_INDEX))) { |
| false, /*with_hidden_flag*/ | ||
| true /*is_built_in_index*/))) { | ||
| if (OB_TABLE_NOT_EXIST == ret) { | ||
| ret = check_vector_index_ddl_task_exist(base_table_id, has_ddl_task); |
|
|
||
| connection conn_create; | ||
| send create vector index idx_create_fail on t_vec_create_fail(c2) with (distance=l2, type=hnsw); | ||
| sleep 5; |
ab604ee to
fd9efa3
Compare
fd9efa3 to
26b5c9d
Compare
There was a problem hiding this comment.
Reviewed commit 26b5c9d.
I reviewed the force-pushed fixes for the existing Copilot threads. This review only reports an additional issue that is not covered there.
[Major] Register the new regressions in mysqltest CI
Location: tools/deploy/mysqltest_config.yaml:258-279
Neither vector_index.vector_index_create_failure_cleanup nor vector_index.vector_index_refresh_while_creating is listed in runtime_configs.psmall.test-set. discover_cases() executes only configured names and silently ignores unconfigured suite cases. The current successful run 33607668840 ran 68+68+68+67 = 271 configured cases, and neither new name appears in its logs. Add both cases to the vector-index list and rerun the slices so this fix is enforced by PR and future CI.
Summary
CURLE_RECV_ERRORfailures in embedding tasks with the existing bounded exponential backoffOB_TABLE_NOT_EXISTVerification
make -j80model_max_retries=2Table doesn't exist)Issue: SEEK-511