stdpain commented on a change in pull request #5491:
URL: https://github.com/apache/incubator-doris/pull/5491#discussion_r592426836
##########
File path: be/src/runtime/tablets_channel.cpp
##########
@@ -260,7 +260,6 @@ Status TabletsChannel::cancel() {
for (auto& it : _tablet_writers) {
it.second->cancel();
}
- DCHECK_EQ(_mem_tracker->consumption(), 0);
Review comment:
We can't guarantee that by the time we get here,
TabletsChannel::add_batch have been executed, TabletsChannel::add_batch's
RowBatch may not have been destroyed and tracker's consumption may greater
than zero
##########
File path: be/src/olap/aggregate_func.h
##########
@@ -461,10 +461,9 @@ struct
AggregateFuncTraits<OLAP_FIELD_AGGREGATION_HLL_UNION, OLAP_FIELD_TYPE_HLL
// we use zero size represent this slice is a agg object
dst_slice->size = 0;
auto* hll = new HyperLogLog(*src_slice);
+
dst_slice->data = reinterpret_cast<char*>(hll);
- mem_pool->mem_tracker()->Consume(sizeof(HyperLogLog));
Review comment:
we don't have to record 4 bytes in the tracker，if we want to tracker it,
If we want to record it, we need a wrapper and release it at the time of
deconstruction. This process requires additional memory overhead and lock
application. I don't think it's worth doing it.
```c++
auto* hll = new HyperLogLog(*src_slice);
dst_slice->data = reinterpret_cast<char*>(hll);
mem_pool->mem_tracker()->Consume(sizeof(HyperLogLog));
auto defer = new Defer([]() {
mem_pool->mem_tracker()->Release();
});
agg_pool->add(defer);
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
