Copilot commented on code in PR #68365:
URL: https://github.com/apache/doris/pull/68365#discussion_r4068358997


##########
be/src/storage/rowset/rowset_meta.cpp:
##########
@@ -56,6 +57,27 @@ RowsetMeta::~RowsetMeta() {
     }
 }
 
+Status RowsetMeta::collect_packed_slice_location(const io::FileWriter& 
file_writer,
+                                                 const std::string& file_path) 
{
+    if (file_writer.state() != io::FileWriter::State::CLOSED) {
+        return Status::OK();
+    }
+    if (!file_writer.is_in_packed_file()) {
+        return Status::OK();
+    }
+
+    // Read the writer-owned location so collection cannot race with global 
index cleanup.
+    io::PackedSliceLocation location;
+    RETURN_IF_ERROR(static_cast<const io::PackedFileWriter&>(file_writer)
+                            .get_packed_slice_location(&location));
+    DORIS_CHECK(!location.packed_file_path.empty());

Review Comment:
   `LoadStreamWriter::close_writer` explicitly permits a zero-byte 
inverted-index file, but `PackedFileWriter::is_in_packed_file()` remains true 
when no data was appended, so `get_packed_slice_location()` returns an empty 
location for that file. The new check therefore aborts cloud stream loads that 
produce an empty index; retain the previous behavior of treating an empty 
packed location as a non-packed file and returning success.



##########
be/src/exec/sink/writer/vtablet_writer_v2.cpp:
##########
@@ -318,7 +327,8 @@ Status VTabletWriterV2::_open_streams_to_backend(int64_t 
dst_id, LoadStreamStubs
                     { tablets_for_schema.clear(); });
     auto st = streams.open(_state->exec_env()->brpc_streaming_client_cache(), 
*node_info, _txn_id,
                            *_schema, tablets_for_schema, _total_streams, 
idle_timeout_ms,
-                           _state->enable_profile());
+                           _state->enable_profile(), _txn_expiration, 
_storage_vault_id,
+                           _write_file_cache);

Review Comment:
   This forwards the expiration and vault only for the direct `LoadStreamStub` 
path. When memtable-on-sink creates its internal `WriteRequest` in 
`_write_memtable`, it still sets neither `txn_expiration` nor 
`storage_vault_id` (the latter is `{}`), so the receiving `CloudRowsetBuilder` 
gets expiration 0 and cannot select a non-default vault. Propagate both members 
into that request as well; otherwise this new cloud path can immediately expire 
txn metadata or fail to write to the configured vault.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to