Optimize: Keep cont->mutex locked, during probe the bucket by hash object within HostDBProcessor::getby

[oknet]

No description provided.

[SolidWallOfCode]

Should delete this comment, it's clearly not safe because the lock on line 619 is scoped and has been dropped at this point.

[oknet]

deleted.

[SolidWallOfCode]

I'm very much not fond of goto. In this case, I'd be tempted to change the do;while(loop) into a while (loop). E.g. something like

MUTEX_TRY_LOCK(...);
bool loop = lock.is_locked();
while (loop) {
  loop = false; // ...
  // ...
}

[oknet]

Updated per your suggestion.

[SolidWallOfCode]

Thanks.

[scw00]

Could it be dead lock ?

See

trafficserver/iocore/hostdb/HostDB.cc

Lines 668 to 683 in 784caa6

	Lretry:
	// Otherwise, create a continuation to do a deeper probe in the background
	//
	HostDBContinuation *c = hostDBContAllocator.alloc();
	HostDBContinuation::Options opt;
	opt.timeout = dns_lookup_timeout;
	opt.force_dns = aforce_dns;
	opt.cont = cont;
	opt.host_res_style = host_res_style;
	c->init(hash, opt);
	SET_CONTINUATION_HANDLER(c, (HostDBContHandler)&HostDBContinuation::probeEvent);
	thread->schedule_in(c, MUTEX_RETRY_DELAY);
	return &c->action;

I'm not sure here, But the HostDBContinuation use the same bucket lock. And it grab the bucket lock first then get continuation's lock (which equals to action.mutex). I know that the the cont will run after getby but it would be better to comment it if my understanding is correctly.

[SolidWallOfCode]

I think not. The locking is try, not hard locks, which prevents deadlock. Any continuation that gets one lock but not the other will drop the acquired lock rather than waiting on the second lock.

[scw00]

Thanks +1 ..

[oknet]

@scw00 @SolidWallOfCode

Rebased to resolve merge conflicts with #4926.