Skip to content

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 #126

@DrEsteban

Description

@DrEsteban

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: RedisLabs/memtier_benchmark#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

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions