github-actions[bot] commented on code in PR #69682:
URL: https://github.com/apache/doris/pull/69682#discussion_r4412699447
##########
be/src/core/data_type/data_type.h:
##########
@@ -155,6 +155,12 @@ class IDataType : private boost::noncopyable {
virtual char* serialize(const IColumn& column, char* buf, int
be_exec_version) const = 0;
virtual const char* deserialize(const char* buf, MutableColumnPtr* column,
int be_exec_version) const = 0;
+ // Variable-length types can override this overload to validate embedded
lengths
+ // against end. Fixed-size types can use the default forwarding.
+ virtual const char* deserialize(const char* buf, const char* end,
MutableColumnPtr* column,
+ int be_exec_version) const {
Review Comment:
[P1] Propagate the end bound through nested datatype deserializers
`Block::deserialize` now passes `buf_end`, but the default overload added
here immediately forwards to the old three-argument virtual. `DataTypeArray`,
`DataTypeMap`, and `DataTypeStruct` still use that old overload for their
children, so `Array<HLL>` (whose element is wrapped in `Nullable(HLL)`) reaches
`DataTypeHLL` with `end == nullptr`. A malformed current-format payload can
declare an HLL length of 16385 and provide only the `HLL_DATA_FULL` tag; the
child validator then reads past the PBlock before Block's final cursor check.
Add end-aware overrides for the nested wrappers, validate their headers/offset
byte ranges, and pass `end` to every child; add truncated HLL coverage for both
wire layouts.
##########
be/src/exec/exchange/vdata_stream_recvr.cpp:
##########
@@ -68,18 +68,18 @@ VDataStreamRecvr::SenderQueue::~SenderQueue() {
}
Status VDataStreamRecvr::SenderQueue::get_batch(Block* block, bool* eos) {
-#ifndef NDEBUG
- if (!_is_cancelled && _block_queue.empty() && _num_remaining_senders > 0) {
- throw doris::Exception(ErrorCode::INTERNAL_ERROR,
- "_is_cancelled: {}, _block_queue_empty: {}, "
- "_num_remaining_senders: {},
_debug_string_info: {}",
- _is_cancelled, _block_queue.empty(),
_num_remaining_senders,
- _debug_string_info());
- }
-#endif
BlockItem block_item;
{
INJECT_MOCK_SLEEP(std::lock_guard<std::mutex> l(_lock));
+#ifndef NDEBUG
+ if (!_is_cancelled && _block_queue.empty() && _num_remaining_senders >
0) {
+ throw doris::Exception(ErrorCode::INTERNAL_ERROR,
+ "_is_cancelled: {}, _block_queue_empty: {},
"
+ "_num_remaining_senders: {},
_debug_string_info: {}",
+ _is_cancelled, _block_queue.empty(),
_num_remaining_senders,
+ _debug_string_info());
+ }
+#endif
//check and get block_item from data_queue
if (_is_cancelled) {
Review Comment:
[P2] Release accounting when close drops buffered blocks
The new `Defer` balances the block removed by `get_batch()`, but
`SenderQueue::close()` can still clear additional buffered `BlockItem`s without
calling `sub_blocks_memory_usage()` or decrementing `_memory_used_counter`.
Both remote and local `add_block()` increment those counters, and
`VDataStreamRecvr::close()` only marks dependencies ready before clearing the
queue. Closing a receiver with queued blocks therefore leaves its
queue/receiver MemTrackers and profile bytes permanently inflated. Sum the
queued `block_byte_size()` values while detaching them, then perform the
matching releases exactly once during teardown.
--
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]
