gavinchou commented on code in PR #68365:
URL: https://github.com/apache/doris/pull/68365#discussion_r4232298879
##########
be/src/exec/pipeline/pipeline_fragment_context.cpp:
##########
@@ -1305,7 +1305,7 @@ Status
PipelineFragmentContext::_create_data_sink(ObjectPool* pool, const TDataS
int child_node_id = pipeline->operators().back()->node_id();
if (state->query_options().enable_memtable_on_sink_node &&
Review Comment:
[P1] Please add a rolling-upgrade capability fence before selecting the
cloud V2 sink. enable_memtable_on_sink_node defaults to true, so a newly
upgraded sender immediately opens load streams to tablet targets. An older
cloud receiver still constructs LoadStreamWriter with
storage_engine().to_local(), whose CHECK_EQ(Type::LOCAL) aborts the BE process.
The optional request fields are wire-compatible, but they do not negotiate
receiver support. Please make one protocol decision per load based on the
minimum target-BE capability, fall back the whole load to V1 when necessary,
and add mixed-version and rollback coverage.
##########
be/src/load/channel/load_stream.cpp:
##########
@@ -375,7 +387,7 @@ Status IndexStream::append_data(const PStreamHeader&
header, butil::IOBuf* data)
std::lock_guard lock_guard(_lock);
auto it = _tablet_streams_map.find(tablet_id);
if (it == _tablet_streams_map.end()) {
- _init_tablet_stream(tablet_stream, tablet_id,
header.partition_id());
+ _init_tablet_stream(tablet_stream, tablet_id,
header.partition_id(), false);
Review Comment:
[P1] Please avoid initializing the cloud tablet while holding
IndexStream::_lock. _init_tablet_stream() reaches CloudRowsetBuilder::init(),
which may synchronize rowsets or storage-vault state and performs a
prepare_rowset Meta Service RPC for every non-empty tablet. One slow RPC
therefore head-of-line blocks first-touch initialization for every other tablet
in this index. The close path is now bounded-parallel, but initialization is
still serialized. Please install only a per-tablet once/future state under the
lock, perform initialization outside it with bounded concurrency, and add a
test with delayed prepare_rowset showing that unrelated tablets continue to
make progress.
--
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]
