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] - prune data and compact state #4998

Closed
wants to merge 9 commits into from

Conversation

countvonzero
Copy link
Contributor

@countvonzero countvonzero commented Sep 12, 2023

Motivation

Closes #3049
Closes #3588

Changes

  • prune data consistently with tortoise hdist distance.

    • proposals
    • certificates
    • proposal id<->tid mapping

    certificate is needed for consensus within hdist.
    the same distance is used for all for simplicity.

  • extract active set data from ballots and save them in activesets table.

  • vacuum and checkpoint database after migration 4 is complete

this PR concludes the first update described #4984 (comment)

@countvonzero countvonzero changed the title prune data and vacuum prune data and compact state Sep 12, 2023
@codecov
Copy link

codecov bot commented Sep 12, 2023

Codecov Report

Merging #4998 (9bb6af0) into develop (dcd501d) will decrease coverage by 0.1%.
The diff coverage is 63.9%.

@@            Coverage Diff            @@
##           develop   #4998     +/-   ##
=========================================
- Coverage     77.1%   77.1%   -0.1%     
=========================================
  Files          254     257      +3     
  Lines        30113   30242    +129     
=========================================
+ Hits         23241   23318     +77     
- Misses        5363    5398     +35     
- Partials      1509    1526     +17     
Files Changed Coverage Δ
blocks/generator.go 91.0% <ø> (+0.8%) ⬆️
blocks/metrics.go 65.2% <ø> (ø)
sql/database.go 70.7% <42.8%> (-3.7%) ⬇️
sql/vacuum.go 45.4% <45.4%> (ø)
prune/prune.go 48.5% <48.5%> (ø)
sql/migrations.go 60.0% <58.3%> (-1.9%) ⬇️
sql/certificates/certs.go 75.7% <62.5%> (-0.9%) ⬇️
sql/ballots/ballots.go 68.5% <66.6%> (-0.1%) ⬇️
sql/transactions/transactions.go 77.2% <66.6%> (-0.4%) ⬇️
sql/ballots/util/extract.go 68.5% <68.5%> (ø)
... and 4 more

... and 4 files with indirect coverage changes

@countvonzero countvonzero marked this pull request as ready for review September 13, 2023 03:16
sql/vacuum/vacuum.go Outdated Show resolved Hide resolved
mesh/janitor.go Outdated Show resolved Hide resolved
mesh/janitor.go Outdated Show resolved Hide resolved
sql/vacuum/vacuum.go Outdated Show resolved Hide resolved
cmd/root.go Show resolved Hide resolved
node/node.go Outdated Show resolved Hide resolved
cmd/root.go Show resolved Hide resolved
DELETE FROM proposals WHERE layer < 19000;
DELETE FROM proposal_transactions WHERE layer < 19000;
UPDATE certificates SET cert = NULL WHERE layer < 19000;
UPDATE ballots SET ballot = prune_actives(ballot);
Copy link
Contributor Author

@countvonzero countvonzero Sep 14, 2023

Choose a reason for hiding this comment

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

considering #5002 and the fact that right now the ref ballot is still saved with activeset.
ActiveSetFromEpochFirstBlock also still retrieve activeset directly from ref ballot.
is there a safe range i should apply to remove the activeset from ballots?

at layer 17800, the state size changed from 8.2GB to 470MB with this migration. the migration took 1.5 min.

Copy link
Contributor

@dshulyak dshulyak Sep 14, 2023

Choose a reason for hiding this comment

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

my proposal was to save active sets into activeset table while pruning. and then we either have an option to sql-join them on activesethash field (requires extending ballots table so maybe not optimal) or do another query to get activeset hash when we need it.

Copy link
Contributor

@dshulyak dshulyak Sep 14, 2023

Choose a reason for hiding this comment

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

#4984 (comment)

we should also stop writing active sets encoded together with ballot, after this migration is done. not necessarily within same change, but would be better to release them together

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed to extract active set from ballots and save the prune blob.

please consider #4984 (comment)
i'll continue with the other code change accordingly.

sql/vacuum/vacuum.go Outdated Show resolved Hide resolved
node/node.go Outdated Show resolved Hide resolved
Copy link
Contributor

@dshulyak dshulyak left a comment

Choose a reason for hiding this comment

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

please see notes about pruner/extract code

mesh/janitor.go Outdated Show resolved Hide resolved
mesh/janitor.go Outdated Show resolved Hide resolved
@countvonzero
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Sep 17, 2023
@bors
Copy link

bors bot commented Sep 17, 2023

try

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@countvonzero
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Sep 19, 2023
@bors
Copy link

bors bot commented Sep 19, 2023

try

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@countvonzero
Copy link
Contributor Author

bors merge

bors bot pushed a commit that referenced this pull request Sep 19, 2023
## Motivation
Closes #3049 
Closes #3588 

## Changes
- prune data consistently with tortoise hdist distance. 
  - proposals
  - certificates
  - proposal id<->tid mapping
  
  certificate is needed for consensus within hdist.
  the same distance is used for all for simplicity.

- extract active set data from ballots and save them in activesets table.
- vacuum and checkpoint database after migration 4 is complete

this PR concludes the first update described #4984 (comment)
@bors
Copy link

bors bot commented Sep 19, 2023

Build failed:

@countvonzero
Copy link
Contributor Author

bors merge

bors bot pushed a commit that referenced this pull request Sep 19, 2023
## Motivation
Closes #3049 
Closes #3588 

## Changes
- prune data consistently with tortoise hdist distance. 
  - proposals
  - certificates
  - proposal id<->tid mapping
  
  certificate is needed for consensus within hdist.
  the same distance is used for all for simplicity.

- extract active set data from ballots and save them in activesets table.
- vacuum and checkpoint database after migration 4 is complete

this PR concludes the first update described #4984 (comment)
@bors
Copy link

bors bot commented Sep 19, 2023

Build failed:

@countvonzero
Copy link
Contributor Author

bors merge

bors bot pushed a commit that referenced this pull request Sep 19, 2023
## Motivation
Closes #3049 
Closes #3588 

## Changes
- prune data consistently with tortoise hdist distance. 
  - proposals
  - certificates
  - proposal id<->tid mapping
  
  certificate is needed for consensus within hdist.
  the same distance is used for all for simplicity.

- extract active set data from ballots and save them in activesets table.
- vacuum and checkpoint database after migration 4 is complete

this PR concludes the first update described #4984 (comment)
@bors
Copy link

bors bot commented Sep 19, 2023

Build failed:

@countvonzero
Copy link
Contributor Author

bors merge

bors bot pushed a commit that referenced this pull request Sep 19, 2023
## Motivation
Closes #3049 
Closes #3588 

## Changes
- prune data consistently with tortoise hdist distance. 
  - proposals
  - certificates
  - proposal id<->tid mapping
  
  certificate is needed for consensus within hdist.
  the same distance is used for all for simplicity.

- extract active set data from ballots and save them in activesets table.
- vacuum and checkpoint database after migration 4 is complete

this PR concludes the first update described #4984 (comment)
@bors
Copy link

bors bot commented Sep 19, 2023

Build failed:

@countvonzero
Copy link
Contributor Author

bors merge

bors bot pushed a commit that referenced this pull request Sep 19, 2023
## Motivation
Closes #3049 
Closes #3588 

## Changes
- prune data consistently with tortoise hdist distance. 
  - proposals
  - certificates
  - proposal id<->tid mapping
  
  certificate is needed for consensus within hdist.
  the same distance is used for all for simplicity.

- extract active set data from ballots and save them in activesets table.
- vacuum and checkpoint database after migration 4 is complete

this PR concludes the first update described #4984 (comment)
@bors
Copy link

bors bot commented Sep 19, 2023

Pull request successfully merged into develop.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title prune data and compact state [Merged by Bors] - prune data and compact state Sep 19, 2023
@bors bors bot closed this Sep 19, 2023
@bors bors bot deleted the prune-for-real branch September 19, 2023 05:33
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.

certificate: prune from network after hdist layer prune proposals from database
2 participants