Optimize /solve request serialization in autopilot#4160
Open
MartinquaXD wants to merge 2 commits intostream-http-body-only-oncefrom
Open
Optimize /solve request serialization in autopilot#4160MartinquaXD wants to merge 2 commits intostream-http-body-only-oncefrom
MartinquaXD wants to merge 2 commits intostream-http-body-only-oncefrom
Conversation
Contributor
There was a problem hiding this comment.
Code Review
This pull request aims to optimize /solve request handling by using the X-Auction-Id header for deduplication. However, the current implementation introduces a cache poisoning vulnerability due to trusting the header without body verification, and a Denial of Service (DoS) risk by holding a global mutex during asynchronous body streaming. Furthermore, there's a critical backward compatibility issue where requests missing the X-Auction-Id header will fail, impacting rolling updates. These security and compatibility concerns need to be addressed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Because the driver serves multiple solvers it receives a bunch of duplicated
/solverequests. There is already logic to deduplicate the pre-processing but we there is still one part left that's done unnecessarily often: streaming the HTTP body.Streaming the http body currently takes up to 700ms which is surprisingly slow considering that the HTTP request goes from one k8s pod to another and not via the public internet.
I suspect the problem is that we are actually streaming ~10MB
/solverequests 23 times in parallel (numbers from mainnet).#4159 introduced a new header (
X-Auction-Id) that can be used to detect which auction a request is related to without having to stream the entire body.With this change everything but prioritizing (i.e. sorting and allocating balances for orders) and the serialization of the driver's
/solverequest will be de-duplicated. That means adding more solvers to the driver will be less costly.If we consider enforcing the same prioritization logic for ALL solvers that could also be de-duplicated leading to more or less 0 overhead for adding more solvers to the same driver.
Changes
X-Auction-Idheader to figure out whether we have to process the request or just await an existing pre-processing taskNote that this change must be released AFTER https://github.com/cowprotocol/services/pull/4159`. The reason is that k8s first rolls out
driverpods so there would be a period where the oldautopilotis still sending requests without theX-Auction-Idheader.How to test
e2e tests