Skip to content

Commit e904800

Browse files
author
qiye
authored
[fix](index compaction)Fix construct index compaction columns core when reader close error (#46508)
fix #46507
1 parent f360ee2 commit e904800

File tree

3 files changed

+199
-61
lines changed

3 files changed

+199
-61
lines changed

‎be/src/olap/compaction.cpp

Lines changed: 75 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -858,72 +858,86 @@ Status Compaction::construct_output_rowset_writer(RowsetWriterContext& ctx, bool
858858
return false;
859859
}
860860
for (auto i = 0; i < rowset->num_segments(); i++) {
861-
auto segment_file = rowset->segment_file_path(i);
862-
io::Path segment_path(segment_file);
863-
auto inverted_index_file_reader =
864-
std::make_unique<InvertedIndexFileReader>(
865-
fs, segment_path.parent_path(),
866-
segment_path.filename(),
867-
_cur_tablet_schema
868-
->get_inverted_index_storage_format());
869-
bool open_idx_file_cache = false;
870-
auto st = inverted_index_file_reader->init(
871-
config::inverted_index_read_buffer_size,
872-
open_idx_file_cache);
873-
if (!st.ok()) {
874-
LOG(WARNING) << "init index "
875-
<< inverted_index_file_reader->get_index_file_path(
876-
index_meta)
877-
<< " error:" << st;
878-
return false;
879-
}
880-
881-
bool exists = false;
882-
if (!inverted_index_file_reader
883-
->index_file_exist(index_meta, &exists)
884-
.ok()) {
885-
LOG(ERROR) << inverted_index_file_reader->get_index_file_path(
886-
index_meta)
887-
<< " fs->exists error";
888-
return false;
889-
}
890-
891-
if (!exists) {
861+
std::string index_file_path;
862+
try {
863+
auto segment_file = rowset->segment_file_path(i);
864+
io::Path segment_path(segment_file);
865+
auto inverted_index_file_reader =
866+
std::make_unique<InvertedIndexFileReader>(
867+
fs, segment_path.parent_path(),
868+
segment_path.filename(),
869+
_cur_tablet_schema
870+
->get_inverted_index_storage_format());
871+
bool open_idx_file_cache = false;
872+
auto st = inverted_index_file_reader->init(
873+
config::inverted_index_read_buffer_size,
874+
open_idx_file_cache);
875+
index_file_path =
876+
inverted_index_file_reader->get_index_file_path(
877+
index_meta);
878+
if (!st.ok()) {
879+
LOG(WARNING) << "init index " << index_file_path
880+
<< " error:" << st;
881+
return false;
882+
}
883+
884+
bool exists = false;
885+
if (!inverted_index_file_reader
886+
->index_file_exist(index_meta, &exists)
887+
.ok()) {
888+
LOG(ERROR) << index_file_path << " fs->exists error";
889+
return false;
890+
}
891+
892+
if (!exists) {
893+
LOG(WARNING)
894+
<< "tablet[" << _tablet->tablet_id()
895+
<< "] column_unique_id[" << col_unique_id << "],"
896+
<< index_file_path
897+
<< " is not exists, will skip index compaction";
898+
return false;
899+
}
900+
901+
// check index meta
902+
auto result = inverted_index_file_reader->open(index_meta);
903+
if (!result.has_value()) {
904+
LOG(WARNING) << "open index " << index_file_path
905+
<< " error:" << result.error();
906+
return false;
907+
}
908+
auto reader = std::move(result.value());
909+
std::vector<std::string> files;
910+
reader->list(&files);
911+
reader->close();
912+
913+
DBUG_EXECUTE_IF(
914+
"Compaction::construct_skip_inverted_index_index_"
915+
"reader_"
916+
"close_error",
917+
{
918+
_CLTHROWA(CL_ERR_IO,
919+
"debug point: reader close error");
920+
})
921+
922+
// why is 3?
923+
// slice type index file at least has 3 files: null_bitmap, segments_N, segments.gen
924+
if (files.size() < 3) {
925+
LOG(WARNING) << "tablet[" << _tablet->tablet_id()
926+
<< "] column_unique_id[" << col_unique_id
927+
<< "]," << index_file_path
928+
<< " is corrupted, will skip index compaction";
929+
return false;
930+
}
931+
} catch (CLuceneError& err) {
892932
LOG(WARNING) << "tablet[" << _tablet->tablet_id()
893-
<< "] column_unique_id[" << col_unique_id << "],"
894-
<< inverted_index_file_reader->get_index_file_path(
895-
index_meta)
896-
<< " is not exists, will skip index compaction";
897-
return false;
898-
}
899-
900-
// check index meta
901-
auto result = inverted_index_file_reader->open(index_meta);
902-
if (!result.has_value()) {
903-
LOG(WARNING) << "open index "
904-
<< inverted_index_file_reader->get_index_file_path(
905-
index_meta)
906-
<< " error:" << result.error();
907-
return false;
908-
}
909-
auto reader = std::move(result.value());
910-
std::vector<std::string> files;
911-
reader->list(&files);
912-
reader->close();
913-
914-
// why is 3?
915-
// slice type index file at least has 3 files: null_bitmap, segments_N, segments.gen
916-
if (files.size() < 3) {
917-
LOG(WARNING) << "tablet[" << _tablet->tablet_id()
918-
<< "] column_unique_id[" << col_unique_id << "],"
919-
<< inverted_index_file_reader->get_index_file_path(
920-
index_meta)
921-
<< " is corrupted, will skip index compaction";
933+
<< "] column_unique_id[" << col_unique_id
934+
<< "] open index[" << index_file_path
935+
<< "], will skip index compaction, error:"
936+
<< err.what();
922937
return false;
923938
}
924939
}
925940
return true;
926-
return true;
927941
});
928942
if (all_have_inverted_index) {
929943
ctx.columns_to_do_index_compaction.insert(col_unique_id);

‎regression-test/data/fault_injection_p0/test_index_compaction_fault_injection.out

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,106 @@
204204
3 bason bason hate pear 99
205205
3 bason bason hate pear 99
206206

207+
-- !sql --
208+
1 andy andy love apple 100
209+
1 andy andy love apple 100
210+
1 andy andy love apple 100
211+
1 andy andy love apple 100
212+
1 andy andy love apple 100
213+
1 bason bason hate pear 99
214+
1 bason bason hate pear 99
215+
1 bason bason hate pear 99
216+
1 bason bason hate pear 99
217+
1 bason bason hate pear 99
218+
2 andy andy love apple 100
219+
2 andy andy love apple 100
220+
2 andy andy love apple 100
221+
2 andy andy love apple 100
222+
2 andy andy love apple 100
223+
2 bason bason hate pear 99
224+
2 bason bason hate pear 99
225+
2 bason bason hate pear 99
226+
2 bason bason hate pear 99
227+
2 bason bason hate pear 99
228+
3 andy andy love apple 100
229+
3 andy andy love apple 100
230+
3 andy andy love apple 100
231+
3 andy andy love apple 100
232+
3 andy andy love apple 100
233+
3 bason bason hate pear 99
234+
3 bason bason hate pear 99
235+
3 bason bason hate pear 99
236+
3 bason bason hate pear 99
237+
3 bason bason hate pear 99
238+
239+
-- !sql --
240+
1 andy andy love apple 100
241+
1 andy andy love apple 100
242+
1 andy andy love apple 100
243+
1 andy andy love apple 100
244+
1 andy andy love apple 100
245+
2 andy andy love apple 100
246+
2 andy andy love apple 100
247+
2 andy andy love apple 100
248+
2 andy andy love apple 100
249+
2 andy andy love apple 100
250+
3 andy andy love apple 100
251+
3 andy andy love apple 100
252+
3 andy andy love apple 100
253+
3 andy andy love apple 100
254+
3 andy andy love apple 100
255+
256+
-- !sql --
257+
1 bason bason hate pear 99
258+
1 bason bason hate pear 99
259+
1 bason bason hate pear 99
260+
1 bason bason hate pear 99
261+
1 bason bason hate pear 99
262+
2 bason bason hate pear 99
263+
2 bason bason hate pear 99
264+
2 bason bason hate pear 99
265+
2 bason bason hate pear 99
266+
2 bason bason hate pear 99
267+
3 bason bason hate pear 99
268+
3 bason bason hate pear 99
269+
3 bason bason hate pear 99
270+
3 bason bason hate pear 99
271+
3 bason bason hate pear 99
272+
273+
-- !sql --
274+
1 bason bason hate pear 99
275+
1 bason bason hate pear 99
276+
1 bason bason hate pear 99
277+
1 bason bason hate pear 99
278+
1 bason bason hate pear 99
279+
2 bason bason hate pear 99
280+
2 bason bason hate pear 99
281+
2 bason bason hate pear 99
282+
2 bason bason hate pear 99
283+
2 bason bason hate pear 99
284+
3 bason bason hate pear 99
285+
3 bason bason hate pear 99
286+
3 bason bason hate pear 99
287+
3 bason bason hate pear 99
288+
3 bason bason hate pear 99
289+
290+
-- !sql --
291+
1 bason bason hate pear 99
292+
2 bason bason hate pear 99
293+
3 bason bason hate pear 99
294+
295+
-- !sql --
296+
297+
-- !sql --
298+
1 bason bason hate pear 99
299+
2 bason bason hate pear 99
300+
3 bason bason hate pear 99
301+
302+
-- !sql --
303+
1 bason bason hate pear 99
304+
2 bason bason hate pear 99
305+
3 bason bason hate pear 99
306+
207307
-- !sql --
208308
1 bason bason hate pear 99
209309
2 bason bason hate pear 99

‎regression-test/suites/fault_injection_p0/test_index_compaction_fault_injection.groovy

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,30 @@ suite("test_index_compaction_failure_injection", "nonConcurrent") {
223223
assert (rowsetCount == 1 * replicaNum)
224224

225225
run_sql.call()
226+
227+
// insert more data, trigger full compaction again
228+
insert_data.call()
229+
230+
// insert 6 rows, so there are 7 rowsets.
231+
rowsetCount = get_rowset_count.call(tablets);
232+
assert (rowsetCount == 7 * replicaNum)
233+
234+
// trigger full compaction for all tablets with fault injection
235+
try {
236+
GetDebugPoint().enableDebugPointForAllBEs("Compaction::construct_skip_inverted_index_index_reader_close_error")
237+
logger.info("trigger_full_compaction_on_tablets with fault injection: Compaction::construct_skip_inverted_index_index_reader_close_error")
238+
trigger_full_compaction_on_tablets.call(tablets)
239+
wait_full_compaction_done.call(tablets)
240+
} finally {
241+
GetDebugPoint().disableDebugPointForAllBEs("Compaction::construct_skip_inverted_index_index_reader_close_error")
242+
}
243+
244+
// the debug point will cause index skip index compaction, so the compaction process will be successful.
245+
// after full compaction, there is only 1 rowset.
246+
rowsetCount = get_rowset_count.call(tablets);
247+
assert (rowsetCount == 1 * replicaNum)
248+
249+
run_sql.call()
226250
}
227251

228252
boolean invertedIndexCompactionEnable = false

0 commit comments

Comments
 (0)