database/doris
2
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

[Enhancement](inverted index) make InvertedIndexReader shared_from_this (#21381)

Browse Source 
This PR proposes several changes to improve code safety and readability by replacing raw pointers with smart pointers in several places.

use enable_factory_creator in InvertedIndexIterator and InvertedIndexReader, remove explicit new constructor.
make InvertedIndexReader shared_from_this, it may desctruct when InvertedIndexIterator use it.
This commit is contained in:
airborne12
2023-07-06 11:52:59 +08:00
committed by GitHub
parent fb14950887
commit 9d2f879bd2
6 changed files with 58 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
be/src/olap/rowset/segment_v2/column_reader.cpp
View File

@ -233,7 +233,7 @@ Status ColumnReader::new_bitmap_index_iterator(BitmapIndexIterator** iterator) {
Status ColumnReader::new_inverted_index_iterator(const TabletIndex* index_meta,
OlapReaderStatistics* stats,
InvertedIndexIterator** iterator) {
std::unique_ptr<InvertedIndexIterator>* iterator) {
RETURN_IF_ERROR(_ensure_inverted_index_loaded(index_meta));
if (_inverted_index) {
RETURN_IF_ERROR(_inverted_index->new_iterator(stats, iterator));
@ -505,16 +505,16 @@ Status ColumnReader::_load_inverted_index_index(const TabletIndex* index_meta) {
if (is_string_type(type)) {
if (parser_type != InvertedIndexParserType::PARSER_NONE) {
_inverted_index.reset(new FullTextIndexReader(
_file_reader->fs(), _file_reader->path().native(), index_meta));
_inverted_index = FullTextIndexReader::create_shared(
_file_reader->fs(), _file_reader->path().native(), index_meta);
return Status::OK();
} else {
_inverted_index.reset(new StringTypeInvertedIndexReader(
_file_reader->fs(), _file_reader->path().native(), index_meta));
_inverted_index = StringTypeInvertedIndexReader::create_shared(
_file_reader->fs(), _file_reader->path().native(), index_meta);
}
} else if (is_numeric_type(type)) {
_inverted_index.reset(
new BkdIndexReader(_file_reader->fs(), _file_reader->path().native(), index_meta));
_inverted_index = BkdIndexReader::create_shared(_file_reader->fs(),
_file_reader->path().native(), index_meta);
} else {
_inverted_index.reset();
}

4
be/src/olap/rowset/segment_v2/column_reader.h
View File

@ -120,7 +120,7 @@ public:
Status new_bitmap_index_iterator(BitmapIndexIterator** iterator);
Status new_inverted_index_iterator(const TabletIndex* index_meta, OlapReaderStatistics* stats,
InvertedIndexIterator** iterator);
std::unique_ptr<InvertedIndexIterator>* iterator);
// Seek to the first entry in the column.
Status seek_to_first(OrdinalPageIndexIterator* iter);
@ -237,7 +237,7 @@ private:
std::unique_ptr<ZoneMapIndexReader> _zone_map_index;
std::unique_ptr<OrdinalIndexReader> _ordinal_index;
std::unique_ptr<BitmapIndexReader> _bitmap_index;
std::unique_ptr<InvertedIndexReader> _inverted_index;
std::shared_ptr<InvertedIndexReader> _inverted_index;
std::unique_ptr<BloomFilterIndexReader> _bloom_filter_index;
DorisCallOnce<Status> _load_zone_map_index_once;
DorisCallOnce<Status> _load_ordinal_index_once;

17
be/src/olap/rowset/segment_v2/inverted_index_reader.cpp
View File

@ -213,8 +213,8 @@ Status InvertedIndexReader::read_null_bitmap(InvertedIndexQueryCacheHandle* cach
}
Status FullTextIndexReader::new_iterator(OlapReaderStatistics* stats,
InvertedIndexIterator** iterator) {
*iterator = new InvertedIndexIterator(stats, this);
std::unique_ptr<InvertedIndexIterator>* iterator) {
*iterator = InvertedIndexIterator::create_unique(stats, shared_from_this());
return Status::OK();
}
@ -404,9 +404,9 @@ InvertedIndexReaderType FullTextIndexReader::type() {
return InvertedIndexReaderType::FULLTEXT;
}
Status StringTypeInvertedIndexReader::new_iterator(OlapReaderStatistics* stats,
InvertedIndexIterator** iterator) {
*iterator = new InvertedIndexIterator(stats, this);
Status StringTypeInvertedIndexReader::new_iterator(
OlapReaderStatistics* stats, std::unique_ptr<InvertedIndexIterator>* iterator) {
*iterator = InvertedIndexIterator::create_unique(stats, shared_from_this());
return Status::OK();
}
@ -543,13 +543,14 @@ BkdIndexReader::BkdIndexReader(io::FileSystemSPtr fs, const std::string& path,
LOG(WARNING) << "bkd index: " << index_file.string() << " not exist.";
return;
}
_compoundReader = new DorisCompoundReader(
_compoundReader = std::make_unique<DorisCompoundReader>(
DorisCompoundDirectory::getDirectory(fs, index_dir.c_str()), index_file_name.c_str(),
config::inverted_index_read_buffer_size);
}
Status BkdIndexReader::new_iterator(OlapReaderStatistics* stats, InvertedIndexIterator** iterator) {
*iterator = new InvertedIndexIterator(stats, this);
Status BkdIndexReader::new_iterator(OlapReaderStatistics* stats,
std::unique_ptr<InvertedIndexIterator>* iterator) {
*iterator = InvertedIndexIterator::create_unique(stats, shared_from_this());
return Status::OK();
}

57
be/src/olap/rowset/segment_v2/inverted_index_reader.h
View File

@ -64,15 +64,16 @@ enum class InvertedIndexReaderType {
BKD = 2,
};
class InvertedIndexReader {
class InvertedIndexReader : public std::enable_shared_from_this<InvertedIndexReader> {
public:
explicit InvertedIndexReader(io::FileSystemSPtr fs, const std::string& path,
const TabletIndex* index_meta)
: _fs(std::move(fs)), _path(path), _index_meta(*index_meta) {}
: _fs(fs), _path(path), _index_meta(*index_meta) {}
virtual ~InvertedIndexReader() = default;
// create a new column iterator. Client should delete returned iterator
virtual Status new_iterator(OlapReaderStatistics* stats, InvertedIndexIterator** iterator) = 0;
virtual Status new_iterator(OlapReaderStatistics* stats,
std::unique_ptr<InvertedIndexIterator>* iterator) = 0;
virtual Status query(OlapReaderStatistics* stats, const std::string& column_name,
const void* query_value, InvertedIndexQueryType query_type,
roaring::Roaring* bit_map) = 0;
@ -86,9 +87,9 @@ public:
virtual InvertedIndexReaderType type() = 0;
bool indexExists(io::Path& index_file_path);
uint32_t get_index_id() const { return _index_meta.index_id(); }
[[nodiscard]] uint32_t get_index_id() const { return _index_meta.index_id(); }
const std::map<string, string>& get_index_properties() const {
[[nodiscard]] const std::map<string, string>& get_index_properties() const {
return _index_meta.properties();
}
@ -103,18 +104,21 @@ protected:
bool _is_match_query(InvertedIndexQueryType query_type);
friend class InvertedIndexIterator;
io::FileSystemSPtr _fs;
std::string _path;
const std::string& _path;
TabletIndex _index_meta;
};
class FullTextIndexReader : public InvertedIndexReader {
ENABLE_FACTORY_CREATOR(FullTextIndexReader);
public:
explicit FullTextIndexReader(io::FileSystemSPtr fs, const std::string& path,
const TabletIndex* index_meta)
: InvertedIndexReader(std::move(fs), path, index_meta) {}
: InvertedIndexReader(fs, path, index_meta) {}
~FullTextIndexReader() override = default;
Status new_iterator(OlapReaderStatistics* stats, InvertedIndexIterator** iterator) override;
Status new_iterator(OlapReaderStatistics* stats,
std::unique_ptr<InvertedIndexIterator>* iterator) override;
Status query(OlapReaderStatistics* stats, const std::string& column_name,
const void* query_value, InvertedIndexQueryType query_type,
roaring::Roaring* bit_map) override;
@ -128,13 +132,16 @@ public:
};
class StringTypeInvertedIndexReader : public InvertedIndexReader {
ENABLE_FACTORY_CREATOR(StringTypeInvertedIndexReader);
public:
explicit StringTypeInvertedIndexReader(io::FileSystemSPtr fs, const std::string& path,
const TabletIndex* index_meta)
: InvertedIndexReader(std::move(fs), path, index_meta) {}
: InvertedIndexReader(fs, path, index_meta) {}
~StringTypeInvertedIndexReader() override = default;
Status new_iterator(OlapReaderStatistics* stats, InvertedIndexIterator** iterator) override;
Status new_iterator(OlapReaderStatistics* stats,
std::unique_ptr<InvertedIndexIterator>* iterator) override;
Status query(OlapReaderStatistics* stats, const std::string& column_name,
const void* query_value, InvertedIndexQueryType query_type,
roaring::Roaring* bit_map) override;
@ -181,18 +188,28 @@ public:
};
class BkdIndexReader : public InvertedIndexReader {
ENABLE_FACTORY_CREATOR(BkdIndexReader);
public:
explicit BkdIndexReader(io::FileSystemSPtr fs, const std::string& path,
const TabletIndex* index_meta);
~BkdIndexReader() override {
if (_compoundReader != nullptr) {
_compoundReader->close();
delete _compoundReader;
_compoundReader = nullptr;
try {
_compoundReader->close();
} catch (const CLuceneError& e) {
// Handle exception, e.g., log it, but don't rethrow.
LOG(ERROR) << "Exception caught in BkdIndexReader destructor: " << e.what()
<< std::endl;
} catch (...) {
// Handle all other exceptions, but don't rethrow.
LOG(ERROR) << "Unknown exception caught in BkdIndexReader destructor." << std::endl;
}
}
}
Status new_iterator(OlapReaderStatistics* stats, InvertedIndexIterator** iterator) override;
Status new_iterator(OlapReaderStatistics* stats,
std::unique_ptr<InvertedIndexIterator>* iterator) override;
Status query(OlapReaderStatistics* stats, const std::string& column_name,
const void* query_value, InvertedIndexQueryType query_type,
@ -211,12 +228,14 @@ public:
private:
const TypeInfo* _type_info {};
const KeyCoder* _value_key_coder {};
DorisCompoundReader* _compoundReader;
std::unique_ptr<DorisCompoundReader> _compoundReader;
};
class InvertedIndexIterator {
ENABLE_FACTORY_CREATOR(InvertedIndexIterator);
public:
InvertedIndexIterator(OlapReaderStatistics* stats, InvertedIndexReader* reader)
InvertedIndexIterator(OlapReaderStatistics* stats, std::shared_ptr<InvertedIndexReader> reader)
: _stats(stats), _reader(reader) {}
Status read_from_inverted_index(const std::string& column_name, const void* query_value,
@ -230,12 +249,12 @@ public:
return _reader->read_null_bitmap(cache_handle, dir);
}
InvertedIndexReaderType get_inverted_index_reader_type() const;
const std::map<string, string>& get_index_properties() const;
[[nodiscard]] InvertedIndexReaderType get_inverted_index_reader_type() const;
[[nodiscard]] const std::map<string, string>& get_index_properties() const;
private:
OlapReaderStatistics* _stats;
InvertedIndexReader* _reader;
std::shared_ptr<InvertedIndexReader> _reader;
};
} // namespace segment_v2

4
be/src/olap/rowset/segment_v2/segment.cpp
View File

@ -346,10 +346,8 @@ Status Segment::new_inverted_index_iterator(const TabletColumn& tablet_column,
std::unique_ptr<InvertedIndexIterator>* iter) {
auto col_unique_id = tablet_column.unique_id();
if (_column_readers.count(col_unique_id) > 0 && index_meta) {
InvertedIndexIterator* it;
RETURN_IF_ERROR(_column_readers.at(col_unique_id)
->new_inverted_index_iterator(index_meta, stats, &it));
iter->reset(it);
->new_inverted_index_iterator(index_meta, stats, iter));
return Status::OK();
}
return Status::OK();

1
be/src/olap/rowset/segment_v2/segment_iterator.cpp
View File

@ -820,6 +820,7 @@ std::string SegmentIterator::_gen_predicate_result_sign(ColumnPredicateInfo* pre
bool SegmentIterator::_column_has_fulltext_index(int32_t unique_id) {
bool has_fulltext_index =
_inverted_index_iterators.count(unique_id) > 0 &&
_inverted_index_iterators[unique_id] != nullptr &&
_inverted_index_iterators[unique_id]->get_inverted_index_reader_type() ==
InvertedIndexReaderType::FULLTEXT;