HPCC-31990 Add timeout to DNS lookups for soapcalls using a threadpool 4

Signed-off-by: M Kelly <[email protected]>
hpcc-systems · Jul 23, 2024 · 86d4a90 · 86d4a90
1 parent 94f5195
commit 86d4a90
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 14 deletions.
diff --git a/system/jlib/jsocket.cpp b/system/jlib/jsocket.cpp
@@ -3608,11 +3608,11 @@ static bool lookupHostAddress(const char *name, unsigned *netaddr, unsigned time
                 return true;
             }
         }
-        // if join() returns false thread still running, but its detached,
+        // if join() returns false either call failed or thread still running, but its detached,
         // will terminate, release thread args and return to pool on its own ...
 
         StringBuffer emsg;
-        emsg.appendf("getaddrinfo timed out (thread) (%d)", timeoutms);
+        emsg.appendf("getaddrinfo failed or timed out (thread) (%d)", timeoutms);
         RecursionSafeLogErr(EAI_AGAIN, 1, emsg.str(), __LINE__, name);
         return false;
     }

diff --git a/testing/unittests/jlibtests.cpp b/testing/unittests/jlibtests.cpp
@@ -4580,10 +4580,15 @@ class CAddrThreadArgs : public CInterface, implements IInterface
     IMPLEMENT_IINTERFACE;
     StringAttr name;
     unsigned timeoutms;
+    bool logIt = true;
 
     CAddrThreadArgs(const char *_name, unsigned _timeoutms) : name(_name), timeoutms(_timeoutms)
     {
     }
+
+    CAddrThreadArgs(const char *_name, unsigned _timeoutms, bool _logIt) : name(_name), timeoutms(_timeoutms), logIt(_logIt)
+    {
+    }
 };
 
 class CAddrPoolFactory : public CInterface, public IThreadFactory
@@ -4611,13 +4616,15 @@ class CAddrPoolFactory : public CInterface, public IThreadFactory
             unsigned lookupTimeMS = timer.elapsedMs();
 
             StringBuffer ipstr;
-            if (srtn)
+            if (args->logIt && srtn)
                 ep.getIpText(ipstr);
-            else
+            else if (!srtn)
                 ipstr.append("failed");
-
-            DBGLOG("%s (%d) -> %s (%u ms)", name.str(), (int)timeoutms, ipstr.str(), lookupTimeMS);
-            fflush(NULL);
+            if ((args->logIt && srtn) || (!srtn))
+            {
+                DBGLOG("%s (%d) -> %s (%u ms)", name.str(), (int)timeoutms, ipstr.str(), lookupTimeMS);
+                fflush(NULL);
+            }
         }
 
         virtual bool stop() override
@@ -4659,7 +4666,7 @@ class getaddrinfotest : public CppUnit::TestFixture
  *      maxDNSThreads: 100
  */
 
-    void testaddr1(const char *_name, unsigned timeoutms)
+    void testaddr1(const char *_name, unsigned timeoutms, bool logIt=true)
     {
         StringBuffer name(_name);
 
@@ -4669,13 +4676,15 @@ class getaddrinfotest : public CppUnit::TestFixture
         unsigned lookupTimeMS = timer.elapsedMs();
 
         StringBuffer ipstr;
-        if (srtn)
+        if (logIt && srtn)
             ep.getIpText(ipstr);
-        else
+        else if (!srtn)
             ipstr.append("failed");
-
-        DBGLOG("%s (%d) -> %s (%u ms)", name.str(), (int)timeoutms, ipstr.str(), lookupTimeMS);
-        fflush(NULL);
+        if ((logIt && srtn) || (!srtn))
+        {
+            DBGLOG("%s (%d) -> %s (%u ms)", name.str(), (int)timeoutms, ipstr.str(), lookupTimeMS);
+            fflush(NULL);
+        }
     }
 
     void testaddr()
@@ -4688,7 +4697,7 @@ class getaddrinfotest : public CppUnit::TestFixture
         testaddr1("google.com", 500);
 
         Owned<CAddrPoolFactory> threadFactory = new CAddrPoolFactory();
-        Owned<IThreadPool> threadPool = createThreadPool("GetAddrThreadPool", threadFactory, false, nullptr, 60);
+        Owned<IThreadPool> threadPool = createThreadPool("GetAddrThreadPool", threadFactory, true, nullptr, 60);
 
         // -----------------
 
@@ -4746,12 +4755,42 @@ class getaddrinfotest : public CppUnit::TestFixture
 
         threadPool->joinAll();
 
+        fflush(NULL);
+
         threadPool->startNoBlock(t1c);
         threadPool->startNoBlock(t2c);
         threadPool->startNoBlock(t3c);
 
         threadPool->joinAll(true);
 
+        fflush(NULL);
+
+        // ---------------
+
+        CCycleTimer timer;
+        for (int i=0; i<10000; i++)
+        {
+            testaddr1("google.com", 500, false);
+        }
+        unsigned lookupTimeMS = timer.elapsedMs();
+        DBGLOG("10k lookups (same thread) time = %u ms", lookupTimeMS);
+        fflush(NULL);
+
+        Owned<IThreadPool> threadPool1 = createThreadPool("GetAddrThreadPool1", threadFactory, true, nullptr, 1);
+
+        timer.reset();
+        Owned<CAddrThreadArgs> t10a = new CAddrThreadArgs("google.com", 500, false);
+        for (int i=0; i<10000; i++)
+        {
+            threadPool1->start(t10a, "threadpool-test", 99999999);
+        }
+
+        threadPool1->joinAll(true);
+
+        lookupTimeMS = timer.elapsedMs();
+        fflush(NULL);
+        DBGLOG("10k lookups (threadpool of 1) time = %u ms", lookupTimeMS);
+
         fflush(NULL);
         DBGLOG("testaddr complete");
         fflush(NULL);