github-actions[bot] commented on code in PR #68498:
URL: https://github.com/apache/doris/pull/68498#discussion_r4127083935
##########
be/src/core/value/quantile_state.cpp:
##########
@@ -28,7 +30,50 @@
#include "util/tdigest.h"
#include "util/unaligned.h"
+#ifdef BE_TEST
+#include "cpp/sync_point.h"
+#endif
+
namespace doris {
+
+// Shares a digest across QuantileState copies and detaches it before sample
+// writes. Readers use shared locks; compression uses an exclusive lock.
+struct QuantileState::TDigestHolder {
+ explicit TDigestHolder(float compression) : digest(compression) {}
+ TDigestHolder(const TDigestHolder& other) : digest(other.digest) {}
+
+ std::shared_lock<std::shared_mutex> lock_processed_digest() {
+ std::shared_lock read_lock(mutex);
+ if (digest.have_unprocessed()) {
+ read_lock.unlock();
+ {
+ std::unique_lock write_lock(mutex);
+ if (digest.have_unprocessed()) {
+ digest.compress();
+ }
+ }
+ read_lock.lock();
+ }
+ return read_lock;
+ }
+
+ TDigest digest;
+ std::shared_mutex mutex;
+};
+
+TDigest& QuantileState::_mutable_tdigest() {
+ std::shared_ptr<TDigestHolder> detached;
+ {
+ std::unique_lock lock(_tdigest_ptr->mutex);
+ if (_tdigest_ptr.use_count() == 1) {
+ return _tdigest_ptr->digest;
+ }
+ detached = std::make_shared<TDigestHolder>(*_tdigest_ptr);
Review Comment:
For a growing `quantile_union(q)` window, the analytic sink copies the
accumulator into the result column after every row; the next add detaches here,
leaving the prior result's holder alive. Both the initial TDigest and each
detached copy reserve `8 * compression + 1` unprocessed centroids, so at
compression 10000, 1000 retained results reserve about 610 MiB just for these
buffers. `quantile_percent` later empties them without releasing capacity, and
`ColumnQuantileState::allocated_bytes()` misses it. Please compact saved
results or separate their storage from the live accumulator, and cover a long
high-compression window. This is distinct from the earlier snapshot-cache
comment: that cache no longer exists, and fixing it would not release these
analytic result holders.
--
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]
