Skip to content

Commit

Permalink
[fix](index) should not use light index change for bloom filter index #…
Browse files Browse the repository at this point in the history
  • Loading branch information
cambyzju committed May 28, 2024
1 parent 757e709 commit b511413
Show file tree
Hide file tree
Showing 7 changed files with 179 additions and 0 deletions.
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 @@ -742,6 +742,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 {
}
}

0 comments on commit b511413

Please sign in to comment.