apache / apache/kvrocks

Refactor TDigest metadata preparation and merging logic into a helper function

Open
#2,907 3 comments 1 reaction 0 assignees View on GitHub
enhancement
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.