Reimplement the TS-374 to eliminate the risk of deadlock

[oknet]

Within TS-374 ([ba3b7d0]), it acquires the lock "bucket_mutex" in blocking mode instead of the original non-blocking mode.

we have encountered deadlock like #1335 and avoids such scenarios by #1756.
But it didn't prevent anyone from accidentally causing a deadlock similar to #1335.

By reading the comments of TS-374, reviewing the commit ([ba3b7d0]) and rewriting the HostDBProcessor::getby ( #4961 ), I have a new idea to TS-374.

• Can I create a thread local pool to hold a certain number of recent queries for HostDBInfos?
• HostDBProcessor::getby() will lookup HostDBInfo from the thread local pool first, then will do MUTEX_TRY_LOCK on bucket_mutex if not found or has expired.

The solution will

• Resolve the TS-374 where a single (or very few) HostDB entries are under "pressure" in reverse proxy mode.
• Avoid the deadlock issues

@jplevyak @bryancall @SolidWallOfCode @vmamidi

Please leave your comments and ideas.

Thanks.

References:

• TS-374
• Issue Deadlock in HostDB #1335
• Pull Request delay DNS for origin server until it is required #1756
• Pull Request Optimize: rewrite getbyname_imm and getSRVbyname_imm as wrappers for getby #4961

[SolidWallOfCode]

Is it not possible to change the hard lock to a try lock and reschedule on failure to lock?

[oknet]

@SolidWallOfCode Do you mean revert the TS-374 ?

commit ba3b7d023577e33a93cc038ad64f41781633424a
Author: Bryan Call <bcall@apache.org>
Date:   Thu May 16 11:01:37 2013 -0700

    TS-374 Changed the retry to use HOST_DB_RETRY_PERIOD (2ms) and switch the
    locking from a trylock to a lock.

diff --git a/iocore/hostdb/HostDB.cc b/iocore/hostdb/HostDB.cc
index 51f58a4cc..4de46c4d6 100644
--- a/iocore/hostdb/HostDB.cc
+++ b/iocore/hostdb/HostDB.cc
@@ -995,7 +995,7 @@ HostDBProcessor::getbyname_imm(Continuation * cont, process_hostdb_info_pfn proc
       loop = false; // loop only on explicit set for retry
       // find the partition lock
       ProxyMutex *bucket_mutex = hostDB.lock_for_bucket((int) (fold_md5(md5.hash) % hostDB.buckets));
-      MUTEX_TRY_LOCK(lock, bucket_mutex, thread);
+      MUTEX_LOCK(lock, bucket_mutex, thread);
 
       if (lock) {
         // If we can get the lock do a level 1 probe for immediate result.
@@ -1028,7 +1028,7 @@ HostDBProcessor::getbyname_imm(Continuation * cont, process_hostdb_info_pfn proc
   c->init(md5, copt);
   SET_CONTINUATION_HANDLER(c, (HostDBContHandler) & HostDBContinuation::probeEvent);
 
-  thread->schedule_in(c, MUTEX_RETRY_DELAY);
+  thread->schedule_in(c, HOST_DB_RETRY_PERIOD);
 
   return &c->action;
 }

[vmamidi]

Treating hostDB as a processor is a good idea and it is going to remove the possibility of accidentally causing a deadlock. Although i like your solution with regards to HostDB, i personally think that we should do not DNS in ATS and use should use any of the existing libraries available.

[bryancall]

Looks like this has been fixed. Please reopen if there is still an issue.