Fix -rpcwait with -netinfo returning negative time durations #25157

jonatack commented at 2:25 PM on May 17, 2022: member

Fix bitcoin-cli -rpcwait -netinfo 1 returning negative time durations on its first invocation after node startup in the "send", "recv", and "age" columns (potentially the "txn" and "blk" columns also). To reproduce, start bitcoind on mainnet (for a longer startup time) and run bitcoin-cli -rpcwait -netinfo <n> where n is 1 or larger. The negative time durations are larger with a slower CPU speed or e.g. higher checkblocks/checklevel config option settings.

Examples:

<->   type   net  mping   ping send recv  txn  blk  hb addrp addrl  age id
out manual onion               -126 -126                             -2  0 
                     ms     ms  sec  sec  min  min                  min

<->   type   net  mping   ping send recv  txn  blk  hb addrp addrl  age id
out manual cjdns                -64  -64                             -1  0
                     ms     ms  sec  sec  min  min                  min

<->   type   net  mping   ping send recv  txn  blk  hb addrp addrl  age id
out manual  ipv4                -89  -89    *              .         -1  0
                     ms     ms  sec  sec  min  min                  min

<->   type   net  mping   ping send recv  txn  blk  hb addrp addrl  age id
out manual  ipv6               -133         *              .         -2  0  
                     ms     ms  sec  sec  min  min                  min

Use steady_clock in ConnectAndCallRPC and inline the time call in the loop conditional to avoid unnecessary invocations and an unneeded local variable allocation.

Fix -rpcwait with -netinfo printing negative time durations

Fixes negative time duration values in the "send", "recv",
and "age" columns (potentially the "txn" and "blk" columns also)
for the first run of -rpcwait -netinfo after bitcoind startup.

To reproduce, start bitcoind on mainnet and run
`bitcoin-cli -rpcwait -netinfo <n>` where n is 1 or larger.

The negative times will be larger/more apparent with a slower
CPU speed or e.g. higher checkblocks/checklevel config option
settings.

3799d2dcdd

Use steady_clock in ConnectAndCallRPC and inline time call in loop conditional

to avoid unnecessary invocations and an unneeded local variable allocation.

3a998d2e37

in src/bitcoin-cli.cpp:860 in a428d90d83 outdated

 856 | @@ -857,8 +857,7 @@ static UniValue ConnectAndCallRPC(BaseRequestHandler* rh, const std::string& str
 857 |              }
 858 |              break; // Connection succeeded, no need to retry.
 859 |          } catch (const CConnectionFailed& e) {
 860 | -            const auto now{GetTime<std::chrono::microseconds>()};
 861 | -            if (fWait && (timeout <= 0 || now < deadline)) {
 862 | +            if (fWait && (timeout <= 0 || GetTime<std::chrono::microseconds>() < deadline)) {

MarcoFalke commented at 2:47 PM on May 17, 2022:

wouldn't it be better to use std::chrono::steady_clock while touching this line?

jonatack commented at 3:27 PM on May 17, 2022:

Thanks for the feedback. Was just looking more at #25101, done.

jonatack force-pushed on May 17, 2022

DrahtBot added the label Utils/log/libs on May 17, 2022

MarcoFalke commented at 3:52 PM on May 17, 2022: member

cr ACK 3a998d2e37bf76667b08cd947807ada1305520d7

DrahtBot commented at 5:15 AM on May 18, 2022: member

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#25101 (Add mockable clock type by MarcoFalke)

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.

MarcoFalke merged this on May 18, 2022

MarcoFalke closed this on May 18, 2022

jonatack deleted the branch on May 18, 2022

luke-jr referenced this in commit abdaa1c760 on May 21, 2022

luke-jr referenced this in commit 52a1190a1c on May 21, 2022

sidhujag referenced this in commit 75baeb1a46 on May 28, 2022

jonatack referenced this in commit 0e326f5c5e on Jun 11, 2022

DrahtBot locked this on May 18, 2023