net: helpfully log in ConnectNode() which peer we are already connected to #25271

pull jonatack wants to merge 1 commits into bitcoin:master from jonatack:ConnectNode-say-which-peer-we-are-already-connected-to changing 1 files +3 −4

jonatack commented at 9:06 AM on June 3, 2022: contributor
It would be helpful to say which peer we are already connected to, when failing to connect for that reason. Also update from LogPrintf to LogPrintLevel(BCLog::NET, BCLog::Level::Info).

before
```
Failed to open new connection, already connected
```
after
```
[net:info] Failed to open new connection to c6adtamnttsuwqorf4atri67prd7vyq.b32.i2p:0, already connected (peer=5)
```
DrahtBot commented at 9:14 AM on June 3, 2022: contributor



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

No conflicts as of last run.
in src/net.cpp:451 in 5cf96bce12 outdated
```
 446 | @@ -447,9 +447,8 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
 447 |  
 448 |          // Look for an existing connection
 449 |          CNode* pnode = FindNode(static_cast<CService>(addrConnect));
 450 | -        if (pnode)
 451 | -        {
 452 | -            LogPrintf("Failed to open new connection, already connected\n");
 453 | +        if (pnode) {
 454 | +            LogPrintf("Failed to open new connection to %s, already connected (peer=%d)\n", addrConnect.ToString(), pnode->GetId());
```
MarcoFalke commented at 9:18 AM on June 3, 2022:

Maybe use LogPrintfCategory from the conflicting pull to avoid another breaking change in the log message later on?

jonatack commented at 9:24 AM on June 3, 2022:

I'm happy to update the other pull after this (it's likely to see further updates, and rebases from other merges).

jonatack commented at 9:27 AM on June 3, 2022:

Or can just close for now.

jonatack commented at 9:29 AM on June 3, 2022:

No idea if that pull will be merged though, it has no Concept ACKs, will wait and see I guess and re-open this change later.

MarcoFalke commented at 9:37 AM on June 3, 2022:

Yeah, it is pretty large. I think splitting up LogPrintfCategory on its own could make sense?

jonatack commented at 9:49 AM on June 3, 2022:

It solves a few issues while at the same time reducing the amount of code, so I'm hopeful it will move forward, but yes, will probably begin splitting it up.

jonatack commented at 4:54 PM on July 30, 2022:

Maybe use LogPrintfCategory

Re-opening and will update.
jonatack closed this on Jun 3, 2022
jonatack reopened this on Jul 30, 2022
jonatack force-pushed on Jul 30, 2022

Helpfully log in ConnectNode() which peer we are already connected to

when failing to connect for that reason, and update from
LogPrintf to LogPrintLevel(BCLog::NET, BCLog::Level::Info

before:

Failed to open new connection, already connected

after:

[net:info] Failed to open new connection to c6adtam67prd7vyq.b32.i2p:0, already connected (peer=5)

3fa7a70827

jonatack force-pushed on Jul 30, 2022
jonatack commented at 5:30 PM on July 30, 2022: contributor

Updated from from LogPrintf to LogPrintLevel(BCLog::NET, BCLog::Level::Info)
DrahtBot added the label P2P on Jul 30, 2022
jonatack commented at 7:54 PM on August 5, 2022: contributor

This is a very easy change to review and an obvious improvement, but closing for now due to lack of interest.
jonatack closed this on Aug 5, 2022
bitcoin locked this on Aug 5, 2023

Contributors

Labels

net: helpfully log in ConnectNode() which peer we are already connected to #25271

Conflicts