Skip to content

BM-615 - Add caching for assessor and set builder images #393

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 20 commits into from
Mar 20, 2025

Conversation

willemolding
Copy link
Contributor

Closes #388

Uses reqwest middleware to add a HTTP-aware cache. See https://docs.rs/http-cache-reqwest/latest/http_cache_reqwest/index.html. This is used only for caching requests for assessor and set-builder images at this time.

  • Adds caching to requests to retrieve assessor and set builder images
    • Can trivially also add it to user images but this may need more discussion
  • Add new command line flag --cache-images to broker
  • Adds new command line arg --cache-dir to broker (default /tmp/broker_cache/)

Copy link

linear bot commented Mar 12, 2025

@willemolding willemolding force-pushed the willem/bm-615-image-cache branch 2 times, most recently from e018f20 to 9dc3c71 Compare March 12, 2025 04:15
@willemolding willemolding requested a review from nategraf March 12, 2025 04:16
@willemolding willemolding force-pushed the willem/bm-615-image-cache branch from 9dc3c71 to 0b5df2b Compare March 12, 2025 04:25
@willemolding willemolding requested a review from rlukata as a code owner March 17, 2025 20:33
Copy link
Contributor

@willpote willpote left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason we wouldn't default this to true? Otherwise it still feels like a footgun that provers will eventually run into.

Copy link

github-actions bot commented Mar 17, 2025

🚀 Documentation Preview

Deployment URL: https://boundless-documentation-aqg49wqr4-risczero.vercel.app

Updated at: 2025-03-20 23:43:40 UTC

@willemolding
Copy link
Contributor Author

Any reason we wouldn't default this to true? Otherwise it still feels like a footgun that provers will eventually run into.

Ok yeah we can default to true. In this case maybe a --nocache flag is a nicer way to do it

willemolding and others added 13 commits March 21, 2025 12:26
)

Closes #390

- Adds new public method to `boundless_market` -
`with_stake_balance_alert` which takes a warning and error threshold to
alert on
- Adds new config fields to broker to set these on the market in
`order_monitor`
- Adds to existing test to check that market is correctly alerting when
stake balance goes below thresholds
The error would panic and crash the instance, which was indefinitely
irrecoverable. This rejects that invalid case, such that the node will
continue.

The part I am unsure about is the sanity check within the executor for
keccak requests. This changes to error log and skip those requests, and
didn't seem like a clean mechanism to fail to proof as a whole, but
perhaps this should still go through to fail with the prove agent so
that it errors and then perhaps the tasks get cancelled through taskdb?
Haven't looked that in depth at the implications yet.
We now favor "Explorer" over "Indexer" in all comms
Automatic testing of Bento when the bento/ subdirectory is modified.

---------

Co-authored-by: Austin Abell <[email protected]>
Co-authored-by: Victor Graf <[email protected]>
Partially handles BM-616

Co-authored-by: Richard Howard <[email protected]>
rlukata and others added 6 commits March 21, 2025 12:27
Partially handles BM-616
We've seen transient errors when calling the Bonsai API. This adds
retries to all API calls (previously we only retried calls to check job
status). This also adds retries to the entire proving workflow, to
better capture the case where are able to kick off a proving job and it
terminates, but it terminates with a retryable internal server error.

This should help with:
#410

---------

Co-authored-by: Willem Olding <[email protected]>
Co-authored-by: Victor Snyder-Graf <[email protected]>
Co-authored-by: Rami Lukata <[email protected]>
Co-authored-by: Angelo Capossele <[email protected]>
CI wasn't running any rust test on CI due to a wrong `ignored` keyword
@willemolding willemolding force-pushed the willem/bm-615-image-cache branch from eec0428 to d735ee1 Compare March 20, 2025 23:39
@willemolding willemolding enabled auto-merge (squash) March 20, 2025 23:40
@willemolding willemolding merged commit 6e8ba39 into main Mar 20, 2025
18 checks passed
@willemolding willemolding deleted the willem/bm-615-image-cache branch March 20, 2025 23:51
nategraf added a commit that referenced this pull request Mar 20, 2025
Closes #388 

Uses reqwest middleware to add a HTTP-aware cache. See
https://docs.rs/http-cache-reqwest/latest/http_cache_reqwest/index.html.
This is used only for caching requests for assessor and set-builder
images at this time.

- Adds caching to requests to retrieve assessor and set builder images
- - Can trivially also add it to user images but this may need more
discussion
- Add new command line flag `--cache-images` to broker
- Adds new command line arg `--cache-dir` to broker (default
`/tmp/broker_cache/`)

---------

Co-authored-by: Austin Abell <[email protected]>
Co-authored-by: Cohan <[email protected]>
Co-authored-by: Sasha <[email protected]>
Co-authored-by: pote.eth <[email protected]>
Co-authored-by: Parker Thompson <[email protected]>
Co-authored-by: Victor Graf <[email protected]>
Co-authored-by: Rami Lukata <[email protected]>
Co-authored-by: Richard Howard <[email protected]>
Co-authored-by: Angelo Capossele <[email protected]>
nategraf added a commit that referenced this pull request Mar 20, 2025
Closes #388 

Uses reqwest middleware to add a HTTP-aware cache. See
https://docs.rs/http-cache-reqwest/latest/http_cache_reqwest/index.html.
This is used only for caching requests for assessor and set-builder
images at this time.

- Adds caching to requests to retrieve assessor and set builder images
- - Can trivially also add it to user images but this may need more
discussion
- Add new command line flag `--cache-images` to broker
- Adds new command line arg `--cache-dir` to broker (default
`/tmp/broker_cache/`)

---------

Co-authored-by: Austin Abell <[email protected]>
Co-authored-by: Cohan <[email protected]>
Co-authored-by: Sasha <[email protected]>
Co-authored-by: pote.eth <[email protected]>
Co-authored-by: Parker Thompson <[email protected]>
Co-authored-by: Victor Graf <[email protected]>
Co-authored-by: Rami Lukata <[email protected]>
Co-authored-by: Richard Howard <[email protected]>
Co-authored-by: Angelo Capossele <[email protected]>
nategraf added a commit that referenced this pull request Mar 21, 2025
Closes #388 

Uses reqwest middleware to add a HTTP-aware cache. See
https://docs.rs/http-cache-reqwest/latest/http_cache_reqwest/index.html.
This is used only for caching requests for assessor and set-builder
images at this time.

- Adds caching to requests to retrieve assessor and set builder images
- - Can trivially also add it to user images but this may need more
discussion
- Add new command line flag `--cache-images` to broker
- Adds new command line arg `--cache-dir` to broker (default
`/tmp/broker_cache/`)

---------

Co-authored-by: Austin Abell <[email protected]>
Co-authored-by: Cohan <[email protected]>
Co-authored-by: Sasha <[email protected]>
Co-authored-by: pote.eth <[email protected]>
Co-authored-by: Parker Thompson <[email protected]>
Co-authored-by: Victor Graf <[email protected]>
Co-authored-by: Rami Lukata <[email protected]>
Co-authored-by: Richard Howard <[email protected]>
Co-authored-by: Angelo Capossele <[email protected]>
nategraf added a commit that referenced this pull request Mar 21, 2025
Closes #388 

Uses reqwest middleware to add a HTTP-aware cache. See
https://docs.rs/http-cache-reqwest/latest/http_cache_reqwest/index.html.
This is used only for caching requests for assessor and set-builder
images at this time.

- Adds caching to requests to retrieve assessor and set builder images
- - Can trivially also add it to user images but this may need more
discussion
- Add new command line flag `--cache-images` to broker
- Adds new command line arg `--cache-dir` to broker (default
`/tmp/broker_cache/`)

---------

Co-authored-by: Austin Abell <[email protected]>
Co-authored-by: Cohan <[email protected]>
Co-authored-by: Sasha <[email protected]>
Co-authored-by: pote.eth <[email protected]>
Co-authored-by: Parker Thompson <[email protected]>
Co-authored-by: Victor Graf <[email protected]>
Co-authored-by: Rami Lukata <[email protected]>
Co-authored-by: Richard Howard <[email protected]>
Co-authored-by: Angelo Capossele <[email protected]>
nategraf added a commit that referenced this pull request Mar 21, 2025
Closes #388 

Uses reqwest middleware to add a HTTP-aware cache. See
https://docs.rs/http-cache-reqwest/latest/http_cache_reqwest/index.html.
This is used only for caching requests for assessor and set-builder
images at this time.

- Adds caching to requests to retrieve assessor and set builder images
- - Can trivially also add it to user images but this may need more
discussion
- Add new command line flag `--cache-images` to broker
- Adds new command line arg `--cache-dir` to broker (default
`/tmp/broker_cache/`)

---------

Co-authored-by: Austin Abell <[email protected]>
Co-authored-by: Cohan <[email protected]>
Co-authored-by: Sasha <[email protected]>
Co-authored-by: pote.eth <[email protected]>
Co-authored-by: Parker Thompson <[email protected]>
Co-authored-by: Victor Graf <[email protected]>
Co-authored-by: Rami Lukata <[email protected]>
Co-authored-by: Richard Howard <[email protected]>
Co-authored-by: Angelo Capossele <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cache guest binaries downloaded from IPFS
7 participants