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

Skip stale tip checking if outbound connections are off or if reindexing. #14027

Merged
merged 1 commit into from Sep 27, 2018

Conversation

Projects
None yet
7 participants
@gmaxwell
Copy link
Member

commented Aug 22, 2018

I got tired of the pointless stale tip notices in reindex and on nodes with connections disabled.

@fanquake fanquake added the P2P label Aug 22, 2018

@fanquake

This comment has been minimized.

Copy link
Member

commented Aug 23, 2018

macOS build failed on Travis with:

  CXX      libbitcoin_server_a-torcontrol.o
In file included from net_processing.cpp:6:
./net_processing.h:68:84: error: use of undeclared identifier 'cs_main'
    void EvictExtraOutboundPeers(int64_t time_in_seconds) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
                                                                                   ^
./threadsafety.h:31:79: note: expanded from macro 'EXCLUSIVE_LOCKS_REQUIRED'
#define EXCLUSIVE_LOCKS_REQUIRED(...) __attribute__((exclusive_locks_required(__VA_ARGS__)))
                                                                              ^
1 error generated.
Makefile:6006: recipe for target 'libbitcoin_server_a-net_processing.o' failed
@promag

This comment has been minimized.

Copy link
Member

commented Aug 23, 2018

Concept ACK.

@promag

This comment has been minimized.

Copy link
Member

commented Aug 23, 2018

Restarted travis job, looks unrelated.

Edit: ops, didn't see that @fanquake. I guess #include <validation.h> should be moved from net_processing.cpp to net_processing.h. Or adding extern CCriticalSection cs_main is acceptable?

@MarcoFalke
Copy link
Member

left a comment

utACK 73b1272521b88f7dde9b8774968943116767cccf (doesn't compile for me, though)

src/net.h Outdated
@@ -416,6 +418,7 @@ class CConnman
int nMaxOutbound;
int nMaxAddnode;
int nMaxFeeler;
bool fUseAddrmanOutgoing;

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Aug 23, 2018

Member

nit: Members should be snake_case and prefixed with m_ to distinguish them from non-members

src/net_processing.cpp Outdated
@@ -3154,7 +3154,7 @@ void PeerLogicValidation::ConsiderEviction(CNode *pto, int64_t time_in_seconds)
}
}

void PeerLogicValidation::EvictExtraOutboundPeers(int64_t time_in_seconds)
void PeerLogicValidation::EvictExtraOutboundPeers(int64_t time_in_seconds) EXCLUSIVE_LOCKS_REQUIRED(cs_main)

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Aug 23, 2018

Member

nit: Should be enough to put this into the header file.

@@ -65,7 +65,7 @@ class PeerLogicValidation final : public CValidationInterface, public NetEventsI
/** Evict extra outbound peers. If we think our tip may be stale, connect to an extra outbound */
void CheckForStaleTipAndEvictPeers(const Consensus::Params &consensusParams);
/** If we have extra outbound peers, try to disconnect the one with the oldest block announcement */
void EvictExtraOutboundPeers(int64_t time_in_seconds);
void EvictExtraOutboundPeers(int64_t time_in_seconds) EXCLUSIVE_LOCKS_REQUIRED(cs_main);

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Aug 23, 2018

Member

Missing extern CCriticalSection cs_main; and #include <sync.h> further up?

Or just a forward decl:

+class CCriticalSection;
+extern CCriticalSection cs_main;

@gmaxwell gmaxwell force-pushed the gmaxwell:2018_08_noout_noextra branch Aug 23, 2018

@gmaxwell

This comment has been minimized.

Copy link
Member Author

commented Aug 23, 2018

@MarcoFalke how about now? :)

@practicalswift

This comment has been minimized.

Copy link
Member

commented Aug 23, 2018

Concept ACK

Increases signal to noise in our logging

@gmaxwell gmaxwell force-pushed the gmaxwell:2018_08_noout_noextra branch to 66b3fc5 Aug 23, 2018

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Aug 23, 2018

re-utACK 66b3fc5

@sdaftuar

This comment has been minimized.

Copy link
Member

commented Sep 10, 2018

ACK

@promag

This comment has been minimized.

Copy link
Member

commented Sep 10, 2018

utACK 66b3fc5.

@ken2812221

This comment has been minimized.

Copy link
Member

commented Sep 27, 2018

utACK 66b3fc5

@MarcoFalke MarcoFalke merged commit 66b3fc5 into bitcoin:master Sep 27, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

MarcoFalke added a commit that referenced this pull request Sep 27, 2018

Merge #14027: Skip stale tip checking if outbound connections are off…
… or if reindexing.

66b3fc5 Skip stale tip checking if outbound connections are off or if reindexing. (Gregory Maxwell)

Pull request description:

  I got tired of the pointless stale tip notices in reindex and on nodes with connections disabled.

Tree-SHA512: eb07d9c5c787ae6dea02cdd1d67a48a36a30adc5ccc74d6f1c0c7364d404dc8848b35d2b8daf5283f7c8f36f1a3c463aacb190d70a22d1fe796a301bb1f03228
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.