Refactor TDigest metadata preparation and merging logic into a helper function
- Dominant language
- C++
- Stars
- 4.4k
- Forks
- 658
- Avg merge
- 1d 20h
- Merged PRs (30d)
- 10
Description
### Search before asking
- [x] I had searched in the [issues](https://github.com/apache/kvrocks/issues) and found no similar issues.
### Motivation
Currently, in the functions below,
- [TDigest::CDF](https://github.com/apache/kvrocks/pull/2887)
- [TDigest::Quantile](https://github.com/apache/kvrocks/blob/36a5776a438334629029ef8e22fb414c1f9bbba5/src/types/redis_tdigest.cc#L190C1-L223C1)
we repeat the same block of code to:
- Append namespace prefix to the key
- Acquire a lock
- Fetch TDigest metadata
- Merge unmerged nodes (if any)
- Update metadata and write it back
- Refresh the snapshot
As new TDigest features are added, the same preprocessing steps (metadata preparation and merging) will likely be needed repeatedly. And this will most likely cause code duplication of around 20-30 lines across multiple functions.
If this change is accepted then the dependent PR (#2887) is based on this refactor, and cannot be merged without it.
### Solution
This can be avoided by using a helper function to do this work and keep things simple.
### Are you willing to submit a PR?
- [x] I'm willing to submit a PR!
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in src/types/redis_tdigest.cc at TDigest::CDF and TDigest::Quantile, and compare their repeated metadata preparation and merging steps. Refactor that shared work into a helper, then confirm both entry points use it while preserving metadata updates and snapshot refreshes; PR #2887 depends on this change.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- databases
- Issue type
- Refactor
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100