[Tickets #13231] Re: Hashtable session handler doesn't unlock on session close

[noreply at bugs.horde.org]

DO NOT REPLY TO THIS MESSAGE. THIS EMAIL ADDRESS IS NOT MONITORED.

Ticket URL: http://bugs.horde.org/ticket/13231
------------------------------------------------------------------------------
  Ticket             | 13231
  Updated By         | Michael Slusarz <slusarz at horde.org>
  Summary            | Hashtable session handler doesn't unlock on session
                     | close
  Queue              | Horde Framework Packages
  Version            | Git master
-Type               | Bug
+Type               | Enhancement
-State              | Feedback
+State              | Assigned
-Priority           | 1. Low
+Priority           | 1. Low
  Milestone          |
  Patch              |
  Owners             |
------------------------------------------------------------------------------


Michael Slusarz <slusarz at horde.org> (2014-05-29 13:42) wrote:

>> This doesn't help.  As I noted above, 10 requests/second is already
>> fairly heavy usage.  Your solution bumps that up to 20
>> requests/second (you have to assume worst-case scenario).
>
> No, you don't. The requests will follow a Poisson distribution,  
> assuming mt_rand is not biased towards certain values

It is.  (although not for the numbers you gave).

> So the with increasing number of requests, the average will quickly  
> stabilise towards 10 requests/second.

Not really.  10-20 requests is going to show bias one way or the  
other.  Sure enough, running a sample size of 20 mt_rand() calls  
created an average time of 82806ns.  18% from the expected "median" is  
not "stabilizing" in that sample size.  And if we are running  
thousands/millions of retries, the problem is not in the random  
distribution but rather in the session access in the first place.

And the original problem remains: you are potentially delaying the  
**normal** scenario - a single request is queued - in an attempt to  
make the outlier work better - since you potentially increase this  
common case to 50% slower than it currently is.  We need to optimize  
for the normal scenario.  I'm going to commit a slightly better  
algorithm for this purpose, by ramping up to the the 0.1 second delay.

>> And there is a general (albeit not mandatory) requirement that
>> requests SHOULD be handled in a FIFO basis. If Kronolith for some
>> reason sends 15 listEvents requests in a row, it is a reasonable
>> assumption that the 15th is the least important request.  It should
>> not, by virtue of random luck, suddenly jump to 2nd in the queue.
>
> There is no way (whatever method you use) if these requests are  
> handled by separate processes and it is required to get a lock on a  
> shared resource
> If that is needed, a new request can only be initiated when the  
> previous one is know to be active (not waiting to get a lock on a  
> resource). Since this is not done, requests can (and will be) served  
> in random order.

Only for the memcache sessionhandler - or at least a poll-based  
locking handler instead of an event-based handler.  WHICH IS EXPECTED  
(and one of the reasons why we don't recommend memcache  
sessionhandler).  That doesn't mean the assumption doesn't hold ...  
and the fact that the other sessionhandlers do support this  
generalization.

Randomizing the queue order is something you do to prevent deadlocks.   
It doesn't do anything to make a single access any faster.  In some  
instances it might be faster than the current strategy, but in some  
instances it might be worse.

The limiting factor remains the median polling time.

I'm with Mike in that there definitely is no bug here - this is an  
enhancement request at best.  It's possible that the 0.1 sec poll time  
is not ideal - but I'm not going to change that without evidence to  
prove a better value, especially since the sessionhandler works and  
the performance issues are known and have been known since the  
beginning.