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

refactor(storage/participants): change key structure, add cleanup job #1845

Open
wants to merge 52 commits into
base: stage
Choose a base branch
from
Open

refactor(storage/participants): change key structure, add cleanup job #1845

wants to merge 52 commits into from
+749 −503

Conversation

anatolie-ssv
Copy link
Contributor

@anatolie-ssv anatolie-ssv commented Nov 8, 2024 •
edited
Loading

Replace convert.RunnerRole with BeaconRole from spectypes.

  • Refactor QBFTStores into ParticipantStores
  • Get rid of the convert package
  • Add migration to remove leftover highest instance
  • Refactor participant storage key to begin with Role+Slot
  • Add API to retrieve all participants within slot range (no pubkey)
  • Add background job to remove obsolete slots on ticker
  • Add migration to remove all past records that use the previous storage key schema

Closes: #1810

@anatolie-ssv anatolie-ssv marked this pull request as draft November 8, 2024 10:20
@anatolie-ssv anatolie-ssv self-assigned this Nov 8, 2024
@anatolie-ssv anatolie-ssv marked this pull request as ready for review November 10, 2024 11:59
anatolie-ssv
anatolie-ssv commented Nov 10, 2024
View reviewed changes
ibft/storage/store.go Outdated Show resolved Hide resolved
@anatolie-ssv anatolie-ssv force-pushed the fix/remove-convert-role-2 branch from d1b4b08 to e3d2042 Compare November 12, 2024 07:36
@nkryuchkov nkryuchkov changed the title fix: remove 'convert' package refactor: (convert) remove the package Nov 14, 2024
@anatolie-ssv anatolie-ssv changed the title refactor: (convert) remove the package refactor(convert): remove the package Nov 17, 2024
@moshe-blox
Copy link
Contributor

@anatolie-ssv i made a few small refactors, can you please review?

@anatolie-ssv anatolie-ssv force-pushed the fix/remove-convert-role-2 branch from 098bc41 to a89011d Compare January 2, 2025 16:02
y0sher
y0sher reviewed Jan 6, 2025
View reviewed changes
api/handlers/exporter.go Outdated Show resolved Hide resolved
y0sher
y0sher reviewed Jan 6, 2025
View reviewed changes
ibft/storage/store.go
qbftstorage "github.com/ssvlabs/ssv/protocol/v2/qbft/storage"
"github.com/ssvlabs/ssv/storage/basedb"
)

const (
highestInstanceKey = "highest_instance"
instanceKey = "instance"
participantsKey = "participants"
participantsKey = "pt"
Copy link
Contributor

Choose a reason for hiding this comment

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

this change isn't backward compatible, it means we have to do one of the following:

  1. need to do migration
  2. resync the db
  3. search by the old key if the search is from older slot

or we just leave the old key as is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, there's a migration I just haven't committed it because I wanted to finish testing of the main functionalities without dropping old keys, will add it at a later point.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we can't do a migration since the db is too big, so we need to instruct whoever updates on how to deal with it (reset the db).

y0sher
y0sher reviewed Jan 6, 2025
View reviewed changes
ibft/storage/store.go
Comment on lines +57 to +58
prefix: []byte{role},
oldPrefix: prefix.String(),
Copy link
Contributor

Choose a reason for hiding this comment

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

the change in prefix has the same meaning as I mentioned above for the database key change.

y0sher
y0sher reviewed Jan 6, 2025
View reviewed changes
ibft/storage/store.go Outdated Show resolved Hide resolved
@anatolie-ssv anatolie-ssv requested a review from y0sher January 10, 2025 17:13
y0sher
y0sher reviewed Jan 13, 2025
View reviewed changes
cli/operator/node.go Outdated Show resolved Hide resolved
@anatolie-ssv anatolie-ssv requested a review from y0sher January 13, 2025 13:09
y0sher
y0sher reviewed Jan 13, 2025
View reviewed changes
cli/operator/node.go Outdated Show resolved Hide resolved
y0sher
y0sher reviewed Jan 13, 2025
View reviewed changes
cli/operator/node.go Outdated Show resolved Hide resolved
@anatolie-ssv anatolie-ssv requested a review from y0sher January 13, 2025 14:15
moshe-blox
moshe-blox reviewed Jan 14, 2025
View reviewed changes
cli/operator/node.go Outdated Show resolved Hide resolved
anatolie-ssv and others added 3 commits January 14, 2025 17:48
@anatolie-ssv @moshe-blox
rename slot cleanup methods
0afd685 
Co-authored-by: moshe-blox <89339422+moshe-blox@users.noreply.github.com>
@anatolie-ssv
PR feedback refactorings
f49240b
@anatolie-ssv
slot pruning only when exporter enabled
74ccb01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@y0sher y0sher y0sher approved these changes

@MatusKysel MatusKysel Awaiting requested review from MatusKysel

@moshe-blox moshe-blox Awaiting requested review from moshe-blox

At least 2 approving reviews are required to merge this pull request.

Assignees

@anatolie-ssv anatolie-ssv

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@anatolie-ssv @moshe-blox @y0sher @MatusKysel