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

[Merged by Bors] - Switch default slasher backend to LMDB #4360

Closed
wants to merge 4 commits into from

Conversation

michaelsproul
Copy link
Member

Issue Addressed

Closes #4354
Closes #3987

Replaces #4305, #4283

Proposed Changes

This switches the default slasher backend back to LMDB.

If an MDBX database exists and the MDBX backend is enabled then MDBX will continue to be used. Our release binaries and Docker images will continue to include MDBX for as long as it is practical, so users of these should not notice any difference.

The main benefit is to users compiling from source and devs running tests. These users no longer have to struggle to compile MDBX and deal with the compatibility issues that arises. Similarly, devs don't need to worry about toggling feature flags in tests or risk forgetting to run the slasher tests due to backend issues.

@michaelsproul michaelsproul added work-in-progress PR is a work-in-progress slasher backwards-incompat Backwards-incompatible API change v4.3.0 Estimated Q2 2023 labels Jun 1, 2023
@michaelsproul
Copy link
Member Author

michaelsproul commented Jun 1, 2023

WIP, I still need to update CI, docs, and add some tests

@michaelsproul michaelsproul added ready-for-review The code is ready for review RFC Request for comment and removed work-in-progress PR is a work-in-progress labels Jun 1, 2023
Copy link
Member

@paulhauner paulhauner left a comment

Choose a reason for hiding this comment

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

The code looks good and I agree with the decision to move away from MDBX 👍

book/src/slasher.md Outdated Show resolved Hide resolved
@michaelsproul michaelsproul added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Jun 2, 2023
@michaelsproul michaelsproul added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Jun 2, 2023
@michaelsproul
Copy link
Member Author

Fixed up an unused import in the slasher tests, should be ready to merge after that

@paulhauner
Copy link
Member

It looks like the Windows tests are running out of disk :(

@paulhauner paulhauner added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Jun 2, 2023
@michaelsproul
Copy link
Member Author

Ah, #2342 strikes again. Will fix.

@michaelsproul michaelsproul added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. ready-for-review The code is ready for review labels Jun 4, 2023
@michaelsproul michaelsproul added the ready-for-merge This PR is ready to merge. label Jun 6, 2023
@michaelsproul
Copy link
Member Author

bors r+

bors bot pushed a commit that referenced this pull request Jun 7, 2023
## Issue Addressed

Closes #4354
Closes #3987

Replaces #4305, #4283

## Proposed Changes

This switches the default slasher backend _back_ to LMDB.

If an MDBX database exists and the MDBX backend is enabled then MDBX will continue to be used. Our release binaries and Docker images will continue to include MDBX for as long as it is practical, so users of these should not notice any difference.

The main benefit is to users compiling from source and devs running tests. These users no longer have to struggle to compile MDBX and deal with the compatibility issues that arises. Similarly, devs don't need to worry about toggling feature flags in tests or risk forgetting to run the slasher tests due to backend issues.
@bors
Copy link

bors bot commented Jun 7, 2023

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Jun 7, 2023
## Issue Addressed

Closes #4354
Closes #3987

Replaces #4305, #4283

## Proposed Changes

This switches the default slasher backend _back_ to LMDB.

If an MDBX database exists and the MDBX backend is enabled then MDBX will continue to be used. Our release binaries and Docker images will continue to include MDBX for as long as it is practical, so users of these should not notice any difference.

The main benefit is to users compiling from source and devs running tests. These users no longer have to struggle to compile MDBX and deal with the compatibility issues that arises. Similarly, devs don't need to worry about toggling feature flags in tests or risk forgetting to run the slasher tests due to backend issues.
@bors
Copy link

bors bot commented Jun 7, 2023

Build failed (retrying...):

@michaelsproul
Copy link
Member Author

bors r-

@bors
Copy link

bors bot commented Jun 7, 2023

Canceled.

@michaelsproul
Copy link
Member Author

bors r+

bors bot pushed a commit that referenced this pull request Jun 7, 2023
## Issue Addressed

Closes #4354
Closes #3987

Replaces #4305, #4283

## Proposed Changes

This switches the default slasher backend _back_ to LMDB.

If an MDBX database exists and the MDBX backend is enabled then MDBX will continue to be used. Our release binaries and Docker images will continue to include MDBX for as long as it is practical, so users of these should not notice any difference.

The main benefit is to users compiling from source and devs running tests. These users no longer have to struggle to compile MDBX and deal with the compatibility issues that arises. Similarly, devs don't need to worry about toggling feature flags in tests or risk forgetting to run the slasher tests due to backend issues.
@bors
Copy link

bors bot commented Jun 7, 2023

@bors bors bot changed the title Switch default slasher backend to LMDB [Merged by Bors] - Switch default slasher backend to LMDB Jun 7, 2023
@bors bors bot closed this Jun 7, 2023
@michaelsproul michaelsproul deleted the lmdb-default branch June 7, 2023 04:40
ghost pushed a commit to oone-world/lighthouse that referenced this pull request Jul 13, 2023
## Issue Addressed

Closes sigp#4354
Closes sigp#3987

Replaces sigp#4305, sigp#4283

## Proposed Changes

This switches the default slasher backend _back_ to LMDB.

If an MDBX database exists and the MDBX backend is enabled then MDBX will continue to be used. Our release binaries and Docker images will continue to include MDBX for as long as it is practical, so users of these should not notice any difference.

The main benefit is to users compiling from source and devs running tests. These users no longer have to struggle to compile MDBX and deal with the compatibility issues that arises. Similarly, devs don't need to worry about toggling feature flags in tests or risk forgetting to run the slasher tests due to backend issues.
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
## Issue Addressed

Closes sigp#4354
Closes sigp#3987

Replaces sigp#4305, sigp#4283

## Proposed Changes

This switches the default slasher backend _back_ to LMDB.

If an MDBX database exists and the MDBX backend is enabled then MDBX will continue to be used. Our release binaries and Docker images will continue to include MDBX for as long as it is practical, so users of these should not notice any difference.

The main benefit is to users compiling from source and devs running tests. These users no longer have to struggle to compile MDBX and deal with the compatibility issues that arises. Similarly, devs don't need to worry about toggling feature flags in tests or risk forgetting to run the slasher tests due to backend issues.
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
## Issue Addressed

Closes sigp#4354
Closes sigp#3987

Replaces sigp#4305, sigp#4283

## Proposed Changes

This switches the default slasher backend _back_ to LMDB.

If an MDBX database exists and the MDBX backend is enabled then MDBX will continue to be used. Our release binaries and Docker images will continue to include MDBX for as long as it is practical, so users of these should not notice any difference.

The main benefit is to users compiling from source and devs running tests. These users no longer have to struggle to compile MDBX and deal with the compatibility issues that arises. Similarly, devs don't need to worry about toggling feature flags in tests or risk forgetting to run the slasher tests due to backend issues.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backwards-incompat Backwards-incompatible API change ready-for-merge This PR is ready to merge. RFC Request for comment slasher v4.3.0 Estimated Q2 2023
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants