Hello,

while working on
https://issues.apache.org/bugzilla/show_bug.cgi?id=56226 for a
possible way to use vhost's KeepAliveTimeout with mpm_event (by using
a skiplist instead of the actual keepalive queue), I realized that
apr_skiplists do not accept multiple values per key (unlike apr_table
for example, or std::multimap).

AFAICT this is only an implementation choice, which could be changed
with something like :
Index: tables/apr_skiplist.c
===================================================================
--- tables/apr_skiplist.c    (revision 1595631)
+++ tables/apr_skiplist.c    (working copy)
@@ -406,11 +406,7 @@ APR_DECLARE(apr_skiplistnode *) apr_skiplist_inser
         if (m->next) {
             compared = comp(data, m->next->data);
         }
-        if (compared == 0) {
-            free(stack);    /* OK. was malloc'ed */
-            return 0;
-        }
-        if ((m->next == NULL) || (compared < 0)) {
+        if (compared < 0) {
             if (ch <= nh) {
                 /* push on stack */
                 stack[stacki++] = m;

(probably with a new apr_skiplist_set_multi() which would make it controlable).

The skiplist's key used for timers by mpm_event is the expiration time
(in microseconds), I think the issue is multiple timers created at the
same time (which may happen when different threads request a timer,
eg. with event_register_[timed|socket]_callback).
In that case, none but the first one will be inserted in the skiplist
(and hence known by listener).

Shouldn't skiplists accept multiple values for the same key by default?

For 56226, if we were to replace the actual keepalive queue with a
skiplist, the proposed patch suffers the same issue.
If however the current behaviour (and speed) of using the base
server's KeepaliveTimeout for all the vhosts is a choice, I think it
should at least be documented on the KeepAliveTimeout directive, as a
limitation for mpm_event.

Regards,
Yann.

Reply via email to