Race building execution and builder block with cutoff #5223
Conversation
This file contains 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
|
Can this be done with somehting like: function allSettledWithTimeout<T>(promises: Promise<T>, timeoutMs: number) {
const timerPromise = new Promise((_resolve, reject) => setTimeout(reject, timeoutMs));
return Promise.allSettled(promises.map((promise) => Promise.race(promise, timerPromise)));
} |
Performance Report✔️ no performance regression detected Full benchmark results
|
problem with this is ( i think) it will force timeout both execution/blinded block at 3 sec for e.g. , but we want to resolve for whoever completes first post cutoff if all promises exceeds cutoff let me see how i can code it up in a cleaner manner |
g11tech
commented
Mar 2, 2023
g11tech
commented
Mar 2, 2023
| @@ -1,7 +1,7 @@ | |||
| { | |||
| "compilerOptions": { | |||
| "target": "es2019", | |||
| "lib": ["es2020", "esnext.bigint", "es2020.string", "es2020.symbol.wellknown", "dom"], | |||
| "lib": ["es2020", "esnext.bigint", "es2020.string", "es2020.symbol.wellknown", "dom", "es2021.promise"], | |||
There was a problem hiding this comment.
required for Promise.any
|
@wemeetagain i have updated the racefn using the inbuild primitives, let me know how it looks now |
g11tech
commented
Mar 3, 2023
| @@ -11,13 +11,20 @@ import {ValidatorStore, BuilderSelection} from "./validatorStore.js"; | |||
| import {BlockDutiesService, GENESIS_SLOT} from "./blockDuties.js"; | |||
|
|
|||
| const ETH_TO_WEI = BigInt("1000000000000000000"); | |||
| const BLOCK_PRODUCTION_RACE_CUTOFF_MS = 3_000; | |||
| const BLOCK_PRODUCTION_RACE_TIMEOUT_MS = 12_000; | |||
There was a problem hiding this comment.
are these values optimal (or good enough to being with)?
There was a problem hiding this comment.
do you know how other CL clients handle this and what cutoff they use?
There was a problem hiding this comment.
these values make sense to me
twoeths
reviewed
Mar 3, 2023
packages/beacon-node/test/scripts/el-interop/mergemock/post-merge.sh
Outdated
Show resolved
Hide resolved
twoeths
reviewed
Mar 3, 2023
nflaig
approved these changes
Mar 4, 2023
| precutoff = "precutoff-return", | ||
| /** cutoff reached as some were pending till cutoff **/ | ||
| cutoff = "cutoff-reached", | ||
| /** atleast one resolved till cutoff so no race required */ |
There was a problem hiding this comment.
Suggested change
| /** atleast one resolved till cutoff so no race required */ | |
| /** at least one resolved till cutoff so no race required */ |
| return mapStatusesToResponses(promisesStates); | ||
| } | ||
|
|
||
| // Post deadline resolve with any of the promise or all rejected before timeout |
There was a problem hiding this comment.
deadline is the cutoff here, right?
Suggested change
| // Post deadline resolve with any of the promise or all rejected before timeout | |
| // Post cutoff resolve with any of the promise or all rejected before timeout |
twoeths
reviewed
Mar 4, 2023
| // or rejected (-ve number) | ||
| // Third item in testcase row is the expected value or error message in string | ||
| // Last item is expected events | ||
| const testcases: [string, number[], (string | Error)[], RaceEvent[]][] = [ |
There was a problem hiding this comment.
thanks for a bunch of test cases 👍
twoeths
approved these changes
Mar 4, 2023
|
🎉 This PR is included in v1.6.0 🎉 |
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.
We have scenarios where builder api responded with block but execution api took forever to resolve causing an avoidable delay in block proposal (and hence orphaned block)
This PR races building execution and builder block with cutoff of 3 seconds post which the race resolves with whoever wins
racePromisesWithCutoffutilityCloses #5219