Use async name resolving to improve net thread responsiveness #4421

1. 
   4tar commented at 7:00 pm on June 26, 2014: contributor

   NOTE: This is a re-submit of PR 4259, just re-base to latest master for avoiding merge conflct, no real change was made. The previous discussion could be checked here: #4259.

   In the LookupIntern(), things changed are:

   1. Call getaddrinfo_a() instead of getaddrinfo() if available, the former is a sync version of the latter;
   2. Try using inet_pton()/inet_addr() to convert the input text to a network addr structure at first, if success the extra name resolving thread inside getaddrinfo_a() could be avoided;
   3. An interruption point added in the waiting loop for return from getaddrinfo_a(), which completes the improve for thread responsiveness.

   A easy way to see the effect is to kick off a ‘bitcoind stop’ immediately after ‘bitcoind -daemon’, before the change it would take several, or even tens of, minutes on a bad network situation to wait for the running bitcoind to exit, now it costs only seconds.

   Signed-off-by: Huang Le 4tarhl@gmail.com

2. Use async name resolving to improve net thread responsiveness

   In the LookupIntern(), things changed are:
   
     1. Call getaddrinfo_a() instead of getaddrinfo() if available, the former is a sync version of the latter;
     2. Try using inet_pton()/inet_addr() to convert the input text to a network addr structure at first, if success the extra name resolving thread inside getaddrinfo_a() could be avoided;
     3.  An interruption point added in the waiting loop for return from getaddrinfo_a(), which completes the improve for thread responsiveness.
   
   A easy way to see the effect is to kick off a 'bitcoind stop' immediately after 'bitcoind -daemon', before the change it would take several, or even tens of, minutes on a bad network situation to wait for the running bitcoind to exit, now it costs only seconds.
   
   Signed-off-by: Huang Le <4tarhl@gmail.com>

   caf6150e97
3. 
   BitcoinPullTester commented at 7:17 pm on June 26, 2014: none

   Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4421_caf6150e9785da408f1e603ae70eae25b5202d98/ for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
4. 
   theuni commented at 7:25 pm on June 26, 2014: member

   Untested ACK. The current behavior is very annoying, and this looks like a pretty clear improvement with very little added complexity.

   Nit: really the headers should be checked as well in configure, but realistically the lib checks should work on all reasonable platforms.

5. 
   laanwj commented at 8:18 pm on June 26, 2014: member

   Agreed @theuni

   BTW to rebase a pull you do not need to resubmit it. You can re-push to the same branch on github to update the current pull.

6. 
   4tar commented at 5:59 am on June 27, 2014: contributor

   @laanwj Yes I know that, I re-submit since I feel it is “cleaner” when just one commit included in one PR:)
7. 
   laanwj commented at 7:30 am on June 27, 2014: member

   You still don’t need to resubmit for that. If you rebase and force-push (push -f) you’ll only have the commits that exist in your remote repository.
8. 
   laanwj commented at 8:33 am on June 27, 2014: member

   Anyhow the code changes look good to me, going to test it.
9. 
   4tar commented at 2:21 pm on June 29, 2014: contributor

   @laanwj Got it. Will try that next time…
10. 
    laanwj commented at 6:36 am on July 4, 2014: member

    I’ve tested this for a week. No apparent problems. ACK.
11. laanwj merged this on Jul 4, 2014
12. laanwj closed this on Jul 4, 2014

    ---

13. laanwj referenced this in commit e81e2e8f7c on Jul 4, 2014
14. 4tar deleted the branch on Jul 6, 2014
15. in src/netbase.cpp: in caf6150e97

     2@@ -3,6 +3,18 @@
     3 // Distributed under the MIT/X11 software license, see the accompanying
     4 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
     5 
     6+#ifdef HAVE_CONFIG_H
     7+#include "bitcoin-config.h"
     8+#endif
     9+
    10+#ifdef HAVE_INET_PTON
    11+#include <arpa/inet.h>
    

    ---

    ---

    

    unknown commented at 5:57 pm on July 6, 2014:

    gitian-win fails to build on this see #4473
16. 
    TheBlueMatt commented at 10:33 pm on December 1, 2016: member

    Reverting in #9229 due to glibc bugs in getaddrinfo_a.
17. MarcoFalke locked this on Sep 8, 2021

Contributors
4tar BitcoinPullTester theuni laanwj TheBlueMatt