storage: limit fruitless SearchV2 metabase iterations#3915
storage: limit fruitless SearchV2 metabase iterations#3915
Conversation
End-rey
requested review from
carpawell,
cthulhu-rider and
roman-khimov
as code owners
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3915 +/- ##
=======================================
Coverage 26.80% 26.80%
=======================================
Files 677 677
Lines 44708 44732 +24
=======================================
+ Hits 11983 11992 +9
- Misses 31613 31627 +14
- Partials 1112 1113 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| Perm fs.FileMode `mapstructure:"perm"` | ||
| MaxBatchSize internal.Size `mapstructure:"max_batch_size"` | ||
| MaxBatchDelay time.Duration `mapstructure:"max_batch_delay"` | ||
| SearchIterationLimit internal.Size `mapstructure:"search_iteration_limit"` |
There was a problem hiding this comment.
Is it possible to have zero internal.Size?
There was a problem hiding this comment.
Actually, didn't notice. Changed it slightly to a different semantics with -1 for no limit.
docs/storage-node-configuration.md
Outdated
| | `perm` | file mode | `0640` | Permissions to set for the database file. | | ||
| | `max_batch_size` | `int` | `1000` | Maximum amount of write operations to perform in a single transaction. | | ||
| | `max_batch_delay` | `duration` | `10ms` | Maximum delay before a batch starts. | | ||
| | `search_iteration_limit` | `int` | `10000` | Maximum number of objects to be iterated without a match during search operations. | |
There was a problem hiding this comment.
Behavior for zero should be specified as well.
| // [ErrSearchIterationLimitExceeded]. | ||
| func WithSearchIterationLimit(limit uint64) Option { | ||
| return func(c *cfg) { | ||
| if limit != 0 { |
There was a problem hiding this comment.
How can I disable this behavior then? The default is 10K, 0 isn't allowed.
| if !handleKV(primKey, nil) { | ||
| break | ||
| } | ||
| if db.searchIterationLimit > 0 && iterations >= db.searchIterationLimit && len(resHolder.Objects) == 0 { |
There was a problem hiding this comment.
Not sure it's effective this way, maybe some request can be created to have a single element and then still make iterations for all the other cases. I'd rather count iterations between resHolder.Objects increments (or not), resetting the counter when an object is added. However this can also be abused to try 10K*1000 iterations. But it should be harder to do that and then iteration count can be reduced.
End-rey
force-pushed
the
3172-searchv2-dos-protection
branch
from
58dc315 to
2f29338
Compare
End-rey
force-pushed
the
3172-searchv2-dos-protection
branch
from
2f29338 to
8f41303
Compare
Add `storage.shard.metabase.search_iteration_limit` for filtered SearchV2, -1 means no limit, 0 means default. Abort metabase scans after the configured number of consecutive iterations without a match. Closes #3172. Signed-off-by: Andrey Butusov <andrey@nspcc.io>
2 participants
Closes #3172.