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

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -985,6 +985,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 @@ -282,6 +283,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 {
}
}
Loading