feat #6679: add series limit config setting #7095
Conversation
dgnorton
force-pushed
the
dgn-cardinality-limits
branch
2 times, most recently
from
9be3b9c
to
519407e
Compare
| @@ -452,6 +452,10 @@ func (s *Shard) validateSeriesAndFields(points []models.Point) ([]*FieldCreate, | |||
| key := string(p.Key()) | |||
| ss := s.index.Series(key) | |||
| if ss == nil { | |||
| if len(s.index.series)-1 > s.options.Config.MaxSeriesPerDatabase { | |||
| return nil, fmt.Errorf("would exceed series limit: %s", key) | |||
There was a problem hiding this comment.
Maybe "max series per database exceeded" which similar to https://github.com/influxdata/influxdb/blob/master/tsdb/engine/tsm1/cache.go#L18
dgnorton
force-pushed
the
dgn-cardinality-limits
branch
from
519407e
to
5a11386
Compare
| @@ -57,6 +60,9 @@ type Config struct { | |||
| CompactFullWriteColdDuration toml.Duration `toml:"compact-full-write-cold-duration"` | |||
| MaxPointsPerBlock int `toml:"max-points-per-block"` | |||
|
|
|||
| // Limits | |||
| MaxSeriesPerDatabase int `toml:"max-series-per-database"` | |||
There was a problem hiding this comment.
Can you add docs that describe the behavior of this option and and that 0 disables it. Something like:
MaxSeriesPerDatabase is the maximum number of series a node hold per database. When this limit is exceeded, writes return a max series per database exceeded error. A value of 0 disables the limit.
|
Needs changelog entry under features and a note in the release notes section that states that a default limit of 1M per database is in place and may need to be disabled by setting it to |
|
A test for this would be good as well. |
dgnorton
force-pushed
the
dgn-cardinality-limits
branch
4 times, most recently
from
60183a8
to
2b89808
Compare
dgnorton
force-pushed
the
dgn-cardinality-limits
branch
from
2b89808
to
0c45597
Compare
|
While testing this with a very low number, I'm getting errors from the monitor service. Maybe we should exclude the monitor service from this limit since we control that one? Also, is this limit supposed to be per-measurement or is it for the entire database? The PR says database and that seems to be where the limit is, I just wanted to make sure. |
|
Other than the things I just mentioned, it seems to work locally for me. I tested writing enough series to hit the limit, then lowering the limit and rebooting the server to see if it would still load all of the series that had already been written. |
| @@ -6,6 +6,7 @@ | |||
| * Support for config options `[collectd]` and `[opentsdb]` has been removed; use `[[collectd]]` and `[[opentsdb]]` instead. | |||
| * Config option `data-logging-enabled` within the `[data]` section, has been renamed to `trace-logging-enabled`, and defaults to `false`. | |||
| * The keywords `IF`, `EXISTS`, and `NOT` where removed for this release. This means you no longer need to specify `IF NOT EXISTS` for `DROP DATABASE` or `IF EXISTS` for `CREATE DATABASE`. | |||
| * `max-series-per-database` was added with a default of 1M but can be disabled by setting it to `0`. | |||
There was a problem hiding this comment.
Can we break this out in a separate Breaking Changes section? It should clearly state that if you upgrade and have more than 1M series, you will need to modify your config before upgrading or writes will fail:
[data]
max-series-per-database = 0 # 0 to disable
There was a problem hiding this comment.
As a side note, I made sure to test that. If you have over 1M series, you'll still be able to start up, you just won't be able to add new series.
|
@jsternberg re trying to manage the monitor service separate, I think it's better for the user to account for that in the series limit setting or, if the system is extremely resource-constrained, disable the monitor service. |
|
👍 |
This PR adds a configurable limit on the number of series per database.
/cc @jwilder
Required for all non-trivial PRs