qidaye commented on code in PR #41625:
URL: https://github.com/apache/doris/pull/41625#discussion_r1820288638
##########
be/src/olap/rowset/segment_v2/inverted_index_file_writer.cpp:
##########
@@ -370,14 +372,10 @@ int64_t InvertedIndexFileWriter::write_v2() {
out_dir->set_file_writer_opts(_opts);
std::unique_ptr<lucene::store::IndexOutput> compound_file_output;
- // idx v2 writer != nullptr means memtable on sink node now
- if (_idx_v2_writer != nullptr) {
- compound_file_output = std::unique_ptr<lucene::store::IndexOutput>(
- out_dir->createOutputV2(_idx_v2_writer.get()));
- } else {
Review Comment:
What mechanism ensures that _idx_v2_writer must not be null?
##########
be/src/olap/rowset/beta_rowset_writer.cpp:
##########
@@ -748,12 +801,15 @@ Status BetaRowsetWriter::build(RowsetSharedPtr& rowset) {
:
_context.tablet_schema;
_rowset_meta->set_tablet_schema(rowset_schema);
- if (auto idx_files_info =
_idx_files_info.get_inverted_files_info(_segment_start_id);
- !idx_files_info.has_value()) [[unlikely]] {
- LOG(ERROR) << "expected inverted index files info, but none presents: "
- << idx_files_info.error();
- } else {
- _rowset_meta->add_inverted_index_files_info(idx_files_info.value());
+ // If segment compaction occurs, the idx file info will become inaccurate.
Review Comment:
How to handle the idx_files_info in segcompaction?
##########
be/src/olap/rowset/segment_v2/segment_writer.cpp:
##########
@@ -132,17 +133,6 @@ SegmentWriter::SegmentWriter(io::FileWriter* file_writer,
uint32_t segment_id,
}
}
}
- if (_tablet_schema->has_inverted_index()) {
- _inverted_index_file_writer =
std::make_unique<InvertedIndexFileWriter>(
- _opts.rowset_ctx->fs(),
- std::string
{InvertedIndexDescriptor::get_index_file_path_prefix(
- file_writer->path().c_str())},
- _opts.rowset_ctx->rowset_id.to_string(), segment_id,
- _tablet_schema->get_inverted_index_storage_format(),
- std::move(inverted_file_writer));
- _inverted_index_file_writer->set_file_writer_opts(
Review Comment:
Now where to set this option?
##########
be/src/olap/rowset/beta_rowset_writer.cpp:
##########
@@ -891,13 +947,36 @@ Status BaseBetaRowsetWriter::create_file_writer(uint32_t
segment_id, io::FileWri
fmt::format("failed to create file = {}, file type = {}",
segment_path, file_type));
}
+Status BaseBetaRowsetWriter::create_inverted_index_file_writer(
+ uint32_t segment_id, InvertedIndexFileWriterPtr* index_file_writer) {
+
RETURN_IF_ERROR(RowsetWriter::create_inverted_index_file_writer(segment_id,
index_file_writer));
+ // used for inverted index format v1
+
(*index_file_writer)->set_file_writer_opts(_context.get_file_writer_options());
+ return Status::OK();
+}
+
Status BetaRowsetWriter::_create_segment_writer_for_segcompaction(
std::unique_ptr<segment_v2::SegmentWriter>* writer, int64_t begin,
int64_t end) {
DCHECK(begin >= 0 && end >= 0);
std::string path =
BetaRowset::local_segment_path_segcompacted(_context.tablet_path,
_context.rowset_id, begin, end);
io::FileWriterPtr file_writer;
RETURN_IF_ERROR(_create_file_writer(path, file_writer));
+ std::string prefix = std::string
{InvertedIndexDescriptor::get_index_file_path_prefix(path)};
+ std::string index_path =
InvertedIndexDescriptor::get_index_file_path_v2(prefix);
+
+ InvertedIndexFileWriterPtr index_file_writer;
+ if (_context.tablet_schema->has_inverted_index()) {
+ io::FileWriterPtr idx_file_writer;
+ if (_context.tablet_schema->get_inverted_index_storage_format() !=
+ InvertedIndexStorageFormatPB::V1) {
+ RETURN_IF_ERROR(_create_file_writer(index_path, idx_file_writer));
+ }
+ index_file_writer = std::make_unique<InvertedIndexFileWriter>(
+ _context.fs(), path, _context.rowset_id.to_string(),
_num_segcompacted,
+ _context.tablet_schema->get_inverted_index_storage_format(),
+ std::move(idx_file_writer));
Review Comment:
double move?
##########
be/src/olap/rowset/rowset_writer.h:
##########
@@ -95,6 +96,24 @@ class RowsetWriter {
return Status::NotSupported("RowsetWriter does not support
create_file_writer");
}
+ virtual Status create_inverted_index_file_writer(
+ uint32_t segment_id, InvertedIndexFileWriterPtr*
index_file_writer) {
+ std::string segment_prefix
{InvertedIndexDescriptor::get_index_file_path_prefix(
+ _context.segment_path(segment_id))};
+ // Create file writer for the inverted index format v2.
+ io::FileWriterPtr idx_file_v2_ptr;
+ if (_context.tablet_schema->get_inverted_index_storage_format() !=
+ InvertedIndexStorageFormatPB::V1) {
+ RETURN_IF_ERROR(
+ create_file_writer(segment_id, idx_file_v2_ptr,
FileType::INVERTED_INDEX_FILE));
+ }
+ *index_file_writer = std::make_unique<InvertedIndexFileWriter>(
+ _context.fs(), segment_prefix, _context.rowset_id.to_string(),
segment_id,
+ _context.tablet_schema->get_inverted_index_storage_format(),
+ std::move(idx_file_v2_ptr));
Review Comment:
InvertedIndexFileWriter's constructor already has std::move, double move?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
