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.

Sign up for GitHub

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

Jump to bottom

Do not load _everything_ in memory for bookkeeping #189

Merged
merged 41 commits into from
Apr 17, 2024
Merged

Do not load _everything_ in memory for bookkeeping #189

merged 41 commits into from
Apr 17, 2024

Conversation

jeromegn
Copy link
Member

This adds some more data in sqlite to keep memory usage down and reduce time-to-start-syncing considerably.

@jeromegn jeromegn marked this pull request as ready for review April 12, 2024 19:50
@jeromegn jeromegn mentioned this pull request Apr 13, 2024
Open
jeromegn
jeromegn commented Apr 13, 2024
View reviewed changes
Copy link
Member Author

@jeromegn jeromegn left a comment

Choose a reason for hiding this comment

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

I'm a little concerned that Corrosion relies on perfect knowledge in bookkeeping to respond to other nodes. It's not different than before, but it is scary.

@jeromegn jeromegn marked this pull request as draft April 14, 2024 00:08
@jeromegn
Copy link
Member Author

Converting to draft because we should persist the gaps as soon as we detect a gap.

@jeromegn
actually persist as we go instead of once in a while, that was going …
676c940 
…to be a big problem
@jeromegn
log how long it took to load bookkeeping, reduce default apply queue …
bb3785c 
…length to 200
@jeromegn
actually add tests and fix the gaps collection
32be63d
@jeromegn
add tests for initial gap being updated
422b4fa
@jeromegn
maybe try collapsing ranges
3939de9
@jeromegn
for now, insert or ignore
75437ba
@jeromegn
I'm not sure if this is better
617810a
@jeromegn
hold onto the lock between storing in db and in memory
832174b
@jeromegn
may or may not be better, it should at least give consistent results
fd0459c
@jeromegn
better diagnose locks held too long
66450a4
@jeromegn
revolution: process empties in hot path
8e32b4b
@jeromegn
process empties by switching a version's end_version to not-null
91e2689
@jeromegn
Revert "process empties by switching a version's end_version to not-n…
21f7df2 
…ull"

This reverts commit 91e2689.
@jeromegn jeromegn marked this pull request as ready for review April 16, 2024 15:48
@jeromegn jeromegn merged commit 8b3ef0e into main Apr 17, 2024
4 checks passed
@jeromegn jeromegn deleted the reduced-bk branch April 17, 2024 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spacekookie spacekookie Awaiting requested review from spacekookie

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@jeromegn