github-actions[bot] commented on code in PR #63574:
URL: https://github.com/apache/doris/pull/63574#discussion_r3295990330
##########
be/src/exec/operator/exchange_sink_buffer.cpp:
##########
@@ -264,46 +263,33 @@ Status ExchangeSinkBuffer::_send_rpc(RpcInstance&
instance_data) {
if (q_ptr && !q_ptr->empty()) {
auto& q = *q_ptr;
- std::vector<TransmitInfo> requests(_send_multi_blocks ? q.size() : 1);
+ std::vector<TransmitInfo> requests(_send_multi_blocks_byte_size > 0 ?
q.size() : 1);
for (int i = 0; i < requests.size(); i++) {
requests[i] = std::move(q.front());
q.pop();
if (requests[i].block) {
- // make sure rpc byte size under the
_send_multi_blocks_bytes_size
+ // make sure rpc byte size under the
_send_multi_blocks_byte_size
mem_byte += requests[i].block->ByteSizeLong();
- if (_send_multi_blocks && mem_byte >
_send_multi_blocks_byte_size) {
+ if (_send_multi_blocks_byte_size > 0 && mem_byte >
_send_multi_blocks_byte_size) {
requests.resize(i + 1);
break;
}
}
}
// If we have data to shuffle which is not broadcasted
- auto& request = requests[0];
auto& brpc_request = instance_data.request;
brpc_request->set_sender_id(channel->_parent->sender_id());
brpc_request->set_be_number(channel->_parent->be_number());
- if (_send_multi_blocks) {
- for (auto& req : requests) {
- if (req.block && !req.block->column_metas().empty()) {
- auto add_block = brpc_request->add_blocks();
- add_block->Swap(req.block.get());
- }
- }
- } else {
- if (request.block && !request.block->column_metas().empty()) {
- brpc_request->set_allocated_block(request.block.get());
+ for (auto& req : requests) {
+ if (req.block && !req.block->column_metas().empty()) {
+ auto add_block = brpc_request->add_blocks();
+ add_block->Swap(req.block.get());
Review Comment:
This drops the legacy `block` wire path and always sends exchange payloads
through `blocks`, even when `_send_multi_blocks_byte_size <= 0` and only one
block is sent. That breaks mixed-version BE compatibility:
`VDataStreamMgr::transmit_data()` still treats `request->has_block()` as the
old compatibility path, so an older receiver that does not handle field 13 will
ignore this payload and only see EOS, losing exchange rows during rolling
upgrade. Please keep using `set_allocated_block()` when multi-block exchange is
disabled, or gate the repeated `blocks` field on a version/compatibility check.
--
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]
