Skip to content

ref(allocation-policies): Simplify allocation policy config - #8644

Draft
dezzeddi wants to merge 12 commits into
masterfrom
dani/allocation-policies-config-cleanup
Draft

dezzeddi wants to merge 12 commits into
masterfrom
dani/allocation-policies-config-cleanup

Conversation

@dezzeddi

Copy link
Copy Markdown
Member

Simplifies allocation policy config. With current config, no query is limited differently.

  • One setting name per idea: scan_limit, bytes_limit, concurrent_limit, max_bytes_to_read. Old names stay as fallbacks.
  • Per-tenant parameterized override keys are removed. Match blocks in allocation_policies replace them.
  • New key_by setting picks the Redis key. Each default keeps today's key.
  • Removes CrossOrgQueryAllocationPolicy and DeleteConcurrentRateLimitAllocationPolicy, which never limit a query.
  • Removes the never-set bytes cap-instead-of-reject path and rate_limit_shard_factor.
  • Shared base classes for the bytes-scanned and concurrency policies.
  • A whole number in YAML is accepted for a float setting.

Config cleanup: getsentry/sentry-options-automator#10403

YAML writes 2 for 2.0, so a float policy setting such as bytes_throttle_divider
set to a whole number raised when the policy was built, and the resolver does
not catch it.
limit_bytes_instead_of_rejecting and max_bytes_to_read_scan_limit_divider
are not set in any region, so over-limit queries are always rejected.
Remove both settings and the code path.
…_read

organization_max_bytes_to_read and organization_referrer_max_bytes_to_read
both mean "cap this org's queries". Replace them with one max_bytes_to_read
setting, set per org or referrer in a match block. Neither is set today.
…jectingPolicy

Replace the four *_scan_limit_override param configs with one scan_limit
setting (per-tenant values go in match blocks) and a key_by setting that
picks the tenant the scan window counts against. The default key_by keeps
today's Redis key. project_referrer_scan_limit and
organization_referrer_scan_limit still apply when scan_limit is unset, so
config can move in either order.
…indowAllocationPolicy

Replace org_limit_bytes_scanned_override (param org_id, unlike every other
policy's organization_id) with a bytes_limit setting set in match blocks, and
add key_by to count the window per organization (default, today's key) or per
(organization, referrer). org_limit_bytes_scanned still applies when
bytes_limit is unset.
…e bytes policies

BytesScannedRejectingPolicy and BytesScannedWindowAllocationPolicy each built
the same Redis sliding-window check and record calls. Move them, with the
window constants and the rate limiter, into a BaseBytesScannedPolicy. Window
keys, request sizes, limits and decisions are unchanged.
It only limits queries with tenant_ids.cross_org_query set or a referrer in
cross_org_referrer_limits. No config sets cross_org_referrer_limits, and
cross_org_query is only set by the outcomes routing queries, which read storages
the policy is not configured on, so it passes every query through.
…Policy

Replace the four *_override param configs with key_by, which picks the tenant
the concurrency count is kept for: tenant (default, today's bucket),
organization or organization_referrer. Per-tenant limits go in match blocks
as concurrent_limit. No override is set in any region today.
…licy

Replace referrer_concurrent_override (param referrer) with concurrent_limit,
the same name ConcurrentRateLimitAllocationPolicy uses. Per-referrer values go
in match blocks. default_concurrent_request_per_referrer still applies when
concurrent_limit is unset. The throttle threshold now follows the effective
limit.
rate_limit_shard_factor is a per-policy config on every concurrency policy
but is not set in any region. Remove the config and use 1.
…olicy

It is ConcurrentRateLimitAllocationPolicy with query_type DELETE, but the
delete path never loads policies from sentry-options and the resolver
ignores query_type. Its only config entries (us and de search_issues reads)
make it a second read-path concurrency limiter with its own Redis bucket.
An unknown class name in config is skipped with a warning, so this and the
config removal can deploy in either order.
ConcurrentRateLimitAllocationPolicy and ReferrerGuardRailPolicy each
re-implemented the same count-check-end flow.
BaseConcurrentRateLimitAllocationPolicy now runs it; subclasses supply the
bucket and limit, which queries they skip, and how a decision becomes an
allowance. Buckets, limits, explanations and decisions are unchanged.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant