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
…#35214)

## Proposed changes

Only inverted index support light index change, bitmap and bloomfilter
index do not support light index change.

Before this pr, if we add or drop bitmap and bloomfilter index, it only
do linked schema change, do not rewrite index data for history data.
  • Loading branch information
cambyzju authored and dataroaring committed May 31, 2024
1 parent 95e87c4 commit b8d66bf
Show file tree
Hide file tree
Showing 6 changed files with 109 additions and 0 deletions.
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
3 changes: 3 additions & 0 deletions be/src/olap/schema_change.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -731,6 +731,9 @@ SchemaChangeJob::SchemaChangeJob(StorageEngine& local_storage_engine,
if (_base_tablet && _new_tablet) {
_base_tablet_schema = std::make_shared<TabletSchema>();
_base_tablet_schema->update_tablet_columns(*_base_tablet->tablet_schema(), request.columns);
// 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());
// During a schema change, the extracted columns of a variant should not be included in the tablet schema.
// This is because the schema change for a variant needs to ignore the extracted columns.
// Otherwise, the schema types in different rowsets might be inconsistent. When performing a schema change,
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 @@ -995,6 +995,24 @@ void TabletSchema::copy_from(const TabletSchema& tablet_schema) {
_table_id = tablet_schema.table_id();
}

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;
}
auto 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
2 changes: 2 additions & 0 deletions be/src/olap/tablet_schema.h
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ class TabletColumn {
int32_t parent_unique_id() const { return _parent_col_unique_id; }
void set_parent_unique_id(int32_t col_unique_id) { _parent_col_unique_id = col_unique_id; }
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; }
std::shared_ptr<const vectorized::IDataType> get_vec_type() const;

void append_sparse_column(TabletColumn column);
Expand Down Expand Up @@ -284,6 +285,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;
// Don't use.
// TODO: memory size of TabletSchema cannot be accurately tracked.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
-- 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

Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// 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") {
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
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
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");
}
} finally {
}
}

0 comments on commit b8d66bf

Please sign in to comment.