-
Notifications
You must be signed in to change notification settings - Fork 107
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
Add archive mode support #4571
Add archive mode support #4571
Conversation
2976dcd
to
9acded0
Compare
dd24376
to
1b0e7c7
Compare
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.
There still seems to be quite some duplication in the archive node implementation (e.g., get status, converting heights, getting blocks, results, light blocks, parameters, ...), could those things be extracted as well? You could make a common struct that could be embedded into both archive and full node implementations.
Codecov Report
@@ Coverage Diff @@
## master #4571 +/- ##
==========================================
+ Coverage 66.53% 66.94% +0.40%
==========================================
Files 447 448 +1
Lines 50065 50246 +181
==========================================
+ Hits 33313 33639 +326
+ Misses 12587 12441 -146
- Partials 4165 4166 +1
Continue to review full report at Codecov.
|
1ddb7e1
to
c7f61d2
Compare
I tried out how this would look (see latest commit), it's still a bit messy, but I think most of the code can be dedpulicated. Will clean it up a bit more. |
c7f61d2
to
a1daea8
Compare
5305d0c
to
5976af7
Compare
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.
The screaming you hear is, andrej when his branch has all the conflicts.
OwnTxSigner: srv.identity.NodeSigner.Public(), | ||
DisableCheckpointer: true, | ||
InitialHeight: uint64(srv.genesis.Height), | ||
// ReadOnly should actually be preferable for archive node but this fails with: |
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.
This is a badger issue (https://discuss.dgraph.io/t/read-only-log-truncate-required-to-run-db/16444/2).
.changelog/4571.bugfix.md
Outdated
@@ -0,0 +1,4 @@ | |||
RequestShutdown: fix shutdown if registration never succeeded |
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.
Can this get a backport to 22.x?
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.
Yeah makes sense, I'll also extract this into a separate PR, as this PR will wait for andrej's PR to be merged first, so it might take some more time.
Actually thats me 😬 as i'll wait for his PR to get merged first. |
69b5b45
to
5ca789a
Compare
5ca789a
to
f4b6f3f
Compare
f4b6f3f
to
7e88bc7
Compare
Hi When will this feature get ready? @ptrus |
We are interested in running archive oasis nodes with full blockchain history. we can now, correct? |
ccf1495
to
5947500
Compare
5947500
to
056aa80
Compare
056aa80
to
6292854
Compare
6292854
to
51e18d5
Compare
Closes #4539
TODO:
archive.go
andfull.go
)wait for Bump Tendermint #4427 to be merged and rebase