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

Proposal: Tiered Accounts DB Storage #30551

Closed
wants to merge 5 commits into from
Closed

Proposal: Tiered Accounts DB Storage #30551

wants to merge 5 commits into from

Conversation

yhchiang-sol
Copy link
Contributor

@yhchiang-sol yhchiang-sol commented Feb 27, 2023 •
edited
Loading

Summary of Changes

This proposal presents a hierarchical storage architecture that enables
the accounts database to efficiently manage a large number of accounts.
It stores active accounts in a format that is optimized for performance
while accommodating the storage of inactive accounts in a compressed
format that conserves disk space.

The WIP prototype can be found at #30626

@yhchiang-sol yhchiang-sol force-pushed the doc-tiered-storage branch 2 times, most recently from c23b32e to c2f955f Compare February 28, 2023 11:09
@yhchiang-sol yhchiang-sol mentioned this pull request Mar 2, 2023
Closed
@yhchiang-sol yhchiang-sol force-pushed the doc-tiered-storage branch 2 times, most recently from 3541b78 to 5cecdd5 Compare March 21, 2023 07:10
@lurais

This comment was marked as spam.

Sign in to view
This was referenced Apr 12, 2023
Merged
Closed
brooksprumo
brooksprumo reviewed Apr 12, 2023
View reviewed changes
Copy link
Contributor

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

Still reading through; here's a few thoughts.

docs/src/proposals/tiered-accounts-db-storage.md Outdated Show resolved Hide resolved
docs/src/proposals/tiered-accounts-db-storage.md Show resolved Hide resolved
docs/src/proposals/tiered-accounts-db-storage.md Outdated Show resolved Hide resolved
@brooksprumo brooksprumo self-requested a review April 12, 2023 20:00
@brooksprumo brooksprumo self-requested a review April 13, 2023 21:30
mvines
mvines reviewed Apr 13, 2023
View reviewed changes
docs/src/proposals/tiered-accounts-db-storage.md
@@ -0,0 +1,466 @@
---
Copy link
Member

Choose a reason for hiding this comment

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

Some discussion around the relative cost of accessing a hot vs cold account in a transaction would be useful.

If there's large delta then I could imagine somebody wanting to modify their validator to optimize for only accepting transactions with hot accounts to make it more likely that the next leader will build on their block. Or protocols modifying themselves to write unnecessarily into accounts to prevent them from getting cold.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If there's large delta then I could imagine somebody wanting to modify their validator to optimize for only accepting transactions with hot accounts to make it more likely that the next leader will build on their block. Or protocols modifying themselves to write unnecessarily into accounts to prevent them from getting cold.

That's a good insight. The plan is to have the size of hot and cold storage configurable so that validators with different specs/goals can be configured differently to optimize their needs.

In the end, it is a trade-off between the storage size and the run-time performance.

For preventing all validators from avoiding answering RPCs querying cold accounts, do we want to have some type of reward for validators answering cold RPC queries?

yhchiang-sol added a commit that referenced this pull request May 7, 2023
@yhchiang-sol
Implementation of the footer for tiered account storage (#31161)
8120d31 
#### Summary of Changes
This PR includes the implementation of the footer for the tiered account storage.

Tiered account storage proposal: #30551
The prototype implementation of the tiered account storage: #30626.
@github-actions github-actions bot added the stale [bot only] Added to stale content; results in auto-close after a week. label May 18, 2023
@yhchiang-sol yhchiang-sol removed the stale [bot only] Added to stale content; results in auto-close after a week. label May 24, 2023
@github-actions github-actions bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Jun 8, 2023
@yhchiang-sol yhchiang-sol removed the stale [bot only] Added to stale content; results in auto-close after a week. label Jun 12, 2023
@github-actions github-actions bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Jun 26, 2023
@yhchiang-sol yhchiang-sol removed the stale [bot only] Added to stale content; results in auto-close after a week. label Jun 28, 2023
@github-actions github-actions bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Jul 13, 2023
@github-actions github-actions bot closed this Jul 21, 2023
@yhchiang-sol yhchiang-sol removed the stale [bot only] Added to stale content; results in auto-close after a week. label Jul 21, 2023
@yhchiang-sol yhchiang-sol reopened this Jul 21, 2023
@github-actions github-actions bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Aug 7, 2023
@github-actions github-actions bot closed this Aug 15, 2023
@brooksprumo
Copy link
Contributor

@yhchiang-sol I think it would be good to revive this proposal and bring it up-to-date with the current implementation. We'll want this proposal to be completed/accepted as a spec for our implementation.

For example, this proposal still says, for the index entries, that the pubkeys and the offsets are together. Per our January meeting, we decided to have these stored separately.

@yhchiang-sol
Copy link
Contributor Author

@yhchiang-sol I think it would be good to revive this proposal and bring it up-to-date with the current implementation. We'll want this proposal to be completed/accepted as a spec for our implementation.

For example, this proposal still says, for the index entries, that the pubkeys and the offsets are together. Per our January meeting, we decided to have these stored separately.

Sure thing. Let me go over the proposal and make everything up-to-date.

@yhchiang-sol yhchiang-sol reopened this Oct 26, 2023
@yhchiang-sol yhchiang-sol marked this pull request as draft October 26, 2023 14:35
@github-actions github-actions bot removed the stale [bot only] Added to stale content; results in auto-close after a week. label Oct 27, 2023
@github-actions github-actions bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Nov 14, 2023
yhchiang-sol
yhchiang-sol commented Nov 16, 2023
View reviewed changes
docs/src/proposals/tiered-accounts-db-storage.md
| | Entries are either sorted or hashed, depending |
| | on the index format specified in the footer. |
+------------------------------------+------------------------------------------------+
| offsets (8-byte each) | Indices to access the account entry. |
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will need to change to u32. Reading it requires packed (u32, u32).

docs/src/proposals/tiered-accounts-db-storage.md
Comment on lines +289 to +298
| data_block_offset (8 bytes) | The offset to the account's data block. |
| intra_block_offset (2 bytes) | The inner-block offset to the accounts' data |
| | after decompressing its data block. |
| | |
| uncompressed_data_len (2 bytes)| The length of the uncompressed_data. |
| | If this value is u16::MAX, it means the cold |
| | account has its own account block |
| | In this case, its block size can be derived |
| | by comparing the offset of the next cold index |
| | entry which has a different data_block_offset. |
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will be 4 + 2 + 2.

@github-actions github-actions bot removed the stale [bot only] Added to stale content; results in auto-close after a week. label Nov 17, 2023
@github-actions github-actions bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Dec 1, 2023
@github-actions github-actions bot closed this Dec 8, 2023
@brooksprumo brooksprumo added do-not-close Add this tag to exempt an issue/PR from being closed by the stalebot and removed stale [bot only] Added to stale content; results in auto-close after a week. labels Dec 8, 2023
@brooksprumo brooksprumo reopened this Dec 8, 2023
@willhickey
Copy link
Contributor

This repository is no longer in use. Please re-open this pull request in the agave repo: https://github.com/anza-xyz/agave

@willhickey willhickey closed this Mar 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brooksprumo brooksprumo brooksprumo left review comments

@mvines mvines mvines left review comments

Assignees
No one assigned
Labels
do-not-close Add this tag to exempt an issue/PR from being closed by the stalebot
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@yhchiang-sol @lurais @brooksprumo @willhickey @mvines