`bool hdr_record_value(struct hdr_histogram* h, int64_t value)` can record out of bounds values and should be capped with min & max values
@filipecosta90 is already working on this.
Since Nov 12, 2024.
Assessment
This issue has not been assessed yet.
Description
RedisInc's memtier_benchmark suffered from some issues where very-large or very-small values were being written to the histogram and either causing nan/inf output or incorrect output in the benchmarking metrics.
A workaround was introduced in this PR: https://github.com/RedisLabs/memtier_benchmark/pull/273
They introduced a (spiritual) overload, hdr_record_value_capped(), with impl:
// hdr_histogram.c
bool hdr_record_value_capped(struct hdr_histogram* h, int64_t value)
{
int64_t capped_value = (value > h->highest_trackable_value) ? h->highest_trackable_value : value;
capped_value = (capped_value < h->lowest_trackable_value) ? h->lowest_trackable_value : capped_value;
return hdr_record_value(h, capped_value);
}
But it seems as if this change should be made directly to this library. Either in a similar way, or by returning an error to the caller. OR, at the very least, this strange behavior/limitation called out in the method docs for hdr_record_value(). E.g. "This method does not properly handle values that are outside the trackable range of the histogram. Please very inputs before calling."
@filipecosta90
- Dominant language
- C
- Stars
- 290
- Forks
- 111
- Avg merge
- 1d 16h
- Merged PRs (30d)
- 3
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from HdrHistogram/HdrHistogram_c
-
Difficulty 3/5 1-2 days Newbie friendliness 74/100
HdrHistogram/HdrHistogram_c#132 · 1 comment ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 45/100
HdrHistogram/HdrHistogram_c#125 ·
-
gcc 12.2 misoptimizes the code with `ipa-ra` when linking hdr_histogram_static into an `.so` file Open
Difficulty 4/5 3-5 days Newbie friendliness 20/100
HdrHistogram/HdrHistogram_c#124 ·
-
Difficulty 3/5 1-2 days Newbie friendliness 48/100
HdrHistogram/HdrHistogram_c#118 ·
-
Difficulty 3/5 1-2 days Newbie friendliness 35/100
HdrHistogram/HdrHistogram_c#116 · 1 comment ·
All issues in HdrHistogram/HdrHistogram_c
Similar issues
-
[adam] AdamNet network read doesn't cap to MAX_ADAM_PACKET_LEN, overflows client receive buffers Open
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
FujiNetWIFI/fujinet-firmware#1649 · 2 comments ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
HarbourMasters/Shipwright#7229 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
riscv-software-src/riscv-isa-sim#2435 · 1 comment ·
-
bug Self Built Image SNAPSHOT Supported Device target/ramips
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 76/100