Skip to content
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

Prefetch entries in Soroban ops #3651

Open
sisuresh opened this issue Jan 26, 2023 · 3 comments · May be fixed by #4178
Open

Prefetch entries in Soroban ops #3651

sisuresh opened this issue Jan 26, 2023 · 3 comments · May be fixed by #4178
Assignees

Comments

@sisuresh
Copy link
Contributor

sisuresh commented Jan 26, 2023

InvokeHostFunctionOp, BumpFootprintExpirationOp, and RestoreFootprintOp does not prefetch any entries, leading to each entry in the footprint being fetched individually.

@anupsdf
Copy link
Contributor

anupsdf commented Jun 22, 2023

Setting target milestone to Post testnet after discussing with @sisuresh

@sisuresh sisuresh changed the title Prefetch entries in InvokeHostFunctionOp Prefetch entries in Soroban ops Sep 22, 2023
@SirTyson
Copy link
Contributor

It looks like Soroban TXs (Restore, Extend, InvokeHostFunction) don't prefetch any keys. For SQL, this makes sense. We prefetch all keys in a single query, so there is no way for us to enforce byte read limits. We could potentially do something like this but the query performance may be significantly limited by such measures, defeating prefetch gains in the first place. However, for BucketListDB, we can enforce a byte limited prefetch, where we prefetch keys until we reach the given limit. This would provide more efficient disk access but prevent IO DOS attacks, where a malicious TX includes lots of very large keys with a small readBytes value. This will require some plumbing to make sure we propagate the prefetch byte limit correctly since this would have to be a per-tx limit. For SQL, I think it's best that we just permanently disable Soroban TX prefetching since we're shifting to BucketListDB anyway and a SQL solution doesn't seem super clear.

@MonsieurNicolas
Copy link
Contributor

Agree. I think for BucketsDB the benefit is batching should allow us to take advantage of the order of keys and reduce random seeks (and may also yield benefits down the line with parallelization as well).
Assigning to @ThomasBrady that will probably be the one working on this at some point

@ThomasBrady ThomasBrady linked a pull request Feb 21, 2024 that will close this issue
6 tasks
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 a pull request may close this issue.

5 participants