Remove admission queue bypass mode - #27149
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
abf0a5b to
0b5a9bf
Compare
akichidis
left a comment
There was a problem hiding this comment.
Changes make sense. Left a comment regarding the choice of 10 seconds timeout. It might not offer as is - to my understanding - the protection we need during a regression.
|
|
||
| fn default_admission_queue_failover_timeout() -> Duration { | ||
| Duration::from_secs(30) | ||
| Duration::from_secs(10) |
There was a problem hiding this comment.
My understanding is that the queue will get tripped now quicker in case of a stall right? For example, if consensus deals with some temporary regression and queue doesn't move within 10 seconds, then it's possible to start seeing low value transactions coming through as they'll bypass it? Isn't this a point where we actually need to be more aggressive on prioritising high value transactions ?
There was a problem hiding this comment.
Correct. If we don't drain a tx in 10s, we would fail over to direct submission. I lowered this as a safeguard in case we don't entirely trust the queue implementation, given that our risk of a stall now impacts 100% of transactions. Now I'm thinking we leave failover at 30s for the reasons you've stated.
#27202 changes failover detection logic to "if consensus has capacity, AND we haven't drained in a time period" which perhaps is a little bit more indicative of a real halt with the queue implementation. It could be argued at that point that failover adds complexity during an incident without buying us meaningful safety.
There was a problem hiding this comment.
Failover timeout raised back to 30s (the current value)
0b5a9bf to
cc7cb85
Compare
Description
Follow-up to the
sui_mainnet_high_trafficincidents (CORE-198). Bypass let a transaction skip the gas-price priority queue whenever consensus inflight was below a threshold, so admission prioritization and load-shedding were bypassed for effectively all steady-state traffic — and during the transient where inflight dipped, a low-gas transaction could jump straight to consensus ahead of higher-gas entries still waiting in the queue. Remove bypass entirely so all user transactions are admitted through the queue. The only remaining direct-to-consensus paths are: queue turned off by config, failover (queue actor stalled), and ping requests.Test plan
Covered by the existing admission queue e2e tests in
sui-e2e-tests/tests/admission_queue_tests.rs(admission, eviction, rejection, reconfig), which previously forced queue mode viabypass_fraction: 0.0and now exercise the only behavior.Release notes
admission-queue-bypass-fractionvalidator config field; all transactions are now admitted through the gas-price priority queue. The removed field is ignored if still present in a config file, but should be deleted.