Move pblocktree global to BlockManager #22371
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fa104ec to
faa0e12
Compare
|
Concept ACK |
|
Concept ACK Nice to see yet another naked |
|
Concept ACK |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
dongcarl
reviewed
Jun 30, 2021
faa0e12 to
7777be5
Compare
7777be5 to
fadc339
Compare
This was referenced Jul 7, 2021
Draft
fadc339 to
faa54e3
Compare
jamesob
approved these changes
Jul 15, 2021
There was a problem hiding this comment.
ACK faa54e3 (jamesob/ackr/22371.1.MarcoFalke.move_pblocktree_global_t)
One less global in validation. 👍
Built locally, ran tests.
Show signature data
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
ACK faa54e375782b21cbc2761c763128131c569e903 ([`jamesob/ackr/22371.1.MarcoFalke.move_pblocktree_global_t`](https://github.com/jamesob/bitcoin/tree/ackr/22371.1.MarcoFalke.move_pblocktree_global_t))
One less global in validation. :+1:
Built locally, ran tests.
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCgAdFiEEGNRVI1NPYuZCSIrGepNdrbLETwUFAmDwO3AACgkQepNdrbLE
TwWNEA/9GuE3UUeopz0KxIQCZvWv5QUaJNqUzWZZGrJHODKirzJweEa50/Ob5Kv1
JqJw4O2vM35aC5dn3TPrg4xtVX+xOjnxnYHevi8JgVyTY1JsR3ePkypx8B3TROrb
zrFHPxNl4c7KIio150bbz+NibLeMepptP6k7LIm+mv1cOjhpnoBWwcACgl1UQeqv
WzoMIHw/3AR6TTnM7n9x9D4nRWIQv8lcUYzcMOZmJCn7d46jKP9FyqOqh7ZBucrf
Rj8aQkoLEu8oJvWTuSYlifUarQ59hL19l+p9Al8uKVSnfXXekVK8iqZOLRu7+aTJ
QF7F2jsexy4TM0Adh113BNKz+viI0trizIWNgPB4lkVJbkNQt+ZohRSdcutpO1nd
dJ8n2RYXHkMI+e07ZBNvO6KIgk9QGnGNAXxj1uvVmkTEmOgX+j59K/pyEEg64+4T
a+Am6D3OJD/12gIHSlPUA3XpDWf5o7v8RBC0r0QDmdnAb7DisvHQZtA+qRJaKQGz
AJ3U+RIplTRjG3ZX+5ZJKHCU7awO4kerZh2njvLWgDtHJ9A/n3/ZSxJ2GaH+jRjN
XH9Arpr64ZMXzz0LpIRaKCBXg23Qi2698/0aRZwCQqq9XhCvUKdlbnDyCDg4Vwpr
lnFCwLSFn7nNWhprkPW23SxJQLOQC5ti3pbABUO7OcEd240gsmU=
=HRRw
-----END PGP SIGNATURE-----
Show platform data
Tested on Linux-4.19.0-17-amd64-x86_64-with-glibc2.28
Configured with ./configure LDFLAGS=-L/home/james/src/bitcoin2/db4/lib/ CPPFLAGS=-I/home/james/src/bitcoin2/db4/include/ CXXFLAGS=-fPIE -pipe -O2 -g -Wthread-safety-analysis -Wall -Werror=sign-compare -Wsign-compare -Werror=thread-safety-analysis --enable-wallet --enable-debug --with-daemon --enable-natpmp-default CXX=/usr/local/bin/clang++ CC=/usr/local/bin/clang --disable-shared --with-pic --enable-benchmark=no --enable-module-recovery --enable-module-schnorrsig --enable-experimental PYTHONPATH= --disable-shared --with-pic --enable-benchmark=no --enable-module-recovery --enable-module-schnorrsig --enable-experimental --no-create --no-recursion
Compiled with /usr/bin/ccache /usr/local/bin/clang++ -std=c++17 -mavx -mavx2 -fPIE -pipe -O2 -g -Wthread-safety-analysis -Wall -Werror=sign-compare -Wsign-compare -Werror=thread-safety-analysis -O0 -g3 -ftrapv -fdebug-prefix-map=$(abs_srcdir)=. -Wstack-protector -fstack-protector-all -fcf-protection=full -fstack-clash-protection -msse4 -msha -msse4.1 -msse4.2 i
Compiler version: Debian clang version 11.1.0-++20210428103820+1fdec59bffc1-1~exp1~20210428204437.162
| @@ -518,7 +518,7 @@ void ThreadImport(ChainstateManager& chainman, std::vector<fs::path> vImportFile | |||
| } | |||
| nFile++; | |||
| } | |||
| pblocktree->WriteReindexing(false); | |||
| WITH_LOCK(::cs_main, chainman.m_blockman.m_block_tree_db->WriteReindexing(false)); | |||
There was a problem hiding this comment.
So the new lock annotation caught this as unguarded eh?
theStack
approved these changes
Jul 15, 2021
ryanofsky
approved these changes
Jul 20, 2021
There was a problem hiding this comment.
Code review ACK faa54e3. I was thinking this looked like a change Carl would like, so no surprised he Mega-acked
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The block tree db is used within BlockManager to write and read the block index, so make the db global a member variable of BlockManager.