Prevent read on TSDB once closeAllTSDB function has been called#4304
Prevent read on TSDB once closeAllTSDB function has been called#4304pstibrany merged 15 commits intocortexproject:masterfrom
Conversation
pstibrany
left a comment
pstibrany
left a comment
There was a problem hiding this comment.
Thank you for working on this. I think we should just use ingester's state to reject queries. In practice difference between entering Stopping and calling closeAllTSDB is tiny.
bboreham
left a comment
bboreham
left a comment
There was a problem hiding this comment.
Would it be less repetitive to do the check in i.getTSDB() ?
I advocated against that. I think having it on read path is more explicit than returning |
Sorry, I missed that discussion. Did you talk about returning an error? |
I can change it to return an error from |
My opinion is that simple "getter" should simply return element from map, and not check for internal state of the ingester. This state only affects read-path, where it should be checked. |
Not sure I get this part. We wouldn't want to reopen a TSDB for write, would we? |
Right, we wouldn't. What I was trying to say is to add checks to read methods for this state – because that is where it will be more explicit, instead of getter. On write path, we could either use |
Signed-off-by: ilangofman <igofman99@gmail.com>
Signed-off-by: ilangofman <igofman99@gmail.com>
Signed-off-by: ilangofman <igofman99@gmail.com>
Signed-off-by: ilangofman <igofman99@gmail.com>
Signed-off-by: ilangofman <igofman99@gmail.com>
Signed-off-by: ilangofman <igofman99@gmail.com>
Signed-off-by: ilangofman <igofman99@gmail.com>
ilangofman
force-pushed
the
fix-ingester-tsdb-close-bug
branch
from
27d9865 to
0267a21
Compare
Signed-off-by: ilangofman <igofman99@gmail.com>
Signed-off-by: ilangofman <igofman99@gmail.com>
Signed-off-by: ilangofman <igofman99@gmail.com>
ilangofman
force-pushed
the
fix-ingester-tsdb-close-bug
branch
from
18387bf to
3c4798b
Compare
Signed-off-by: ilangofman <igofman99@gmail.com>
Signed-off-by: ilangofman <igofman99@gmail.com>
bboreham
left a comment
bboreham
left a comment
There was a problem hiding this comment.
Looking good now, but I think we can get away without the duplication in Push().
Signed-off-by: ilangofman <igofman99@gmail.com>
treid314
left a comment
treid314
left a comment
There was a problem hiding this comment.
Looks great! Thank you!
…exproject#4304) * Prevent read on TSDB once closeAllTSDB function has been called Signed-off-by: ilangofman <igofman99@gmail.com> * Fix formatting issues Signed-off-by: ilangofman <igofman99@gmail.com> * Address PR comments and remove unit test no longer required Signed-off-by: ilangofman <igofman99@gmail.com> * Remove closed bool no longer used Signed-off-by: ilangofman <igofman99@gmail.com> * Remove error no longer used Signed-off-by: ilangofman <igofman99@gmail.com> * Remove comment and change return var Signed-off-by: ilangofman <igofman99@gmail.com> * Update log message to debug Signed-off-by: ilangofman <igofman99@gmail.com> * Moved log message to separate function Signed-off-by: ilangofman <igofman99@gmail.com> * change function name for checking if tsdb is closing Signed-off-by: ilangofman <igofman99@gmail.com> * ingester should read/write only when state is running for block store Signed-off-by: ilangofman <igofman99@gmail.com> * Move running check to ingester v2 file Signed-off-by: ilangofman <igofman99@gmail.com> * Remove extra space Signed-off-by: ilangofman <igofman99@gmail.com> * Remove duplication from push func Signed-off-by: ilangofman <igofman99@gmail.com> Signed-off-by: Alvin Lin <alvinlin@amazon.com>
5 participants
What this PR does:
Added another variable in the
TSDBStatethat prevents any read occurring on the TSDB once the ingester has called thecloseAllTSDBfunction.Which issue(s) this PR fixes:
Fixes #3350
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]