feat/db: paged loading of nullifiers and account commitments#1536
feat/db: paged loading of nullifiers and account commitments#1536
Conversation
drahnr
force-pushed
the
bernhard-startup-build-and-rebuild-rocksdb
branch
2 times, most recently
from
a4f0119 to
24b0977
Compare
drahnr
force-pushed
the
bernhard-startup-build-rocksdb-paged-queries
branch
from
9f4e15c to
847e701
Compare
…d-rocksdb-paged-queries
drahnr
requested review from
Mirko-von-Leipzig and
bobbinth
and removed request for
Mirko-von-Leipzig
…d-rocksdb-paged-queries
| if page.commitments.is_empty() { | ||
| break; | ||
| } |
There was a problem hiding this comment.
Nit: is this strictly necessary? Applying an empty set if probably close to a no-op I imagine, and we'll exit because cursor.is_none().
Mostly just a style thing but I'd prefer one exit if possible. Similar below.
| if let Some(cursor) = after_nullifier { | ||
| query = query.filter(schema::nullifiers::nullifier.gt(cursor.to_bytes())); | ||
| } |
There was a problem hiding this comment.
Nit: probably quicker to just after_nullifier.unwrap_or_default() since it will cache the statement on the next call 😁
There was a problem hiding this comment.
Ah nvm I see you're using after and not next as the token.
There was a problem hiding this comment.
Ah nvm I see you're using after and not next as the token.
I don't think there is a difference?
after_nullifier.unwrap_or_default()
This only works for an asc sorting and the implicit all-zeros nullifier. I don't like the implicitness.
There was a problem hiding this comment.
Revisiting, there is a small delta in what the cursor means, and I think we should unify eventually.
|
There seem to be some small merge conflicts. @drahnr - could you resolve them when you get a chance? |
3 participants
Addresses performance and memory constraints in #1534 when loading
AccountTreeandNullifierTree.Changes
fn select_account_commitments_pagedfn select_all_nullifiers_pagedOpen Questions
_pagedones to suffix less? Also yes