-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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.17: Allow Blockstore to open unknown columns (backport of #34174) #34287
Conversation
As we develop new features or modifications, we occassionally need to introduce new columns to the Blockstore. Adding a new column introduces a compatibility break given that opening the database in Primary mode (R/W access) requires opening all columns. Reverting to an old software version that is unaware of the new column is obviously problematic. In the past, we have addressed by backporting minimal "stub" PR's to older versions. This is annoying, and only allow compatibility for the single version or two that we backport to. This PR adds a change to automatically detect all columns, and create default column descriptors for columns we were unaware of. As a result, older software versions can open a Blockstore that was modified by a newer software version, even if that new version added columns that the old version is unaware of. (cherry picked from commit 71c1782) # Conflicts: # ledger/src/blockstore_db.rs
I tested the initial PR, but as additional check, I had been running validator with tip of master and just switched it over to this branch. As expected, I see this log and things open just fine:
Going to let it catch up / run for a while, and then switch back to tip of master |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## v1.17 #34287 +/- ##
=======================================
Coverage 81.8% 81.8%
=======================================
Files 803 803
Lines 218021 218064 +43
=======================================
+ Hits 178427 178476 +49
+ Misses 39594 39588 -6 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding the original reviewers + Will to this. I'd like to backport this PR to v1.17. It creates backwards compatibility when new columns are added to the Blockstore. The original PR stemmed from a new column getting added to master in what will become v1.18. There is some relevant discussion in #releng Discord channel starting from below when Ashwin proposed adding the new column: Namely, we were previously going to backport "stub" column PR's to the release branches. But, PR provides a more general solution in that it can account for any new columns that are added. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am in favor of this backport, and the changes look correct to me
This is an automatic backport of pull request #34174 done by Mergify.
Cherry-pick of 71c1782 has failed:
To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/github/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally
Mergify commands and options
More conditions and actions can be found in the documentation.
You can also trigger Mergify actions by commenting on this pull request:
@Mergifyio refresh
will re-evaluate the rules@Mergifyio rebase
will rebase this PR on its base branch@Mergifyio update
will merge the base branch into this PR@Mergifyio backport <destination>
will backport this PR on<destination>
branchAdditionally, on Mergify dashboard you can:
Finally, you can contact us on https://mergify.com