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

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: redis/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

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions