LindaSummer commented on code in PR #2878:
URL: https://github.com/apache/kvrocks/pull/2878#discussion_r2045841999
##########
src/types/redis_tdigest.cc:
##########
@@ -332,10 +332,13 @@ std::string TDigest::internalBufferKey(const std::string&
ns_key, const TDigestM
}
std::string TDigest::internalKeyFromCentroid(const std::string& ns_key, const
TDigestMetadata& metadata,
- const Centroid& centroid) const {
+ const Centroid& centroid,
uint32_t seq) const {
std::string sub_key;
PutFixed8(&sub_key, static_cast<uint8_t>(SegmentType::kCentroids));
PutDouble(&sub_key, centroid.mean); // It uses EncodeDoubleToUInt64 and
keeps original order of double
+ // The tdigest centroids only cares about the weight rather than the mean,
so different centroids may have same mean,
+ // we should keep them with same original order, this seq id could be
discarded in decode
+ PutFixed32(&sub_key, seq);
Review Comment:
Hi @mapleFU ,
Thanks very much for your kind review!😊
I thought about the size when adding this seq-id.
The compression is uint32 now, so the limitation of centroids is uint32
maximum.
So I decided to use uint32 for safety.
Best Regards,
Edward
--
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]
