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

feat: enable WAL mode on sqlite 1.39-1.9xing Trin performance #1662

Merged
merged 2 commits into from
Feb 6, 2025

Conversation

KolbyML
Copy link
Member

@KolbyML KolbyML commented Feb 3, 2025

What was wrong?

I ran #1660 without profiling

Before using WAL
INFO trin_bench: Finished gossiping blocks in 16m 11s, with 270336 offers
After enabling WAL
INFO trin_bench: Finished gossiping blocks in 11m 40s, with 270336 offers

I am testing with era1 files 1000 to 1010

How was it fixed?

So after enabling WAL the benchmark finish 1.39 times faster or enabling WAL resulted in approximately a 27.91% reduction in time

@KolbyML KolbyML self-assigned this Feb 3, 2025
@KolbyML KolbyML changed the title feat: enable WAL mode on sqlite 3.64xing performance feat: enable WAL mode on sqlite 3.64xing Trin performance Feb 3, 2025
@KolbyML KolbyML marked this pull request as draft February 3, 2025 10:51
@KolbyML
Copy link
Member Author

KolbyML commented Feb 3, 2025

I want to test it one more time

@KolbyML
Copy link
Member Author

KolbyML commented Feb 3, 2025

Hmm looks like I might have gotten something wrong, I am closing the issue for the time being

@KolbyML KolbyML closed this Feb 3, 2025
@KolbyML KolbyML changed the title feat: enable WAL mode on sqlite 3.64xing Trin performance feat: enable WAL mode on sqlite 1.39xing Trin performance Feb 3, 2025
@KolbyML KolbyML reopened this Feb 3, 2025
@KolbyML
Copy link
Member Author

KolbyML commented Feb 3, 2025

So I improved the benchmark a little and I think the increase is less, but I found the bug Kim was telling me about and it looks pretty serious and I was able to reproduce it 2 times now, so I will debug that first before continuing with this PR

@KolbyML KolbyML changed the title feat: enable WAL mode on sqlite 1.39xing Trin performance feat: enable WAL mode on sqlite 1.39-2xing Trin performance Feb 6, 2025
@KolbyML
Copy link
Member Author

KolbyML commented Feb 6, 2025

Before
INFO trin_bench: Finished gossiping blocks in 22m 46s, with 270336 offers
After
INFO trin_bench: Finished gossiping blocks in 11m 56s, with 270336 offers

So it seems like results vary a little but generally we are seeing a 1.4 to 1.9 times more performance, depending on uTP reliability which can hopefully be smoothed out with performance improvements to uTP and stability fixes

@carver
Copy link
Collaborator

carver commented Feb 6, 2025

I found the bug Kim was telling me about and it looks pretty serious

Which bug? Are you saying that bug is related to enabling WAL, or just that dealing with it was a higher priority?

@KolbyML
Copy link
Member Author

KolbyML commented Feb 6, 2025

I found the bug Kim was telling me about and it looks pretty serious

Which bug? Are you saying that bug is related to enabling WAL, or just that dealing with it was a higher priority?

It isn't related to WAL I was just talking about things I seen well benchmarking, the error is not related to enabling WAL mode and happening either way

@KolbyML KolbyML marked this pull request as ready for review February 6, 2025 17:39
@KolbyML
Copy link
Member Author

KolbyML commented Feb 6, 2025

@carver ready for a review

@KolbyML
Copy link
Member Author

KolbyML commented Feb 6, 2025

I found the bug Kim was telling me about and it looks pretty serious

Which bug?

I was referring to running my benchmark I seen some error's Kim was telling me about, they were occurring on Trin master

Copy link
Collaborator

@carver carver left a comment

Choose a reason for hiding this comment

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

Yup, looks good. And nice addition to set synchronous mode to normal, now that that's safe to crashes and power failures.

@KolbyML KolbyML changed the title feat: enable WAL mode on sqlite 1.39-2xing Trin performance feat: enable WAL mode on sqlite 1.39-1.9xing Trin performance Feb 6, 2025
@KolbyML KolbyML merged commit b062656 into ethereum:master Feb 6, 2025
11 checks passed
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.

2 participants