performance: opt-in debounce for the provider reconcile - #9803
performance: opt-in debounce for the provider reconcile#9803zhaohuabing wants to merge 1 commit into
Conversation
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #9803 +/- ##
==========================================
+ Coverage 76.26% 76.36% +0.09%
==========================================
Files 261 262 +1
Lines 44351 44557 +206
==========================================
+ Hits 33826 34025 +199
- Misses 8284 8285 +1
- Partials 2241 2247 +6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
df12172 to
7b754de
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b754de0d7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Every watch in the gatewayapi controller enqueues the same GatewayClass request, so the workqueue already collapses events that arrive while a reconcile is in flight: the request sits in the dirty set and is requeued exactly once when Done is called. What the workqueue does not do is wait. When a reconcile is fast relative to the event rate the queue drains between events, and each event then costs a full rebuild of the resource tree even though only the resulting state matters. Wrap the controller's workqueue so that Add is held until no new request has arrived for debounce.after, bounded by debounce.max so that sustained churn cannot defer a reconcile indefinitely. AddAfter and AddRateLimited reach the embedded queue directly, so error backoff is never delayed. Reuses the same top-level debounce field in the EnvoyGateway config, and is disabled by default. Note that status is computed during reconcile, so enabling this also delays status updates by up to debounce.max. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io>
7b754de to
448d26b
Compare
Envoy Gateway coalesces reconcile requests today only by workqueue dedup — every watch enqueues the same GatewayClass request, so events arriving while a reconcile is in flight collapse into a single requeue when
Doneis called. That is not time-based, so the coalescing window is incidentally whatever one reconcile happens to take: when reconciles are fast relative to the event rate the queue drains between events, and each event then costs a full rebuild of the resource tree even though only the resulting state matters.This PR adds a time-based debounce, configured via the same top-level
debouncefield in the EnvoyGateway config as #9773 and disabled by default. A pending batch is flushed once no new request has arrived fordebounce.after(default 100ms), or afterdebounce.max(default 10s) when requests keep arriving, so isolated changes still reconcile promptly while sustained churn has a bounded reconcile rate.The debounce sits on the controller's workqueue, through the existing
NewQueuehook, so it collapses churn ahead ofReconcileitself — the cache Lists and the resource-tree rebuild — and therefore ahead of everything downstream of the provider publish too. OnlyAddis debounced:AddAfterandAddRateLimitedreach the embedded queue directly, so error-requeue backoff is never delayed.Because status is computed during
Reconcile, enabling this also delays Gateway and policy status by up todebounce.max. That is the main tradeoff against #9773, which leaves status untouched.Measured
Scaling a backend Deployment 180 times between 1 and 20 replicas, 100ms apart, against a single proxy:
A 47% raw reduction in pushes to Envoy: 299 -> 158, or 141 fewer snapshot updates.
The provider publish number is lower than the #9773 result because this PR debounces earlier: it wraps the provider controller reconcile queue before provider-resources is published.
Two caveats on reading that. The churn interval here (100ms) equals the default after, which is close to the least favourable spacing for a debouncer: each change tends to arrive near the quiet-period boundary, so faster churn should coalesce more and slower churn should coalesce little or nothing. And this ran against a single Envoy, while xds_snapshot_update_total is counted per node, so absolute savings scale with fleet size.
Churn script: https://gist.github.com/zhaohuabing/33a57e7b6fd3126c17001c3208d288b2
Observability
New metrics
reconcile_debounce_pending,reconcile_debounce_flush_total(labelled by flushreason) andreconcile_debounce_delay_secondscover the debouncer, mirroring thewatchable_debounce_*set in #9773.The existing workqueue metrics already report what determines whether debouncing here helps at all: comparing the
workqueue_adds_totalrate againstworkqueue_work_duration_secondsshows whether the dirty-set dedup is already absorbing a burst. If adds/sec × work_duration is well above 1 the queue is coalescing on its own and there is little left to win; near 1 means roughly one reconcile per event.Alternative
#9773 — debouncing the resource subscription instead with the same 100ms/10s defaults. That placement is downstream of the reconcile, so it does not avoid the provider reconcile, but it leaves status latency untouched.
The two share the
debounceAPI and are alternatives rather than a stack — enabling both would compound the delay.