Fix/load limits from settings at init

[urielkelman]

After a Gateway restart, the first burst to a workflow was limited to the code default instead of the org override. New buckets were created full at the default burst, and the configured value was only applied afterwards by the background poll. Raising the burst does not add tokens, so the bucket stayed short until it refilled. With every30s:30 that takes about 13.5 minutes.

Changes

pkg/settings/limits/rate.go

• newRateLimiter now takes the request ctx and resolves the tenant's rate with rateFn before calling rate.NewLimiter, so a new bucket starts full at the configured burst. If the lookup fails it logs and falls back to the default, like the updater does.
• getOrCreate now returns an existing bucket before building anything, so the settings lookup runs once per tenant bucket instead of on every call. When two callers race on a new tenant, the losing limiter is discarded without ever starting its update loop.
• globalRateLimiter had the same pattern and now resolves the rate once at construction.

SetBurst behaviour is unchanged, so lowering and then raising a limit still does not hand out an extra burst.

[github-actions]

👋 urielkelman, thanks for creating this pull request!

To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team.

Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks!

[github-actions]

📊 API Diff Results

No changes detected for module github.com/smartcontractkit/chainlink-common

View full report

[bolekk]

Nice find! LGTM but I'll let @jmank88 review first. Is it safe to resolve the real limit in constructors?

[jmank88]

Nice find! LGTM but I'll let @jmank88 review first. Is it safe to resolve the real limit in constructors?

It is safe since we still fall back to the other defaults. I don't like the background context, but I'm not sure what else we can do. Maybe defer constructing the actual backing rate limiter until first use?

[bolekk]

@jmank88 are you OK approving then?
It will be useful for BCM's tests to have this fix in, even if it's not perfect.

[jmank88]

@jmank88 are you OK approving then? It will be useful for BCM's tests to have this fix in, even if it's not perfect.

I would like to understand the other part of the change first

[urielkelman]

Nice find! LGTM but I'll let @jmank88 review first. Is it safe to resolve the real limit in constructors?

It is safe since we still fall back to the other defaults. I don't like the background context, but I'm not sure what else we can do. Maybe defer constructing the actual backing rate limiter until first use?

Agree it's not ideal. MakeRateLimiter doesn't get a ctx, and the updater right below already uses context.Background(). For a global limit the ctx wouldn't change the value anyway, and the settings are in memory so there's nothing to cancel. I guess (dont have 100% context on everything) the lazy init proposal could work, but I think that could be done as follow up. or I can drop the global part from this PR if you like, i just wanted to make it consistent with the newRateLimiter constructor.