Re: svn commit: r1331110 - in /httpd/httpd/trunk: ./ modules/cache/ modules/dav/fs/ modules/filters/ modules/generators/ modules/loggers/ modules/mappers/ modules/slotmem/ support/
On Thu, Apr 26, 2012 at 09:44:52PM -, s...@apache.org wrote: Author: sf Date: Thu Apr 26 21:44:51 2012 New Revision: 1331110 URL: http://svn.apache.org/viewvc?rev=1331110view=rev Log: Replace use of apr_file_write() with apr_file_write_full() to prevent incomplete writes. Add comments in some places where error handling/logging is missing. I just saw this in STATUS. I think this isn't right for mod_log_*, which should use a single write() call per log line, and fail in all error case. Using _write_full() means we lose the guarantee of an atomic write. Regards, Joe
Re: [Discuss] mod_lua and database connectivity
On 01/06/2013 07:21 PM, Daniel Gruno wrote: On 01/06/2013 04:23 PM, Stefan Fritsch wrote: On Sun, 6 Jan 2013, Daniel Gruno wrote: -- Regular prepared statement creation: local prepped, err = db:prepare(SELECT * FROM `tbl` WHERE `id` = %s) if not err then local result, err = prepped(foobar) -- insert foobar as id ... end -- Fetch from DBDPrepareSQL: local prepped, err = db:prepared(someTag) if not err then local result, err = prepped(foobar) -- insert foobar as id ... end One last change before I commit the code; prepared statements now have two functions identical to the database object; select and query. These were formerly known as query and run in the db object, but I have renamed them to comply with the naming conventions of apr_dbd, thus 'query' is for running a command and fetching the no. of rows affected, and 'select' is for...selecting :) So, prepared statements are now run as follows: -- select stuff local prepared, err = db:prepare(SELECT * FROM `tbl` WHERE `age` %u) if not err then local result, err = prepared:select(1234) end -- run stuff local prepared, err = db:prepare(DELETE FROM `tbl` WHERE `age` %u) if not err then local result, err = prepared:query(1234) end With regards, Daniel.
Re: Balancer persist and inherit stuff in trunk
re: increased performance. True enough, but we should also be aware that the vast majority of people are still running 2.2.x and that httpd is still seen as a slow, bloated monster compared to nginx, which is gradually being seen as the web-server for the web. On Jan 7, 2013, at 5:36 PM, Stefan Fritsch s...@sfritsch.de wrote: On Sunday 06 January 2013, Jim Jagielski wrote: We could, but it's not my personal pref as RM... After all, ApacheCon will be here before we know it :/ That's probably true ;) FWIW, I see the this dynamic nature of 2.4 as much as a killer feature as the increased performance as related to comparisons between httpd and other web-servers out there, including the fanboys latest flavor. Hence my desire to try to have this in asap, so I can re-dive into the optimized event/simple- MPM hybrid I was working on, which itself I hope to have done by ACNA2013. BTW, I don't believe the increased performance in 2.4 should be overrated. It gives some nice improvements with some workloads, but there are still plenty of cases where scalability remains bad (e.g. ssl or long-lasting proxy-requests).
Re: Balancer persist and inherit stuff in trunk
+1 for both being reset... On Jan 7, 2013, at 4:40 AM, Rainer Jung rainer.j...@kippdata.de wrote: On 06.01.2013 17:48, Jim Jagielski wrote: I had thought that I has responded to that orig email that both below issues where by design but could be adjusted if need be or desired. I have no opinions either way, but to be Used seems more session based and Elected is more long-term statistical. Hmmm, but the Used field should give information about shm slots occupied by configured balancers. Isn't the info (=0) wrong after a restart? Concerning the long term statistical values I'm undecided like you, but I think we should handle them in a consistent way, so traffic and elected should either be both persisted or both reset. Regards, Rainer On Jan 5, 2013, at 1:41 PM, Rainer Jung rainer.j...@kippdata.de wrote: On 04.01.2013 19:48, Jim Jagielski wrote: Have people had a chance to test, review and try the balancer persist and inheritance stuff in trunk? I want to make sure that we have some level of verification and agreement there before I work on the backports for 2.4 ;) I didn't see any changes with respect to two of my original comments: The Used counters (number of shared mem slots) in balancer manager drops to 0 after restart/reboot´with persist on. This seems strange. Not that Used does *not* have anything to do with request counting. Second the Elected counter is persisted but e.g. the traffic counter's not. This seems somewhat inconsistent. I have no strong opinion whether to persist statistics or not, but we might want to behave consistently. I did not observe any functional changes after my Mail from Dec. 14, right? Regards, Rainer
Re: Balancer persist and inherit stuff in trunk
Hold on a tic... let me go thru my notes. I seem to recall an issue I hit. On Jan 8, 2013, at 9:28 AM, Jim Jagielski j...@jagunet.com wrote: +1 for both being reset... On Jan 7, 2013, at 4:40 AM, Rainer Jung rainer.j...@kippdata.de wrote: On 06.01.2013 17:48, Jim Jagielski wrote: I had thought that I has responded to that orig email that both below issues where by design but could be adjusted if need be or desired. I have no opinions either way, but to be Used seems more session based and Elected is more long-term statistical. Hmmm, but the Used field should give information about shm slots occupied by configured balancers. Isn't the info (=0) wrong after a restart? Concerning the long term statistical values I'm undecided like you, but I think we should handle them in a consistent way, so traffic and elected should either be both persisted or both reset. Regards, Rainer On Jan 5, 2013, at 1:41 PM, Rainer Jung rainer.j...@kippdata.de wrote: On 04.01.2013 19:48, Jim Jagielski wrote: Have people had a chance to test, review and try the balancer persist and inheritance stuff in trunk? I want to make sure that we have some level of verification and agreement there before I work on the backports for 2.4 ;) I didn't see any changes with respect to two of my original comments: The Used counters (number of shared mem slots) in balancer manager drops to 0 after restart/reboot´with persist on. This seems strange. Not that Used does *not* have anything to do with request counting. Second the Elected counter is persisted but e.g. the traffic counter's not. This seems somewhat inconsistent. I have no strong opinion whether to persist statistics or not, but we might want to behave consistently. I did not observe any functional changes after my Mail from Dec. 14, right? Regards, Rainer
Re: Balancer persist and inherit stuff in trunk
Yeppers... actually, transferred and read *are* persisted, it's just that the bytraffic lbmethod is smart enough to know that when the config is changed, those counters need to be reset and, by default, a restart is a change. Since elected is never used for any sort of lbmethod, it never is reset and so the persist holds. So the idea is that lbmethods should reset any potential persisted values iff it makes sense for that lbmethod. bytraffic resets transferred/read but byrequests doesn't, for example. On Jan 8, 2013, at 9:49 AM, Jim Jagielski j...@jagunet.com wrote: Hold on a tic... let me go thru my notes. I seem to recall an issue I hit. On Jan 8, 2013, at 9:28 AM, Jim Jagielski j...@jagunet.com wrote: +1 for both being reset... On Jan 7, 2013, at 4:40 AM, Rainer Jung rainer.j...@kippdata.de wrote: On 06.01.2013 17:48, Jim Jagielski wrote: I had thought that I has responded to that orig email that both below issues where by design but could be adjusted if need be or desired. I have no opinions either way, but to be Used seems more session based and Elected is more long-term statistical. Hmmm, but the Used field should give information about shm slots occupied by configured balancers. Isn't the info (=0) wrong after a restart? Concerning the long term statistical values I'm undecided like you, but I think we should handle them in a consistent way, so traffic and elected should either be both persisted or both reset. Regards, Rainer On Jan 5, 2013, at 1:41 PM, Rainer Jung rainer.j...@kippdata.de wrote: On 04.01.2013 19:48, Jim Jagielski wrote: Have people had a chance to test, review and try the balancer persist and inheritance stuff in trunk? I want to make sure that we have some level of verification and agreement there before I work on the backports for 2.4 ;) I didn't see any changes with respect to two of my original comments: The Used counters (number of shared mem slots) in balancer manager drops to 0 after restart/reboot´with persist on. This seems strange. Not that Used does *not* have anything to do with request counting. Second the Elected counter is persisted but e.g. the traffic counter's not. This seems somewhat inconsistent. I have no strong opinion whether to persist statistics or not, but we might want to behave consistently. I did not observe any functional changes after my Mail from Dec. 14, right? Regards, Rainer
Re: Balancer persist and inherit stuff in trunk
Rainer, does the below address your concern and make it OK that the current behavior acts the way it does and we're OK to keep it that way. On Jan 8, 2013, at 10:25 AM, Jim Jagielski j...@jagunet.com wrote: Yeppers... actually, transferred and read *are* persisted, it's just that the bytraffic lbmethod is smart enough to know that when the config is changed, those counters need to be reset and, by default, a restart is a change. Since elected is never used for any sort of lbmethod, it never is reset and so the persist holds. So the idea is that lbmethods should reset any potential persisted values iff it makes sense for that lbmethod. bytraffic resets transferred/read but byrequests doesn't, for example. On Jan 8, 2013, at 9:49 AM, Jim Jagielski j...@jagunet.com wrote: Hold on a tic... let me go thru my notes. I seem to recall an issue I hit. On Jan 8, 2013, at 9:28 AM, Jim Jagielski j...@jagunet.com wrote: +1 for both being reset... On Jan 7, 2013, at 4:40 AM, Rainer Jung rainer.j...@kippdata.de wrote: On 06.01.2013 17:48, Jim Jagielski wrote: I had thought that I has responded to that orig email that both below issues where by design but could be adjusted if need be or desired. I have no opinions either way, but to be Used seems more session based and Elected is more long-term statistical. Hmmm, but the Used field should give information about shm slots occupied by configured balancers. Isn't the info (=0) wrong after a restart? Concerning the long term statistical values I'm undecided like you, but I think we should handle them in a consistent way, so traffic and elected should either be both persisted or both reset. Regards, Rainer On Jan 5, 2013, at 1:41 PM, Rainer Jung rainer.j...@kippdata.de wrote: On 04.01.2013 19:48, Jim Jagielski wrote: Have people had a chance to test, review and try the balancer persist and inheritance stuff in trunk? I want to make sure that we have some level of verification and agreement there before I work on the backports for 2.4 ;) I didn't see any changes with respect to two of my original comments: The Used counters (number of shared mem slots) in balancer manager drops to 0 after restart/reboot´with persist on. This seems strange. Not that Used does *not* have anything to do with request counting. Second the Elected counter is persisted but e.g. the traffic counter's not. This seems somewhat inconsistent. I have no strong opinion whether to persist statistics or not, but we might want to behave consistently. I did not observe any functional changes after my Mail from Dec. 14, right? Regards, Rainer
Re: thread-safety of time conversion caches in util_time.c and mod_log_config.c
It looks like apr_atomic is not in good-enough shape to be used for
lock-free algorithms. It's probably good enough for the use-cases that the
event mpm uses it for, because the event mpm is not using it for protecting
memory besides the counters it's atomically changing. However, the time
caches would use the cache key to protect other memory (the cache value),
so apr_atomic is not good enough, because it's inconsistent on the
different platforms with respect to memory barriers.
For example, with x86, the add32 and cas32 functions act as a full memory
barrier on all memory. However, for powerpc, the add32 and cas32 only acts
as a memory barrier on the individual item being changed, it doesn't act as
a memory barrier on all memory. One would have to add lightweight sync
assembler instructions to the add32 and cas32 function implementations for
powerpc if you want the same semantics as x86.
The problem is that the apr_atomic API is not explicit with the memory
ordering semantics. I agree with you that in the future the apr_atomic API
should be changed to be more like the C11 atomic API, which allows explicit
choice of memory ordering.
But that leaves us with the problem of the current time caching code, which
is not thread-safe. We should fix it using the existing APR APIs, and not
use the apr_atomic API which is broken for this use-case.
I found that there is a lock-free method available to us with the current
APR API. We can use apr_thread_mutex_trylock. In glibc, that just
attempts to cmpxchg the userspace futex, and if it fails, it doesn't do a
futex system call and a wait on the caller, so it's lock free. On Windows,
TryEnterCriticalSection probably works the same way.
So, the pseudo-code would be something like:
For reading from cache:
apr_uint32_t key = cache_element-key;
/* Above is done speculatively */
if (seconds == key seconds != 0) {
/* seconds == 0 may mean cache is uninitialized, so don't use cache
*/
*xt = cache_element-xt;
/* After copying xt, make sure cache_element was not marked invalid
* by another thread beginning an update, and that cache_element
* really contained data for our second. */
if (apr_thread_mutex_trylock(cache_element-mutex) == APR_SUCCESS) {
key = cache_element-key;
apr_thread_mutex_unlock(cache_element-mutex);
if (key == seconds) {
xt-tm_usec = (int)apr_time_usec(t);
return APR_SUCCESS;
}
}
}
/* calculate the value if reached here */
Write to cache code:
if (apr_thread_mutex_trylock(cache_element-mutex) == APR_SUCCESS)
{ /* We won the race to update this cache_element. */
cache_element-key = ~seconds;
/* Above marks cache_element as invalid by using ~seconds,
* for the speculative reads not using the mutex. */
cache_element-xt = *xt;
/* Finished copying, now update key with our key */
cache_element-key = seconds;
apr_thread_mutex_unlock(cache_element-mutex);
}
The above is lock-free, because process scheduling decisions by the OS
won't cause the current thread to block, it'll keep on doing work without
blocking as long as this thread is scheduled. It would be fast, though
perhaps not quite as fast than if we had an atomic api we could use
effectively. With an atomic API, the read-from-time-cache code would only
be reading from cache-lines, not writing to cache-lines with the futex's
cmpxchg instruction. You might also be able to use a lighter-weight memory
barrier. But the mutex is lock-free and fast, and it's something that'd
work with today's APIs; and the existing code in production is not correct.
BTW, I looked at rwlock_trylock, and that isn't lock-free in glibc, because
the readers/writers count variables are protected with a low-level lock
futex that is acquired with lll_lock, not lll_trylock. So, if a thread
acquires the low-level lock, but is unscheduled by the OS before it
finishes updating the reader/writers count variables and unlocks the
low-level lock, that would block other threads from acquiring the
low-level lock, and other threads would block before being able to access
the readers/writers count variables. If it had used lll_trylock instead, I
think it would've been lock-free. However, I don't think rwlock would give
us any particular advantage than a regular mutex for the above algorithm.
So using the simpler mutex is better and is lock-free. I think the
important part is to have a lock-free algorithm: if we lose a little bit of
performance by not being able to use the lightest lock-free algorithm, I
think that is OK.
On Mon, Jan 7, 2013 at 5:50 PM, Stefan Fritsch s...@sfritsch.de wrote:
On Sunday 06 January 2013, Daniel Lescohier wrote:
I'm not sure we should include memory barriers inside the
apr_atomic_read32 and apr_atomic_set32 functions, because
Re: thread-safety of time conversion caches in util_time.c and mod_log_config.c
Some timing tests:
gcc -I/usr/include/apr-1 -lrt -lapr-1 -lpthread test.c
./a.out
apr_time_exp_lt: 108 nanosecs per call
localtime_r: 85 nanosecs per call
apr_time_exp_gmt: 59 nanosecs per call
gmtime_r: 35 nanosecs per call
pthread_mutex_trylock/unlock/read cache: 19 nanosecs per call
lfence/read cache: 8 nanosecs per call
read cache: 4 nanosecs per call
cat test.c
#include time.h
#include stdio.h
#include assert.h
#include apr_time.h
#include pthread.h
int main (int argc, char *argv[]) {
struct timespec old, new;
struct tm tmv;
int i, iterations=1000;
long nanosecs;
#define CALC_NS(new, old) ((new.tv_sec - old.tv_sec)*10 +
new.tv_nsec - old.tv_nsec)
apr_time_t t = apr_time_now();
apr_time_exp_t xt;
pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
unsigned int seconds=1;
typedef struct { char s[44]; unsigned int key;} data_t;
static volatile data_t cache_data;
data_t my_copy;
cache_data.key = 1;
assert(clock_gettime(CLOCK_MONOTONIC_RAW, old)==0);
for (i=0; i iterations; ++i) {
apr_time_exp_lt(xt, t);
}
assert(clock_gettime(CLOCK_MONOTONIC_RAW, new)==0);
nanosecs = CALC_NS(new, old);
printf(apr_time_exp_lt: %d nanosecs per call\n, nanosecs/iterations);
assert(clock_gettime(CLOCK_MONOTONIC_RAW, old)==0);
for (i=0; i iterations; ++i) {
localtime_r(old.tv_sec, tmv);
}
assert(clock_gettime(CLOCK_MONOTONIC_RAW, new)==0);
nanosecs = CALC_NS(new, old);
printf(localtime_r: %d nanosecs per call\n, nanosecs/iterations);
assert(clock_gettime(CLOCK_MONOTONIC_RAW, old)==0);
for (i=0; i iterations; ++i) {
apr_time_exp_gmt(xt, t);
}
assert(clock_gettime(CLOCK_MONOTONIC_RAW, new)==0);
nanosecs = CALC_NS(new, old);
printf(apr_time_exp_gmt: %d nanosecs per call\n, nanosecs/iterations);
assert(clock_gettime(CLOCK_MONOTONIC_RAW, old)==0);
for (i=0; i iterations; ++i) {
gmtime_r(old.tv_sec, tmv);
}
assert(clock_gettime(CLOCK_MONOTONIC_RAW, new)==0);
nanosecs = CALC_NS(new, old);
printf(gmtime_r: %d nanosecs per call\n, nanosecs/iterations);
assert(clock_gettime(CLOCK_MONOTONIC_RAW, old)==0);
for (i=0; i iterations; ++i) {
int key = cache_data.key;
if (seconds==key key!=0) {
my_copy = cache_data;
if (pthread_mutex_trylock(mutex)==0) {
key = cache_data.key;
pthread_mutex_unlock(mutex);
}
}
}
assert(clock_gettime(CLOCK_MONOTONIC_RAW, new)==0);
nanosecs = CALC_NS(new, old);
printf(pthread_mutex_trylock/unlock/read cache: %d nanosecs per
call\n, nanosecs/iterations);
assert(clock_gettime(CLOCK_MONOTONIC_RAW, old)==0);
for (i=0; i iterations; ++i) {
int key = cache_data.key;
if (seconds==key key!=0) {
my_copy = cache_data;
asm volatile(lfence:::memory);
key = cache_data.key;
}
}
assert(clock_gettime(CLOCK_MONOTONIC_RAW, new)==0);
nanosecs = CALC_NS(new, old);
printf(lfence/read cache: %d nanosecs per call\n,
nanosecs/iterations);
assert(clock_gettime(CLOCK_MONOTONIC_RAW, old)==0);
for (i=0; i iterations; ++i) {
int key = cache_data.key;
if (seconds==key key!=0) {
my_copy = cache_data;
key = cache_data.key;
}
}
assert(clock_gettime(CLOCK_MONOTONIC_RAW, new)==0);
nanosecs = CALC_NS(new, old);
printf(read cache: %d nanosecs per call\n, nanosecs/iterations);
}
On Tue, Jan 8, 2013 at 1:50 PM, Daniel Lescohier
daniel.lescoh...@cbsi.comwrote:
It looks like apr_atomic is not in good-enough shape to be used for
lock-free algorithms. It's probably good enough for the use-cases that the
event mpm uses it for, because the event mpm is not using it for protecting
memory besides the counters it's atomically changing. However, the time
caches would use the cache key to protect other memory (the cache value),
so apr_atomic is not good enough, because it's inconsistent on the
different platforms with respect to memory barriers.
For example, with x86, the add32 and cas32 functions act as a full memory
barrier on all memory. However, for powerpc, the add32 and cas32 only acts
as a memory barrier on the individual item being changed, it doesn't act as
a memory barrier on all memory. One would have to add lightweight sync
assembler instructions to the add32 and cas32 function implementations for
powerpc if you want the same semantics as x86.
The problem is that the apr_atomic API is not explicit with the memory
ordering semantics. I agree with you that in the future the apr_atomic API
should be changed to be more like the C11 atomic API, which allows explicit
choice of memory ordering.
But that leaves us with the problem of the current time caching code,
which is not thread-safe. We should fix it using the existing APR APIs,
and not use the
Re: svn commit: r1430590 - /httpd/httpd/branches/2.4.x/STATUS
On 01/08/2013 11:40 PM, j...@apache.org wrote: Author: jim Date: Tue Jan 8 22:40:29 2013 New Revision: 1430590 URL: http://svn.apache.org/viewvc?rev=1430590view=rev Log: BalancerInherit proposal Modified: httpd/httpd/branches/2.4.x/STATUS Modified: httpd/httpd/branches/2.4.x/STATUS URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1430590r1=1430589r2=1430590view=diff == --- httpd/httpd/branches/2.4.x/STATUS (original) +++ httpd/httpd/branches/2.4.x/STATUS Tue Jan 8 22:40:29 2013 @@ -176,6 +176,20 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK: 2.4.x patch: trunk patch works + CHANGES to be added +1: jailletc36, jim + * mod_proxy: Allow balancers to be server-specific, as they should have + been. Inheritance causes too many behind-the-scene interactions + to be reliable in a dynamic environ. We maintain the old-default + of inheritance. + trunk patch: http://svn.apache.org/viewvc?view=revisionrevision=1387603 + http://svn.apache.org/viewvc?view=revisionrevision=1388029 + http://svn.apache.org/viewvc?view=revisionrevision=1420124 + http://svn.apache.org/viewvc?view=revisionrevision=1421288 + http://svn.apache.org/viewvc?view=revisionrevision=1421912 + http://svn.apache.org/viewvc?view=revisionrevision=1422943 + http://svn.apache.org/viewvc?view=revisionrevision=1422980 + http://svn.apache.org/viewvc?view=revisionrevision=1430575 + 2.4.x patch: http://people.apache.org/~jim/patches/proxypassinherit2.patch + +1: jim A list of further possible backports can be found at: Did you mean http://people.apache.org/~jim/patches/proxypassinherit.patch (without the '2') ? With regards, Daniel.
