influxdata / influxdata/influxdb

Incorrect "show series cardinality" using inmem when max-series-per-database limit exceeded

Open
#11,253 4 comments 5 reactions 1 assignee View on GitHub

Nobody has claimed this yet.

1.x needs-backport/1.7
Dominant language
Rust
Stars
31.7k
Forks
3.7k
Avg merge
13h 37m
Merged PRs (30d)
8

Description

`SHOW SERIES CARDINALITY` shows incorrect count after `max-series-per-database limit exceeded` errors.

----

The `inmem` index manages global and per-shard data structures for tracking measurements and series keys. When a `max-series-per-database limit exceeded` condition occurs for a batch write which includes existing and new keys, all keys are rejected by `CreateSeriesListIfNotExists` and the write does not occur, per the following condition:

https://github.com/influxdata/influxdb/blob/26afe32611a36fd61dcf017658ab5ab316f56a5e/tsdb/index/inmem/inmem.go#L262-L266

However, existing series IDs are added to the per-shard roaring bitmap data structure (`SeriesIDSet`) here:
https://github.com/influxdata/influxdb/blob/26afe32611a36fd61dcf017658ab5ab316f56a5e/tsdb/index/inmem/inmem.go#L216

and:

https://github.com/influxdata/influxdb/blob/26afe32611a36fd61dcf017658ab5ab316f56a5e/tsdb/index/inmem/inmem.go#L252

It is now possible that TSM data for these existing keys does not exist in this shard, if the write is the first for these existing keys in this shard. When an older shard drops, series keys are removed from the global inmem data structure iif the associated series id does not exist in any of the *per-shard* `SeriesIDSet` containers.
* this means that the inconsistent per-shard `SeriesIDSet` containers will prevent series IDs from being dropped from the global per:

https://github.com/influxdata/influxdb/blob/26afe32611a36fd61dcf017658ab5ab316f56a5e/tsdb/store.go#L729-L743

This scenario would result in inflated `[database].numSeries` values and the `SHOW SERIES CARDINALITY` commands.

## Secondary Bug

Writes are incorrectly rejected when a batch of writes contains fewer new keys, such that the total series for the database, if the write were to succeed is ≤ `max-series-per-database`.

Specifically, the following logic:

https://github.com/influxdata/influxdb/blob/26afe32611a36fd61dcf017658ab5ab316f56a5e/tsdb/index/inmem/inmem.go#L263

should use `newSeriesN` rather than `len(keys)`:

```go
if max := opt.Config.MaxSeriesPerDatabase; max > 0 && len(i.series)+newSeriesN > max {
```

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.