Copilot commented on code in PR #68366:
URL: https://github.com/apache/doris/pull/68366#discussion_r4068400866
##########
be/src/exec/sink/writer/vtablet_writer_v2.cpp:
##########
@@ -563,6 +573,8 @@ Status
VTabletWriterV2::_write_memtable(std::shared_ptr<Block> block, int64_t ta
.write_file_cache = _write_file_cache,
.storage_vault_id {},
.enable_table_memtable_backpressure =
_tablet_finder->is_adaptive_random_bucket(),
+ .cloud_sink_upload = config::is_cloud_mode() &&
+
_state->query_options().enable_cloud_memtable_sink_upload,
Review Comment:
This enables the new `GET_WRITE_CONTEXT`/`ADD_PARTIAL_ROWSET` protocol
solely from a default-true query option. During a rolling upgrade, an old
target does not handle these opcodes, while an old source can still send
`APPEND_DATA` that the new `TabletStream` rejects after `_sink_upload` is set,
so a load spanning mixed-version BEs fails instead of falling back to
forwarding. Add capability negotiation/gating or retain a compatible forwarding
path.
##########
be/src/load/delta_writer/delta_writer_v2.cpp:
##########
@@ -133,14 +134,20 @@ Status DeltaWriterV2::init() {
context.memtable_on_sink_support_index_v2 = true;
context.encrypt_algorithm = EncryptionAlgorithmPB::PLAINTEXT;
- _rowset_writer = std::make_shared<BetaRowsetWriterV2>(_streams);
- RETURN_IF_ERROR(_rowset_writer->init(context));
+ if (_req.cloud_sink_upload) {
+ RETURN_IF_ERROR(_init_sink_upload_writer(context));
Review Comment:
When this option is enabled, every DeltaWriter immediately starts the new
sink-upload handshake. During a rolling upgrade a new source can select an old
target BE: that target does not recognize `GET_WRITE_CONTEXT` (opcode 5), falls
through its default dispatch path without a response, and the source waits
until timeout. Add capability negotiation/fallback to forwarding, or gate this
option until all target BEs support the new stream protocol.
##########
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:
`PackedFileWriter::is_in_packed_file()` remains true for an empty buffered
file, while `get_packed_slice_location()` returns an empty location when no
slice handle exists. Empty inverted-index files are explicitly allowed in
`close_writer`, so this assertion can abort a valid load; preserve the
empty-location skip before adding the mapping.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/load/NereidsStreamLoadTask.java:
##########
@@ -90,6 +90,7 @@ public class NereidsStreamLoadTask implements
NereidsLoadTaskInfo {
private boolean enableProfile = false;
private boolean memtableOnSinkNode = false;
+ private boolean cloudMemtableSinkUpload = true;
Review Comment:
`GroupCommitPlanner` constructs a `TStreamLoadPutRequest` without this field
and calls `fromTStreamLoadPutRequest`, so this `true` initializer bypasses
`Config.cloud_stream_load_memtable_sink_upload` for group-commit internal
loads. With the documented config set to false, group commit still enables sink
upload; set this path from `Config` while keeping ordinary Stream Load's
header/default behavior separate.
--
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]
