Skip to content

Commit

Permalink
fix: MeasurementsCardinality should not be less than 0 (influxdata#23286
Browse files Browse the repository at this point in the history
)

Clamp the value of Store.MeasurementsCardinality so that it can not be less
than 0. This primarily shows up as a negative `numMeasurements` value in
/debug/vars under some circumstances.

refs influxdata#23285
  • Loading branch information
gwossum authored and chengshiwen committed Aug 11, 2024
1 parent 199422b commit 8c84482
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 1 deletion.
6 changes: 5 additions & 1 deletion tsdb/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -1198,7 +1198,11 @@ func (s *Store) MeasurementsCardinality(ctx context.Context, database string) (i
if err != nil {
return 0, err
}
return int64(ss.Count() - ts.Count()), nil
mc := int64(ss.Count() - ts.Count())
if mc < 0 {
mc = 0
}
return mc, nil
}

// MeasurementsSketches returns the sketches associated with the measurement
Expand Down
40 changes: 40 additions & 0 deletions tsdb/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1428,6 +1428,21 @@ func TestStore_Sketches(t *testing.T) {
if got, exp := int(tsketch.Count()), tmeasurements; got-exp < -delta(tmeasurements) || got-exp > delta(tmeasurements) {
return fmt.Errorf("got measurement tombstone cardinality %d, expected ~%d", got, exp)
}

if mc, err := store.MeasurementsCardinality(context.Background(), "db"); err != nil {
return fmt.Errorf("unexpected error from MeasurementsCardinality: %w", err)
} else {
if mc < 0 {
return fmt.Errorf("MeasurementsCardinality returned < 0 (%v)", mc)
}
expMc := int64(sketch.Count() - tsketch.Count())
if expMc < 0 {
expMc = 0
}
if got, exp := int(mc), int(expMc); got-exp < -delta(exp) || got-exp > delta(exp) {
return fmt.Errorf("got measurement cardinality %d, expected ~%d", mc, exp)
}
}
return nil
}

Expand Down Expand Up @@ -1507,6 +1522,31 @@ func TestStore_Sketches(t *testing.T) {
if err := checkCardinalities(store.Store, expS, expTS, expM, expTM); err != nil {
return fmt.Errorf("[initial|re-open|delete|re-open] %v", err)
}

// Now delete the rest of the measurements.
// This will cause the measurement tombstones to exceed the measurement cardinality for TSI.
mnames, err = store.MeasurementNames(context.Background(), nil, "db", "", nil)
if err != nil {
return err
}

for _, name := range mnames {
if err := store.DeleteSeries("db", []influxql.Source{&influxql.Measurement{Name: string(name)}}, nil); err != nil {
return err
}
}

// Check cardinalities. In this case, the indexes behave differently.
expS, expTS, expM, expTM = 80, 159, 5, 10
if index == inmem.IndexName {
expS, expTS, expM, expTM = 80, 80, 5, 5
}

// Check cardinalities - tombstones should be in
if err := checkCardinalities(store.Store, expS, expTS, expM, expTM); err != nil {
return fmt.Errorf("[initial|re-open|delete] %v", err)
}

return nil
}

Expand Down

0 comments on commit 8c84482

Please sign in to comment.