Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix](index) should not use light index change for bloom filter index #34732

Merged
merged 1 commit into from
May 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions be/src/olap/rowset/segment_v2/bitmap_index_reader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <roaring/roaring.hh>

#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"
Expand Down Expand Up @@ -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();
}
Expand Down
4 changes: 4 additions & 0 deletions be/src/olap/rowset/segment_v2/bloom_filter_index_reader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -46,6 +47,9 @@ Status BloomFilterIndexReader::_load(bool use_page_cache, bool kept_in_memory) {
}

Status BloomFilterIndexReader::new_iterator(std::unique_ptr<BloomFilterIndexIterator>* 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();
}
Expand Down
12 changes: 12 additions & 0 deletions be/src/olap/schema_change.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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();
}
}
}

Expand Down
18 changes: 18 additions & 0 deletions be/src/olap/tablet_schema.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions be/src/olap/tablet_schema.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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; }

Expand Down
Original file line number Diff line number Diff line change
@@ -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

Original file line number Diff line number Diff line change
@@ -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 {
}
}