https://issues.apache.org/bugzilla/show_bug.cgi?id=56226
Yann Ylavic <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #31641|0 |1 is obsolete| | --- Comment #4 from Yann Ylavic <[email protected]> --- Created attachment 31645 --> https://issues.apache.org/bugzilla/attachment.cgi?id=31645&action=edit Use vhost's keepalive timeout in event MPM Thanks Franck for testing, however after some digging it seems that the first patch is incomplete. First, yes the post_read_request hook is enough (the sooner the better). For now I just wanted to be sure the vhost was determined, and actually it's done in a very early step of the request (before post_read). Second, all the connections in the KeepAlive state are currently kept in a single queue, the unique keepalive timeout ensuring that entries are ordered, hence to expire them we can simply look for the first entry (as the oldest) and stop iterating as soon as the current one is ok (that's why vhosts' timeouts were ignored for now, performance). But this won't work with different timeouts, for example : - vhost1 has 60s timeout, - vhost2 has 15s timeout, - request vhost1 (keep the connection alive), - request vhost2 immediately, - wait 60s (instead of 15s) for the second connection to be closed... To preserve the ordering with different (vhosts) timeouts, I think we must move from a queue, and maybe use a skiplist instead (ordered by expiration time), as for timers. But in any case it can't be as efficient as the current implementation (skiplist_insert/remove are O(log n) whereas these are O(1) on queues). That probably would have to be optional or heuristical (depending on whether vhosts have multiple keepalive timeouts or not). Moreover skiplist seems to consider only one value for a key (you can't add another value with the same key, it is ignored), which can be problematic if some expiration date collide (it's maybe already an issue for timers implementation, not sure). Anyway, I'll start a discussion on dev@ about this, to see what others think. For now I'm attaching this new patch which uses ap_hook_post_read_request() and a skiplist, still for testing. -- You are receiving this mail because: You are the assignee for the bug. --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
