-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable and disable the examplers storage with flag #6216
Conversation
795b7f0
to
b785a9e
Compare
5615db5
to
fdb4ced
Compare
a9bd330
to
f9f23b3
Compare
Thanks for this @hiteshwani29, but wonder in line with comment #5789 (comment), do we necessarily need a new flag? |
Yes @matej-g even if we pass If we check this tsdb code https://github.com/prometheus/prometheus/blob/main/tsdb/head_append.go#L56 we can figure out that it enables exempler storage |
f9f23b3
to
691c686
Compare
I concur with @matej-g. It looks like setting |
Workaround you suggested will not work. With EnableExemplarStorage: false
will always disable exemplar storage. we can't able to enable or disable it
by only passing the values to the max-exemplars.
We need this flag set to enable or disable exemplar storage.
…On Thu, 16 Mar 2023 at 16:02, Giedrius Statkevičius < ***@***.***> wrote:
Yes @matej-g <https://github.com/matej-g> even if we pass
--tsdb.max-exemplars=0 it will still enables exempler storage because we
are hardcoding this EnableExemplarStorage to true.
If we check this tsdb code
https://github.com/prometheus/prometheus/blob/main/tsdb/head_append.go#L56
and
https://github.com/prometheus/prometheus/blob/main/tsdb/head_append.go#L144
we can figure out that it enables exempler storage if max-exemplers is <=
0 or EnableExemplarStorage is true Right now this EnableExemplarStorage
is always true so it enables exempler storage even if we pass max-exempler
value <=0
I concur with @matej-g <https://github.com/matej-g>. That sounds like a
bug in Prometheus logic. Either way, then we could workaround this by
setting EnableExemplarStorage: false and that's it? Then we won't need
another flag?
—
Reply to this email directly, view it on GitHub
<#6216 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALGGVYQFE574OZKIPKEUL43W4LT25ANCNFSM6AAAAAAV3USYFQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
See my edited message 😄 @hiteshwani29 |
Yeah 😄 @GiedriusS |
@GiedriusS @matej-g @yeya24 Any update on this PR? |
691c686
to
1a2e1b6
Compare
ec50585
to
af2fcc1
Compare
14fc663
to
dbc73d2
Compare
Did the suggested changes @GiedriusS |
19b95f1
to
26e520d
Compare
Signed-off-by: hiteshwani29 <hiteshwani29@gmail.com>
26e520d
to
0c2e7f7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎖️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks 👍
PR closes #5789
Changes
Added
--tsdb.enable-exemplar-storage
flag to enable examples storage. For enabling examples storage we should enable thistsdb.enable-exemplar-storage
flag and set value oftsdb.enable-exemplar-storage
> 0. To disable examples storage set value oftsdb.enable-exemplar-storage
<=0 and disabletsdb.enable-exemplar-storage
flag.Verification
Tested locally