-
Notifications
You must be signed in to change notification settings - Fork 668
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
Feat/add all indexes #3029
Feat/add all indexes #3029
Conversation
…at the highest burnchain block header is, or return None if the DB doesn't exist yet.
…not just peers we've seen before and haven't banned yet. This ensures that a node quickly resumes downloading blocks if it gets stopped and restarted during ibd
… it immediately begins to resume downloading stacks blocks (the current, erroneous behavior is for it to download 2100 bitcoin blocks unconditionally)
@@ -653,6 +654,11 @@ impl MemPoolDB { | |||
// apply all migrations | |||
MemPoolDB::apply_schema_migrations(&mut tx)?; | |||
|
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.
Wouldn't you want to add the index creation code snippet to the open
function as well? Same comment for other dbs (like open_db
for StacksChainState)
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.
Yes; thanks for catching that.
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.
Fixed
Codecov Report
@@ Coverage Diff @@
## develop #3029 +/- ##
===========================================
+ Coverage 82.42% 82.66% +0.23%
===========================================
Files 242 242
Lines 195691 195780 +89
===========================================
+ Hits 161301 161833 +532
+ Misses 34390 33947 -443
Continue to review full report at Codecov.
|
…e, in addition to doing so on instantiation
…ork/stacks-blockchain into feat/add-all-indexes
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.
LGTM -- just had one comment
…on boot-up (this was only meant to forget about bans)
This PR makes it so all database indexes in all databases are instantiated unconditionally, in an idempotent fashion, after all migrations have been applied. This way, if we want to add future indexes in subsequent PRs and have them take effect on existing chain state, we can do so just by adding the index-creation command to the list of indexes to create.
While testing this, I uncovered and fixed two small bugs that arise only when a node is stopped and restarted during boot-up:
When booting up, the neighbor-walk logic should always try to walk to a bootstrap peer during IBD. This way, the inventory state-machine will be able to resume crawling bootstrap peers right away, which causes block downloads to resume right away. Before, it would take potentially a long time for the node to resume downloading blocks, because it would only stumble upon a bootstrap peer by chance.
When booting up from existing chainstate, the burnchain controller would download 2100 Bitcoin blocks (1 reward cycle) unconditionally from when it left off, before starting the runloop and chains coordinator. This PR makes it so that the node will instead just resume the main run loop and chains coordinator, so that both Bitcoin and Stacks blocks can proceed in parallel.