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

v1.16: add merkle root meta column to blockstore (backport of #33979) #34665

Merged
merged 3 commits into from
Jan 5, 2024

Conversation

steviez
Copy link
Contributor

@steviez steviez commented Jan 5, 2024

Problem

Semi-automated backport of #33979; we had previously marked that branch for v1.16 BP in #34046, but I am unable to reopen that PR. This is the same state of those commits with a rebase for tip of v1.16

As for the motivation for this PR (discussion happened in Discord starting from here):

  • The new column was backported to v1.17
  • Because of the above, a blockstore that has been modified by v1.17 CANNOT be opened by v1.16 due to the extra column
  • The above could be problematic for operators who try upgrading to v1.17 and then want to downgrade to v1.16. As we saw in the testnet restart exercise, the "solution" was to wipe rocksdb ... this is unacceptable for mnb
  • So, BP this PR to create compatibility between v1.16 and v1.17 branches
  • For completeness, backporting Allow Blockstore to open unknown columns #34174 was also considered, but we opted to go with the stub PR as it is logically simpler, and the approach we have used successfully in the past. Once mnb has adopted v1.17, Allow Blockstore to open unknown columns #34174 (which was BP'd to v1.17) should make it such that we'll never have to do any of these "stub column BP's" again

* add merkle root meta column to blockstore

* pr feedback: remove write/reads to column

* pr feedback: u64 -> u32 + revert

* pr feedback: fec_set_index u32, use Self::Index

* pr feedback: key size 16 -> 12

(cherry picked from commit e457c02)

# Conflicts:
#	ledger/src/blockstore.rs
Copy link

codecov bot commented Jan 5, 2024

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Comparison is base (471956c) 81.8% compared to head (a7b4ee5) 81.8%.

Additional details and impacted files
@@            Coverage Diff            @@
##            v1.16   #34665     +/-   ##
=========================================
- Coverage    81.8%    81.8%   -0.1%     
=========================================
  Files         767      767             
  Lines      209271   209297     +26     
=========================================
+ Hits       171297   171304      +7     
- Misses      37974    37993     +19     

Copy link
Contributor

@AshwinSekar AshwinSekar left a comment

Choose a reason for hiding this comment

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

thanks!

@steviez
Copy link
Contributor Author

steviez commented Jan 5, 2024

Ok, I manually tested this PR. Namely,

  • Ensure node is running fine + caught up with mnb with v1.17
  • Downgrade to v1.16.25 and confirm that I see the missing column panic
[...] datapoint: panic program="validator" thread="main" one=1i
message="panicked at 'Failed to open ledger database: RocksDb(Error { message: \"Invalid argument: Column families not opened: merkle_root_meta\" })',
core/src/validator.rs:1608:10" location="core/src/validator.rs:1608:10"
version="1.16.25 (src:8dd7d060; feat:2294205250, client:SolanaLabs)"
  • Switch to this PR and confirm that opening the Blockstore succeeds

@steviez steviez merged commit 835183a into v1.16 Jan 5, 2024
33 checks passed
@steviez steviez deleted the mergify/bp/v1.16/pr-33979 branch January 5, 2024 20:24
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.

3 participants