Pure speculation: Actually we ERR_clear_error unconditionally so that in case of error, we can safely call SSL_get_error.
Doc says: <<<<<<<<<< >>>>>>>>>> ERR_get_error() returns the earliest error code from the thread's error queue and removes the entry. This function can be called repeatedly until there are no more error codes to return. ERR_peek_error() returns the earliest error code from the thread's error queue without modifying it. ERR_peek_last_error() returns the latest error code from the thread's error queue without modifying it. <<<<<<<<<< >>>>>>>>>> Couldn't we avoid the ERR_clear_error call (which looks expensive according to the thread), and: - loop on SSL_get_error to empty the error queue, in case an error occurred and we want la latest one? or - do a ERR_peek_last_error() / ERR_clear_error() in the error handling path (if we really care about clearing the error queue) IMHO, these 2 solutions would do the same as the current code, without requiring ERR_clear_error in the normal case. Just my 2c. I'm not an ssl/tls man. CJ On 2018/08/29 07:17:04, Ruediger Pluem <rpl...@apache.org> wrote: > Having a brief look at ERR_clear_error in openssl it looks like that various > locking operations are performed to get the > error state. I assume that this causes a lot of contention in the load case. > The question is whether there is another solution to BZ62590 but to call > ERR_clear_error before every read and write. So > far, I have no idea for an efficient check of not processed errors in the > stack and only clear if they are present / > consider them correctly for further error processing. > > Regards > > Rüdiger > > > On 08/29/2018 09:16 AM, Ruediger Pluem wrote: > > I guess it would be helpful to know which openssl version was used for the > > test. There was a valid use case for BZ62590 > > and the question is why ERR_clear_error is that expensive. But the > > expensiveness might depend on the used openssl version. > > > > Regards > > > > Rüdiger > > > > > > On 08/28/2018 04:54 PM, William A Rowe Jr wrote: > >> As we unwind various regressions and breakage, one non-lethal but somewhat > >> horrid report stands out. Eric correctly tied > >> it to the patch applied for > >> https://bz.apache.org/bugzilla/show_bug.cgi?id=62590 in the 2.4.24 > >> timeframe. > >> > >> Server Software: Apache/2.2.34 > >> SSL/TLS Protocol: TLSv1/SSLv3,AES256-GCM-SHA384,1024,256 > >> > >> vs > >> > >> Server Software: Apache/2.4.34SSL/TLS Protocol: > >> TLSv1/SSLv3,AES256-GCM-SHA384,1024,256 > >> > >> Measures out with > >> > >> Time taken for tests: 192.131 seconds > >> Total transferred: 731130414 bytes > >> HTML transferred: 88000000 bytes > >> Requests per second: 10409.59 [#/sec] (mean) > >> Time per request: 5.764 [ms] (mean) > >> Time per request: 0.096 [ms] (mean, across all concurrent requests) > >> Transfer rate: 3716.20 [Kbytes/sec] received > >> > >> vs > >> > >> Time taken for tests: 571.058 seconds > >> Total transferred: 689130083 bytes > >> HTML transferred: 90000000 bytes > >> Requests per second: 3502.27 [#/sec] (mean) > >> Time per request: 17.132 [ms] (mean) > >> Time per request: 0.286 [ms] (mean, across all concurrent requests) > >> Transfer rate: 1178.48 [Kbytes/sec] received > >> > >> > >> > >> > >> Connection Times (ms) > >> min mean[+/-sd] median max > >> Connect: 0 0 0.4 0 87 > >> Processing: 0 6 1.2 6 71 > >> Waiting: 0 6 1.2 5 70 > >> Total: 0 6 1.3 6 98 > >> > >> Percentage of the requests served within a certain time (ms) > >> 50% 6 > >> 66% 6 > >> 75% 6 > >> 80% 6 > >> 90% 7 > >> 95% 8 > >> 98% 9 > >> 99% 10 > >> 100% 98 (longest request) > >> > >> ---- > >> > >> I did then the same for Apache/2.4.34 (with apr-1.6.3 and apr-util-1.6.1), > >> with the following changes in the httpd.conf (including ssl-support): > >> StartServers 1 > >> ServerLimit 1 > >> ThreadLimit 2500 > >> ThreadsPerChild 2500 > >> ThreadStackSize 1048576 > >> MinSpareThreads 1 > >> MaxSpareThreads 500 > >> MaxRequestWorkers 2500 > >> MaxConnectionsPerChild 0 > >> > >> > >> and here the output of ApacheBench: > >> > >> ab -k -n 2000000 -c 60 'https://adnvl005:44300/' > >> This is ApacheBench, Version 2.3 <$Revision: 655654 $> > >> Copyright 1996 Adam Twiss, Zeus Technology Ltd, http://www.zeustech.net/ > >> Licensed to The Apache Software Foundation, http://www.apache.org/ > >> > >> Benchmarking adnvl005 (be patient) > >> Completed 200000 requests > >> Completed 400000 requests > >> Completed 600000 requests > >> Completed 800000 requests > >> Completed 1000000 requests > >> Completed 1200000 requests > >> Completed 1400000 requests > >> Completed 1600000 requests > >> Completed 1800000 requests > >> Completed 2000000 requests > >> Finished 2000000 requests > >> > >> > >> Connection Times (ms) > >> min mean[+/-sd] median max > >> Connect: 0 0 2.1 0 208 > >> Processing: 0 17 20.3 10 285 > >> Waiting: 0 17 20.3 10 285 > >> Total: 0 17 20.4 10 285 > >> > >> Percentage of the requests served within a certain time (ms) > >> 50% 10 > >> 66% 16 > >> 75% 23 > >> 80% 28 > >> 90% 44 > >> 95% 59 > >> 98% 79 > >> 99% 94 > >> 100% 285 (longest request) > >> > >> > >> > >> > >> ---------- Forwarded message ---------- > >> From: ** <bugzi...@apache.org <mailto:bugzi...@apache.org>> > >> Date: Mon, Aug 27, 2018 at 9:11 AM > >> Subject: [Bug 62590] performance drop after moving from apache 2.2 to > >> apache 2.4 > >> To: b...@httpd.apache.org <mailto:b...@httpd.apache.org> > >> > >> > >> https://bz.apache.org/bugzilla/show_bug.cgi?id=62590 > >> <https://bz.apache.org/bugzilla/show_bug.cgi?id=62590> > >> > >> --- Comment #1 from paolo <pa...@adnovum.ch <mailto:pa...@adnovum.ch>> --- > >> After some debug-session I found out that the problem are the > >> ERR_clear_error > >> calls in ssl_filter_write and ssl_io_input_read. If I remove those calls > >> the > >> performance is the same like with httpd/2.2. > >> > >> Are those calls really needed in the ssl_io_input_read/ssl_filter_write > >> function? > >> Isn't it enough to have it only in the ssl_io_filter_handshake function. > >> > >> Or what about to call this function only if an error occurred: > >> > >> else /* (rc < 0) */ { > >> int ssl_err = SSL_get_error(inctx->filter_ctx->pssl, rc); > >> conn_rec *c = > >> (conn_rec*)SSL_get_app_data(inctx->filter_ctx->pssl); > >> > >> + ERR_clear_error(); > >> > >> if (ssl_err == SSL_ERROR_WANT_READ) { > >> > >> > >> Many thanks for any answer. > >> > >> -- > >> You are receiving this mail because: > >> You are the assignee for the bug. > >> --------------------------------------------------------------------- > >> To unsubscribe, e-mail: bugs-unsubscr...@httpd.apache.org > >> <mailto:bugs-unsubscr...@httpd.apache.org> > >> For additional commands, e-mail: bugs-h...@httpd.apache.org > >> <mailto:bugs-h...@httpd.apache.org> > >> > >> > > >
