Add builder header timeout flag#5857
Conversation
|
Do we want to force minimum and maximum values for this flag? I don't see why anyone would want this set to less than one second, or to more than 2-3 seconds |
dapplion
left a comment
There was a problem hiding this comment.
Do we want to force minimum and maximum values for this flag? I don't see why anyone would want this set to less than one second, or to more than 2-3 seconds
If you set a very low value you will prioritize local building, which you can do today already.
If you set a very high value you will never consider local building and potentially cause liveness issues if builders misbehave. In extreme cases the failover should kick in switching to local building if the failures are common.
I have a soft opinion towards not specifying neither a minimum nor a maximum
jimmygchen
left a comment
There was a problem hiding this comment.
Looks good - I don't have anything else to add!
michaelsproul
left a comment
There was a problem hiding this comment.
Looks good modulo capitalisation fix
Squashed commit of the following: commit 5950106 Author: Eitan Seri-Levi <eserilev@ucsc.edu> Date: Wed May 29 22:43:11 2024 +0200 update cli commit 773c4d4 Author: Eitan Seri-Levi <eserilev@ucsc.edu> Date: Wed May 29 20:04:02 2024 +0200 add upper limit check, changes based on feedback commit 8d1f865 Author: Eitan Seri-Levi <eserilev@ucsc.edu> Date: Tue May 28 18:43:13 2024 +0200 make cli commit 30f9dec Author: Eitan Seri-Levi <eserilev@ucsc.edu> Date: Tue May 28 18:29:22 2024 +0200 add a new flag that allows for configuring the timeout for get builder header api calls commit e87cf1d Author: Eitan Seri-Levi <eserilev@ucsc.edu> Date: Tue May 28 15:52:10 2024 +0200 fix bitvector test random impl
|
@Mergifyio queue |
✅ The pull request has been merged automaticallyDetailsThe pull request has been merged automatically at fbc230e |
Squashed commit of the following: commit 5950106 Author: Eitan Seri-Levi <eserilev@ucsc.edu> Date: Wed May 29 22:43:11 2024 +0200 update cli commit 773c4d4 Author: Eitan Seri-Levi <eserilev@ucsc.edu> Date: Wed May 29 20:04:02 2024 +0200 add upper limit check, changes based on feedback commit 8d1f865 Author: Eitan Seri-Levi <eserilev@ucsc.edu> Date: Tue May 28 18:43:13 2024 +0200 make cli commit 30f9dec Author: Eitan Seri-Levi <eserilev@ucsc.edu> Date: Tue May 28 18:29:22 2024 +0200 add a new flag that allows for configuring the timeout for get builder header api calls commit e87cf1d Author: Eitan Seri-Levi <eserilev@ucsc.edu> Date: Tue May 28 15:52:10 2024 +0200 fix bitvector test random impl
Issue Addressed
#4709
Proposed Changes
Add a new flag
builder-header-timeoutwhich allows for configuring timeouts for the get builder header api call. Ive added a max limit of 3 seconds to the timeout to prevent potential liveness issues.