This is an automated email from the ASF dual-hosted git repository.
jianliangqi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 9c6734e68e [bugfix](index) Fix build index limitations (#24358)
9c6734e68e is described below
commit 9c6734e68e2f79a26fa1cf55e5f2fbc1ead98e5b
Author: Kang <[email protected]>
AuthorDate: Thu Sep 14 17:53:22 2023 +0800
[bugfix](index) Fix build index limitations (#24358)
1. skip existed index on column with different id on build index
2. allow build index for CANCELED or FINISHED state
---
be/src/olap/task/index_builder.cpp | 25 +++++++++++-----
.../apache/doris/alter/SchemaChangeHandler.java | 4 +--
.../inverted_index_p0/test_build_index.groovy | 33 ++++++++++++++++++----
3 files changed, 48 insertions(+), 14 deletions(-)
diff --git a/be/src/olap/task/index_builder.cpp
b/be/src/olap/task/index_builder.cpp
index 4593aecc82..9c33ceafb2 100644
--- a/be/src/olap/task/index_builder.cpp
+++ b/be/src/olap/task/index_builder.cpp
@@ -55,6 +55,8 @@ Status IndexBuilder::update_inverted_index_info() {
// just do link files
LOG(INFO) << "begin to update_inverted_index_info, tablet=" <<
_tablet->tablet_id()
<< ", is_drop_op=" << _is_drop_op;
+ // index ids that will not be linked
+ std::set<int32_t> without_index_uids;
for (auto i = 0; i < _input_rowsets.size(); ++i) {
auto input_rowset = _input_rowsets[i];
TabletSchemaSPtr output_rs_tablet_schema =
std::make_shared<TabletSchema>();
@@ -78,12 +80,13 @@ Status IndexBuilder::update_inverted_index_info() {
const TabletIndex* exist_index =
output_rs_tablet_schema->get_inverted_index(column_uid);
if (exist_index && exist_index->index_id() !=
index.index_id()) {
- // maybe there are concurrent drop index request did not
obtain the lock,
- // so return error, to wait the drop index request
finished.
- return
Status::Error<ErrorCode::INVERTED_INDEX_BUILD_WAITTING>(
+ LOG(WARNING) << fmt::format(
"column: {} has a exist inverted index, but the
index id not equal "
- "request's index id, , exist index id: {},
request's index id: {}",
+ "request's index id, , exist index id: {},
request's index id: {}, "
+ "remove exist index in new
output_rs_tablet_schema",
column_uid, exist_index->index_id(),
index.index_id());
+ without_index_uids.insert(exist_index->index_id());
+
output_rs_tablet_schema->remove_index(exist_index->index_id());
}
output_rs_tablet_schema->append_index(index);
}
@@ -105,9 +108,17 @@ Status IndexBuilder::update_inverted_index_info() {
if (!status.ok()) {
return
Status::Error<ErrorCode::ROWSET_BUILDER_INIT>(status.to_string());
}
- RETURN_IF_ERROR(input_rowset->link_files_to(_tablet->tablet_path(),
-
output_rs_writer->rowset_id(), 0,
- &_alter_index_ids)); //
build output rowset
+
+ // if without_index_uids is not empty, copy _alter_index_ids to it
+ // else just use _alter_index_ids to avoid copy
+ if (!without_index_uids.empty()) {
+ without_index_uids.insert(_alter_index_ids.begin(),
_alter_index_ids.end());
+ }
+
+ // build output rowset
+ RETURN_IF_ERROR(input_rowset->link_files_to(
+ _tablet->tablet_path(), output_rs_writer->rowset_id(), 0,
+ without_index_uids.empty() ? &_alter_index_ids :
&without_index_uids));
auto input_rowset_meta = input_rowset->rowset_meta();
RowsetMetaSharedPtr rowset_meta = std::make_shared<RowsetMeta>();
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java
b/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java
index 3de1d43163..dbe96c8c33 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java
@@ -2898,8 +2898,8 @@ public class SchemaChangeHandler extends AlterHandler {
&& indexChangeJob.getTableId() == tableId
&& indexChangeJob.getPartitionName().equals(partitionName)
&& indexChangeJob.hasSameAlterInvertedIndex(isDrop,
alterIndexes)
- && indexChangeJob.getJobState() !=
IndexChangeJob.JobState.CANCELLED) {
- // if JobState is CANCELLED, also allow user to create job
again
+ && !indexChangeJob.isDone()) {
+ // if JobState is done (CANCELLED or FINISHED), also allow
user to create job again
return true;
}
}
diff --git a/regression-test/suites/inverted_index_p0/test_build_index.groovy
b/regression-test/suites/inverted_index_p0/test_build_index.groovy
index 13bf046592..d7b63b8d09 100644
--- a/regression-test/suites/inverted_index_p0/test_build_index.groovy
+++ b/regression-test/suites/inverted_index_p0/test_build_index.groovy
@@ -75,6 +75,22 @@ suite("test_build_index", "inverted_index"){
return "wait_timeout"
}
+ def wait_for_last_build_index_on_table_running = { table_name, OpTimeout ->
+ for(int t = delta_time; t <= OpTimeout; t += delta_time){
+ alter_res = sql """SHOW BUILD INDEX WHERE TableName =
"${table_name}" ORDER BY JobId """
+
+ def last_job_state = alter_res[alter_res.size()-1][7];
+ if (last_job_state == "RUNNING") {
+ logger.info(table_name + " last index job running, state: " +
last_job_state + ", detail: " + alter_res)
+ return last_job_state;
+ }
+ useTime = t
+ sleep(delta_time)
+ }
+ assertTrue(useTime <= OpTimeout,
"wait_for_last_build_index_on_table_finish timeout")
+ return "wait_timeout"
+ }
+
def tableName = "hackernews_1m"
sql "DROP TABLE IF EXISTS ${tableName}"
@@ -138,17 +154,18 @@ suite("test_build_index", "inverted_index"){
sql "sync"
+ // ADD INDEX
sql """ ALTER TABLE ${tableName} ADD INDEX idx_comment (`comment`) USING
INVERTED PROPERTIES("parser" = "english") """
+ // BUILD INDEX and expect state is RUNNING
sql """ BUILD INDEX idx_comment ON ${tableName} """
-
- sleep(1000)
-
+ def state = wait_for_last_build_index_on_table_running(tableName, timeout)
def result = sql """ SHOW BUILD INDEX WHERE TableName = "${tableName}"
ORDER BY JobId """
assertEquals(result[result.size()-1][1], tableName)
assertTrue(result[result.size()-1][3].contains("ADD INDEX"))
assertEquals(result[result.size()-1][7], "RUNNING")
+ // CANCEL BUILD INDEX and expect state is CANCELED
sql """ CANCEL BUILD INDEX ON ${tableName} (${result[result.size()-1][0]})
"""
result = sql """ SHOW BUILD INDEX WHERE TableName = "${tableName}" ORDER
BY JobId """
assertEquals(result[result.size()-1][1], tableName)
@@ -156,11 +173,12 @@ suite("test_build_index", "inverted_index"){
assertEquals(result[result.size()-1][7], "CANCELLED")
assertEquals(result[result.size()-1][8], "user cancelled")
-
+ // BUILD INDEX and expect state is FINISHED
sql """ BUILD INDEX idx_comment ON ${tableName}; """
- def state = wait_for_last_build_index_on_table_finish(tableName, timeout)
+ state = wait_for_last_build_index_on_table_finish(tableName, timeout)
assertEquals(state, "FINISHED")
+ // CANCEL BUILD INDEX in FINISHED state and expect exception
def success = false;
try {
sql """ CANCEL BUILD INDEX ON ${tableName}; """
@@ -169,4 +187,9 @@ suite("test_build_index", "inverted_index"){
logger.info(" CANCEL BUILD INDEX ON ${tableName} exception: " + ex)
}
assertFalse(success)
+
+ // BUILD INDEX again and expect state is FINISHED
+ sql """ BUILD INDEX idx_comment ON ${tableName}; """
+ state = wait_for_last_build_index_on_table_finish(tableName, timeout)
+ assertEquals(state, "FINISHED")
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
