Skip to content

Commit 3c4798b

Browse files
ilangofmanilangofman
committed
ingester should read/write only when state is running for block store
Signed-off-by: ilangofman <igofman99@gmail.com>
1 parent feb4f42 commit 3c4798b

File tree

3 files changed

+72
-76
lines changed

3 files changed

+72
-76
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
## master / unreleased
44

55
* [CHANGE] Querier / ruler: Change `-querier.max-fetched-chunks-per-query` configuration to limit to maximum number of chunks that can be fetched in a single query. The number of chunks fetched by ingesters AND long-term storare combined should not exceed the value configured on `-querier.max-fetched-chunks-per-query`. #4260
6-
* [BUGFIX] Ingester: Prevent any reads on TSDBs when the ingester is stopping. #4304
6+
* [BUGFIX] Ingester: When using block storage, prevent any reads or writes while the ingester is stopping. This will prevent accessing TSDB blocks once they have been already closed. #4304
77

88
## 1.10.0-rc.0 / 2021-06-28
99

‎pkg/ingester/ingester.go‎

Lines changed: 67 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -480,10 +480,26 @@ func (i *Ingester) checkRunningOrStopping() error {
480480
return status.Error(codes.Unavailable, s.String())
481481
}
482482

483+
// Using block store, the ingester is only available when it is in a Running state. The ingester is not available
484+
// when stopping to prevent any read or writes to the TSDB after the ingester has closed them.
485+
func (i *Ingester) checkRunning() error {
486+
s := i.State()
487+
if s == services.Running {
488+
return nil
489+
}
490+
return status.Error(codes.Unavailable, s.String())
491+
}
492+
483493
// Push implements client.IngesterServer
484494
func (i *Ingester) Push(ctx context.Context, req *cortexpb.WriteRequest) (*cortexpb.WriteResponse, error) {
485-
if err := i.checkRunningOrStopping(); err != nil {
486-
return nil, err
495+
if i.cfg.BlocksStorageEnabled {
496+
if err := i.checkRunning(); err != nil {
497+
return nil, err
498+
}
499+
} else {
500+
if err := i.checkRunningOrStopping(); err != nil {
501+
return nil, err
502+
}
487503
}
488504

489505
// We will report *this* request in the error too.
@@ -762,14 +778,17 @@ func (i *Ingester) purgeUserMetricsMetadata() {
762778

763779
// Query implements service.IngesterServer
764780
func (i *Ingester) Query(ctx context.Context, req *client.QueryRequest) (*client.QueryResponse, error) {
765-
if err := i.checkRunningOrStopping(); err != nil {
766-
return nil, err
767-
}
768-
769781
if i.cfg.BlocksStorageEnabled {
782+
if err := i.checkRunning(); err != nil {
783+
return nil, err
784+
}
770785
return i.v2Query(ctx, req)
771786
}
772787

788+
if err := i.checkRunningOrStopping(); err != nil {
789+
return nil, err
790+
}
791+
773792
userID, err := tenant.TenantID(ctx)
774793
if err != nil {
775794
return nil, err
@@ -829,14 +848,17 @@ func (i *Ingester) Query(ctx context.Context, req *client.QueryRequest) (*client
829848

830849
// QueryStream implements service.IngesterServer
831850
func (i *Ingester) QueryStream(req *client.QueryRequest, stream client.Ingester_QueryStreamServer) error {
832-
if err := i.checkRunningOrStopping(); err != nil {
833-
return err
834-
}
835-
836851
if i.cfg.BlocksStorageEnabled {
852+
if err := i.checkRunning(); err != nil {
853+
return err
854+
}
837855
return i.v2QueryStream(req, stream)
838856
}
839857

858+
if err := i.checkRunningOrStopping(); err != nil {
859+
return err
860+
}
861+
840862
spanLog, ctx := spanlogger.New(stream.Context(), "QueryStream")
841863
defer spanLog.Finish()
842864

@@ -926,14 +948,17 @@ func (i *Ingester) QueryExemplars(ctx context.Context, req *client.ExemplarQuery
926948

927949
// LabelValues returns all label values that are associated with a given label name.
928950
func (i *Ingester) LabelValues(ctx context.Context, req *client.LabelValuesRequest) (*client.LabelValuesResponse, error) {
929-
if err := i.checkRunningOrStopping(); err != nil {
930-
return nil, err
931-
}
932-
933951
if i.cfg.BlocksStorageEnabled {
952+
if err := i.checkRunning(); err != nil {
953+
return nil, err
954+
}
934955
return i.v2LabelValues(ctx, req)
935956
}
936957

958+
if err := i.checkRunningOrStopping(); err != nil {
959+
return nil, err
960+
}
961+
937962
i.userStatesMtx.RLock()
938963
defer i.userStatesMtx.RUnlock()
939964
state, ok, err := i.userStates.getViaContext(ctx)
@@ -951,14 +976,17 @@ func (i *Ingester) LabelValues(ctx context.Context, req *client.LabelValuesReque
951976

952977
// LabelNames return all the label names.
953978
func (i *Ingester) LabelNames(ctx context.Context, req *client.LabelNamesRequest) (*client.LabelNamesResponse, error) {
954-
if err := i.checkRunningOrStopping(); err != nil {
955-
return nil, err
956-
}
957-
958979
if i.cfg.BlocksStorageEnabled {
980+
if err := i.checkRunning(); err != nil {
981+
return nil, err
982+
}
959983
return i.v2LabelNames(ctx, req)
960984
}
961985

986+
if err := i.checkRunningOrStopping(); err != nil {
987+
return nil, err
988+
}
989+
962990
i.userStatesMtx.RLock()
963991
defer i.userStatesMtx.RUnlock()
964992
state, ok, err := i.userStates.getViaContext(ctx)
@@ -976,14 +1004,17 @@ func (i *Ingester) LabelNames(ctx context.Context, req *client.LabelNamesRequest
9761004

9771005
// MetricsForLabelMatchers returns all the metrics which match a set of matchers.
9781006
func (i *Ingester) MetricsForLabelMatchers(ctx context.Context, req *client.MetricsForLabelMatchersRequest) (*client.MetricsForLabelMatchersResponse, error) {
979-
if err := i.checkRunningOrStopping(); err != nil {
980-
return nil, err
981-
}
982-
9831007
if i.cfg.BlocksStorageEnabled {
1008+
if err := i.checkRunning(); err != nil {
1009+
return nil, err
1010+
}
9841011
return i.v2MetricsForLabelMatchers(ctx, req)
9851012
}
9861013

1014+
if err := i.checkRunningOrStopping(); err != nil {
1015+
return nil, err
1016+
}
1017+
9871018
i.userStatesMtx.RLock()
9881019
defer i.userStatesMtx.RUnlock()
9891020
state, ok, err := i.userStates.getViaContext(ctx)
@@ -1046,14 +1077,17 @@ func (i *Ingester) MetricsMetadata(ctx context.Context, req *client.MetricsMetad
10461077

10471078
// UserStats returns ingestion statistics for the current user.
10481079
func (i *Ingester) UserStats(ctx context.Context, req *client.UserStatsRequest) (*client.UserStatsResponse, error) {
1049-
if err := i.checkRunningOrStopping(); err != nil {
1050-
return nil, err
1051-
}
1052-
10531080
if i.cfg.BlocksStorageEnabled {
1081+
if err := i.checkRunning(); err != nil {
1082+
return nil, err
1083+
}
10541084
return i.v2UserStats(ctx, req)
10551085
}
10561086

1087+
if err := i.checkRunningOrStopping(); err != nil {
1088+
return nil, err
1089+
}
1090+
10571091
i.userStatesMtx.RLock()
10581092
defer i.userStatesMtx.RUnlock()
10591093
state, ok, err := i.userStates.getViaContext(ctx)
@@ -1075,14 +1109,17 @@ func (i *Ingester) UserStats(ctx context.Context, req *client.UserStatsRequest)
10751109

10761110
// AllUserStats returns ingestion statistics for all users known to this ingester.
10771111
func (i *Ingester) AllUserStats(ctx context.Context, req *client.UserStatsRequest) (*client.UsersStatsResponse, error) {
1078-
if err := i.checkRunningOrStopping(); err != nil {
1079-
return nil, err
1080-
}
1081-
10821112
if i.cfg.BlocksStorageEnabled {
1113+
if err := i.checkRunning(); err != nil {
1114+
return nil, err
1115+
}
10831116
return i.v2AllUserStats(ctx, req)
10841117
}
10851118

1119+
if err := i.checkRunningOrStopping(); err != nil {
1120+
return nil, err
1121+
}
1122+
10861123
i.userStatesMtx.RLock()
10871124
defer i.userStatesMtx.RUnlock()
10881125
users := i.userStates.cp()

‎pkg/ingester/ingester_v2.go‎

Lines changed: 4 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -992,10 +992,6 @@ func (i *Ingester) v2Query(ctx context.Context, req *client.QueryRequest) (*clie
992992

993993
i.metrics.queries.Inc()
994994

995-
if i.checkIfAllTSDBClosing() {
996-
return &client.QueryResponse{}, nil
997-
}
998-
999995
db := i.getTSDB(userID)
1000996
if db == nil {
1001997
return &client.QueryResponse{}, nil
@@ -1052,10 +1048,6 @@ func (i *Ingester) v2QueryExemplars(ctx context.Context, req *client.ExemplarQue
10521048

10531049
i.metrics.queries.Inc()
10541050

1055-
if i.checkIfAllTSDBClosing() {
1056-
return &client.ExemplarQueryResponse{}, nil
1057-
}
1058-
10591051
db := i.getTSDB(userID)
10601052
if db == nil {
10611053
return &client.ExemplarQueryResponse{}, nil
@@ -1101,10 +1093,6 @@ func (i *Ingester) v2LabelValues(ctx context.Context, req *client.LabelValuesReq
11011093
return nil, err
11021094
}
11031095

1104-
if i.checkIfAllTSDBClosing() {
1105-
return &client.LabelValuesResponse{}, nil
1106-
}
1107-
11081096
db := i.getTSDB(userID)
11091097
if db == nil {
11101098
return &client.LabelValuesResponse{}, nil
@@ -1137,10 +1125,6 @@ func (i *Ingester) v2LabelNames(ctx context.Context, req *client.LabelNamesReque
11371125
return nil, err
11381126
}
11391127

1140-
if i.checkIfAllTSDBClosing() {
1141-
return &client.LabelNamesResponse{}, nil
1142-
}
1143-
11441128
db := i.getTSDB(userID)
11451129
if db == nil {
11461130
return &client.LabelNamesResponse{}, nil
@@ -1173,10 +1157,6 @@ func (i *Ingester) v2MetricsForLabelMatchers(ctx context.Context, req *client.Me
11731157
return nil, err
11741158
}
11751159

1176-
if i.checkIfAllTSDBClosing() {
1177-
return &client.MetricsForLabelMatchersResponse{}, nil
1178-
}
1179-
11801160
db := i.getTSDB(userID)
11811161
if db == nil {
11821162
return &client.MetricsForLabelMatchersResponse{}, nil
@@ -1244,10 +1224,6 @@ func (i *Ingester) v2UserStats(ctx context.Context, req *client.UserStatsRequest
12441224
return nil, err
12451225
}
12461226

1247-
if i.checkIfAllTSDBClosing() {
1248-
return &client.UserStatsResponse{}, nil
1249-
}
1250-
12511227
db := i.getTSDB(userID)
12521228
if db == nil {
12531229
return &client.UserStatsResponse{}, nil
@@ -1260,10 +1236,6 @@ func (i *Ingester) v2AllUserStats(ctx context.Context, req *client.UserStatsRequ
12601236
i.userStatesMtx.RLock()
12611237
defer i.userStatesMtx.RUnlock()
12621238

1263-
if i.checkIfAllTSDBClosing() {
1264-
return &client.UsersStatsResponse{}, nil
1265-
}
1266-
12671239
users := i.TSDBState.dbs
12681240

12691241
response := &client.UsersStatsResponse{
@@ -1308,10 +1280,6 @@ func (i *Ingester) v2QueryStream(req *client.QueryRequest, stream client.Ingeste
13081280

13091281
i.metrics.queries.Inc()
13101282

1311-
if i.checkIfAllTSDBClosing() {
1312-
return nil
1313-
}
1314-
13151283
db := i.getTSDB(userID)
13161284
if db == nil {
13171285
return nil
@@ -1815,15 +1783,14 @@ func (i *Ingester) openExistingTSDB(ctx context.Context) error {
18151783

18161784
// getMemorySeriesMetric returns the total number of in-memory series across all open TSDBs.
18171785
func (i *Ingester) getMemorySeriesMetric() float64 {
1786+
if err := i.checkRunning(); err != nil {
1787+
return 0
1788+
}
1789+
18181790
i.userStatesMtx.RLock()
18191791
defer i.userStatesMtx.RUnlock()
18201792

18211793
count := uint64(0)
1822-
1823-
if i.checkIfAllTSDBClosing() {
1824-
return 0
1825-
}
1826-
18271794
for _, db := range i.TSDBState.dbs {
18281795
count += db.Head().NumSeries()
18291796
}
@@ -2282,11 +2249,3 @@ func (i *Ingester) getInstanceLimits() *InstanceLimits {
22822249

22832250
return l
22842251
}
2285-
2286-
func (i *Ingester) checkIfAllTSDBClosing() bool {
2287-
if i.State() == services.Stopping {
2288-
level.Debug(i.logger).Log("msg", "TSDB is unavailable, as the Ingester is in the process of stopping and closing all TSDB")
2289-
return true
2290-
}
2291-
return false
2292-
}

0 commit comments

Comments
 (0)