From 764f13438b4ca87edfdf593a47767c7d1d3275c9 Mon Sep 17 00:00:00 2001 From: cambyzhu Date: Wed, 22 May 2024 20:05:37 +0800 Subject: [PATCH] fix add index and drop index do not work for history data --- .../rowset/segment_v2/bitmap_index_reader.cpp | 3 + .../segment_v2/bloom_filter_index_reader.cpp | 4 + be/src/olap/schema_change.cpp | 12 ++ be/src/olap/tablet_schema.cpp | 18 +++ be/src/olap/tablet_schema.h | 4 + .../test_index_ddl_fault_injection.out | 22 ++++ .../test_index_ddl_fault_injection.groovy | 116 ++++++++++++++++++ 7 files changed, 179 insertions(+) create mode 100644 regression-test/data/fault_injection_p0/test_index_ddl_fault_injection.out create mode 100644 regression-test/suites/fault_injection_p0/test_index_ddl_fault_injection.groovy diff --git a/be/src/olap/rowset/segment_v2/bitmap_index_reader.cpp b/be/src/olap/rowset/segment_v2/bitmap_index_reader.cpp index c76de68b7ba33c..fe3609f01adf12 100644 --- a/be/src/olap/rowset/segment_v2/bitmap_index_reader.cpp +++ b/be/src/olap/rowset/segment_v2/bitmap_index_reader.cpp @@ -24,6 +24,7 @@ #include #include "olap/types.h" +#include "util/debug_points.h" #include "vec/columns/column.h" #include "vec/common/string_ref.h" #include "vec/data_types/data_type.h" @@ -53,6 +54,8 @@ Status BitmapIndexReader::_load(bool use_page_cache, bool kept_in_memory, } Status BitmapIndexReader::new_iterator(BitmapIndexIterator** iterator) { + DBUG_EXECUTE_IF("BitmapIndexReader::new_iterator.fail", + { return Status::InternalError("new_iterator for bitmap index failed"); }); *iterator = new BitmapIndexIterator(this); return Status::OK(); } diff --git a/be/src/olap/rowset/segment_v2/bloom_filter_index_reader.cpp b/be/src/olap/rowset/segment_v2/bloom_filter_index_reader.cpp index dd663b01759ac8..09a8866495e59e 100644 --- a/be/src/olap/rowset/segment_v2/bloom_filter_index_reader.cpp +++ b/be/src/olap/rowset/segment_v2/bloom_filter_index_reader.cpp @@ -22,6 +22,7 @@ #include "olap/rowset/segment_v2/bloom_filter.h" #include "olap/types.h" +#include "util/debug_points.h" #include "vec/columns/column.h" #include "vec/common/string_ref.h" #include "vec/data_types/data_type.h" @@ -46,6 +47,9 @@ Status BloomFilterIndexReader::_load(bool use_page_cache, bool kept_in_memory) { } Status BloomFilterIndexReader::new_iterator(std::unique_ptr* iterator) { + DBUG_EXECUTE_IF("BloomFilterIndexReader::new_iterator.fail", { + return Status::InternalError("new_iterator for bloom filter index failed"); + }); iterator->reset(new BloomFilterIndexIterator(this)); return Status::OK(); } diff --git a/be/src/olap/schema_change.cpp b/be/src/olap/schema_change.cpp index 80dc6003619709..2ce0307611736c 100644 --- a/be/src/olap/schema_change.cpp +++ b/be/src/olap/schema_change.cpp @@ -766,6 +766,9 @@ Status SchemaChangeHandler::_do_process_alter_tablet_v2(const TAlterTabletReqV2& for (const auto& column : request.columns) { base_tablet_schema->append_column(TabletColumn(column)); } + // The request only include column info, do not include bitmap or bloomfilter index info, + // So we also need to copy index info from the real base tablet + base_tablet_schema->update_index_info_from(*base_tablet->tablet_schema()); } // Use tablet schema directly from base tablet, they are the newest schema, not contain // dropped column during light weight schema change. @@ -1284,6 +1287,15 @@ Status SchemaChangeHandler::_parse_request(const SchemaChangeParams& sc_params, if (column_mapping->expr != nullptr) { *sc_directly = true; return Status::OK(); + } else if (column_mapping->ref_column >= 0) { + const auto& column_new = new_tablet_schema->column(i); + const auto& column_old = base_tablet_schema->column(column_mapping->ref_column); + // index changed + if (column_new.is_bf_column() != column_old.is_bf_column() || + column_new.has_bitmap_index() != column_old.has_bitmap_index()) { + *sc_directly = true; + return Status::OK(); + } } } diff --git a/be/src/olap/tablet_schema.cpp b/be/src/olap/tablet_schema.cpp index 9bd451dd2a970e..55bba12e99148c 100644 --- a/be/src/olap/tablet_schema.cpp +++ b/be/src/olap/tablet_schema.cpp @@ -735,6 +735,24 @@ void TabletSchema::copy_from(const TabletSchema& tablet_schema) { init_from_pb(tablet_schema_pb); } +void TabletSchema::update_index_info_from(const TabletSchema& tablet_schema) { + for (auto& col : _cols) { + if (col.unique_id() < 0) { + continue; + } + const auto iter = tablet_schema._field_id_to_index.find(col.unique_id()); + if (iter == tablet_schema._field_id_to_index.end()) { + continue; + } + int32_t col_idx = iter->second; + if (col_idx < 0 || col_idx >= tablet_schema._cols.size()) { + continue; + } + col.set_is_bf_column(tablet_schema._cols[col_idx].is_bf_column()); + col.set_has_bitmap_index(tablet_schema._cols[col_idx].has_bitmap_index()); + } +} + std::string TabletSchema::to_key() const { TabletSchemaPB pb; to_schema_pb(&pb); diff --git a/be/src/olap/tablet_schema.h b/be/src/olap/tablet_schema.h index 8947beb6326204..0495b2c9460ad8 100644 --- a/be/src/olap/tablet_schema.h +++ b/be/src/olap/tablet_schema.h @@ -118,6 +118,9 @@ class TabletColumn { std::string get_aggregation_name() const { return _aggregation_name; } bool get_result_is_nullable() const { return _result_is_nullable; } + void set_is_bf_column(bool is_bf_column) { _is_bf_column = is_bf_column; } + void set_has_bitmap_index(bool has_bitmap_index) { _has_bitmap_index = has_bitmap_index; } + private: int32_t _unique_id; std::string _col_name; @@ -218,6 +221,7 @@ class TabletSchema { // Must make sure the row column is always the last column void add_row_column(); void copy_from(const TabletSchema& tablet_schema); + void update_index_info_from(const TabletSchema& tablet_schema); std::string to_key() const; int64_t mem_size() const { return _mem_size; } diff --git a/regression-test/data/fault_injection_p0/test_index_ddl_fault_injection.out b/regression-test/data/fault_injection_p0/test_index_ddl_fault_injection.out new file mode 100644 index 00000000000000..6bbc4f12702dac --- /dev/null +++ b/regression-test/data/fault_injection_p0/test_index_ddl_fault_injection.out @@ -0,0 +1,22 @@ +-- This file is automatically generated. You should know what you did if you want to edit this +-- !order1 -- +1 2 hello + +-- !order2 -- +1 2 hello + +-- !order3 -- +1 2 hello + +-- !order4 -- +1 2 hello + +-- !order5 -- +1 2 hello + +-- !order6 -- +1 2 hello + +-- !order7 -- +1 2 hello + diff --git a/regression-test/suites/fault_injection_p0/test_index_ddl_fault_injection.groovy b/regression-test/suites/fault_injection_p0/test_index_ddl_fault_injection.groovy new file mode 100644 index 00000000000000..d88524b91ea775 --- /dev/null +++ b/regression-test/suites/fault_injection_p0/test_index_ddl_fault_injection.groovy @@ -0,0 +1,116 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +import org.codehaus.groovy.runtime.IOGroovyMethods + +suite("test_index_ddl_fault_injection", "nonConcurrent") { + def getAlterColumnFinalState = { String tableName /* param */ -> + String showAlterColumnSql = "SHOW ALTER TABLE COLUMN WHERE TableName = '${tableName}' ORDER BY CreateTime DESC LIMIT 1" + int max_try_time = 300 + String jobState = "" + while(max_try_time--){ + def alterJob = sql showAlterColumnSql + jobState = alterJob[0][9] + if (jobState == "FINISHED" || jobState == "CANCELLED") { + sleep(3000) + logger.info("alter table ${tableName} column job is ${jobState}, msg: ${alterJob[0][10]} ") + return jobState + } else { + sleep(1000) + } + } + logger.info("alter table ${tableName} column job wait for 300s, status is ${jobState} ... return") + return jobState + } + + try { + sql "DROP TABLE IF EXISTS `test_index_ddl_fault_injection_tbl`" + sql """ + CREATE TABLE test_index_ddl_fault_injection_tbl ( + `k1` int(11) NULL COMMENT "", + `k2` int(11) NULL COMMENT "", + `v1` string NULL COMMENT "" + ) ENGINE=OLAP + DUPLICATE KEY(`k1`) + DISTRIBUTED BY HASH(`k1`) BUCKETS 1 + PROPERTIES ( "replication_allocation" = "tag.location.default: 1"); + """ + + sql """ INSERT INTO test_index_ddl_fault_injection_tbl VALUES (1, 2, "hello"), (3, 4, "world"); """ + sql 'sync' + + qt_order1 """ select * from test_index_ddl_fault_injection_tbl where v1 = 'hello'; """ + + // add bloom filter index + sql """ ALTER TABLE test_index_ddl_fault_injection_tbl set ("bloom_filter_columns" = "v1"); """ + assertEquals("FINISHED", getAlterColumnFinalState("test_index_ddl_fault_injection_tbl")) + + try { + qt_order2 """ select * from test_index_ddl_fault_injection_tbl where v1 = 'hello'; """ + GetDebugPoint().enableDebugPointForAllBEs("BloomFilterIndexReader::new_iterator.fail"); + test { + // if BE add bloom filter correctly, this query will call BloomFilterIndexReader::new_iterator + sql """ select * from test_index_ddl_fault_injection_tbl where v1 = 'hello'; """ + exception "new_iterator for bloom filter index failed" + } + } finally { + GetDebugPoint().disableDebugPointForAllBEs("BloomFilterIndexReader::new_iterator.fail"); + } + + // drop bloom filter index + sql """ ALTER TABLE test_index_ddl_fault_injection_tbl set ("bloom_filter_columns" = ""); """ + assertEquals("FINISHED", getAlterColumnFinalState("test_index_ddl_fault_injection_tbl")) + + try { + qt_order3 """ select * from test_index_ddl_fault_injection_tbl where v1 = 'hello'; """ + GetDebugPoint().enableDebugPointForAllBEs("BloomFilterIndexReader::new_iterator.fail"); + // if BE drop bloom filter correctly, this query will not call BloomFilterIndexReader::new_iterator + qt_order4 """ select * from test_index_ddl_fault_injection_tbl where v1 = 'hello'; """ + } finally { + GetDebugPoint().disableDebugPointForAllBEs("BloomFilterIndexReader::new_iterator.fail"); + } + + // add bitmap index + sql """ ALTER TABLE test_index_ddl_fault_injection_tbl ADD INDEX idx_bitmap(v1) USING BITMAP; """ + assertEquals("FINISHED", getAlterColumnFinalState("test_index_ddl_fault_injection_tbl")) + try { + qt_order5 """ select * from test_index_ddl_fault_injection_tbl where v1 = 'hello'; """ + GetDebugPoint().enableDebugPointForAllBEs("BitmapIndexReader::new_iterator.fail"); + test { + // if BE add bitmap index correctly, this query will call BitmapIndexReader::new_iterator + sql """ select * from test_index_ddl_fault_injection_tbl where v1 = 'hello'; """ + exception "new_iterator for bitmap index failed" + } + } finally { + GetDebugPoint().disableDebugPointForAllBEs("BitmapIndexReader::new_iterator.fail"); + } + + // drop bitmap index + sql """ DROP INDEX idx_bitmap ON test_index_ddl_fault_injection_tbl; """ + assertEquals("FINISHED", getAlterColumnFinalState("test_index_ddl_fault_injection_tbl")) + + try { + qt_order6 """ select * from test_index_ddl_fault_injection_tbl where v1 = 'hello'; """ + GetDebugPoint().enableDebugPointForAllBEs("BitmapIndexReader::new_iterator.fail"); + // if BE drop bitmap index correctly, this query will not call BitmapIndexReader::new_iterator + qt_order7 """ select * from test_index_ddl_fault_injection_tbl where v1 = 'hello'; """ + } finally { + GetDebugPoint().disableDebugPointForAllBEs("BitmapIndexReader::new_iterator.fail"); + } + } finally { + } +}