Skip to content

labels: improve symmetry with stringlabels builds #12230

@npordash

Description

@npordash

Proposal

Hi!

I recently went through the effort to better align our usage of labels.Labels so that we can try out stringlabels builds and evaluate the change on tsdb, ingester, and querier components for an internal system we have.

There were a couple sharp edges I ran into trying to make things compatible with both stringlabels and !stringlabels builds that I wanted to bring to folks attention (I suppose mainly @bboreham ?) if they were not aware of them already.

  1. There is no Overwrite method on labels.ScratchBuilder for !stringlabels builds. I ended up having to write some intermediate struct for the time being to ensure we can do the equivalent for both build types. I noticed this isn't used anywhere in Prometheus, but was in Store series labels as a single string, to save memory grafana/mimir#3555 (I have similar needs with how it's being used in that PR).

  2. Both InternStrings and ReleaseStrings behave unexpectedly with stringlabels builds. Interning the internal representation of the labels here doesn't seem to make much sense (at least I can't think of a reason) compared to interning individual strings with the !stringlabels build.

    I know there is some prototyping going on about potentially using symbol tables as a form of interning for stringlabels builds. Depending on how that pans out I am wondering if it even makes sense to keep these two methods around?

    In places we've been doing interning already we now have separate build files so that it's only done when !stringlabels.

  3. I am kind of scared to call the Len method with a stringlabels build since it requires fully iterating and decoding the labels. If I need to properly size a map or slice before calling Range the penality seems too high.

    Since the label names and values are RLE maybe we can do the same for the total number of labels?

  4. Calls to methods like Get and Range might not return stable strings with a stringlabels build.

    There are certain code paths that heavily rely on the use of a single labels.Labels and calls to labels.ScratchLabels.Overwrite to minimize allocations. With a !stringlabels build the consuming end of the labels can safely retain references to strings returned by Get (assuming the labels were constructed with strings that won't mutate) where this is not true with a stringlabels build since a string slice of the underlying data structure is returned.

    This one was really tricky and difficult to reason about and for the time being I'm being very careful in defensively copying any string returned by labels methods that need to be retained.

    I understand a string isn't defensively copied for calls to things like Get and Range for performance reasons, but this was surprising behavior for sure. I am hoping that maybe this problem will go away if the use of symbol tables works out :)

I'm more than happy to file PRs to address any discrepancies, but I'm also not really sure what direction things are headed and maybe some of these issues will naturally go away with the on-going work @bboreham is doing.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions