github-actions[bot] commented on code in PR #63258:
URL: https://github.com/apache/doris/pull/63258#discussion_r3242103021
##########
be/src/util/proto_util.h:
##########
@@ -39,9 +40,24 @@ constexpr size_t MIN_HTTP_BRPC_SIZE = (1ULL << 31);
// Embed column_values and brpc request serialization string in controller
attachment.
template <typename Params, typename Closure>
Status request_embed_attachment_contain_blockv2(Params* brpc_request,
- std::unique_ptr<Closure>&
closure) {
- std::string column_values =
std::move(*brpc_request->mutable_block()->mutable_column_values());
- brpc_request->mutable_block()->mutable_column_values()->clear();
+ std::unique_ptr<Closure>&
closure,
+ bool restore_column_values =
false) {
+ auto* block = brpc_request->mutable_block();
+ if (restore_column_values) {
+ // Some callers borrow block storage from a shared owner. Temporarily
detach the large
+ // column_values field so the serialized request stays small, then
restore it before
+ // returning so the real owner can still be reused by later sends.
+ auto* column_values = block->release_column_values();
+ DORIS_CHECK(column_values != nullptr);
Review Comment:
This restore mode mutates the borrowed `PBlock` by calling
`release_column_values()`, but broadcast blocks are shared across remote
channels and can be processed concurrently by different `RpcInstance` callback
threads. A concrete path is: an unpartitioned exchange enqueues the same
`BroadcastPBlockHolder` to multiple remote channels while those destinations
already have in-flight RPCs; when those prior RPCs complete around the same
time, each callback calls `_send_rpc()` for its destination and reaches this
HTTP helper for the same holder. The first thread temporarily removes
`column_values`; the second thread races on the same protobuf and can see
`nullptr` here (hitting `DORIS_CHECK`) or otherwise race with the restore.
Please avoid mutating the shared `PBlock` during attachment embedding, or add
synchronization that covers all users of the shared holder.
--
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]
