Mailing List Archive

Re: svn commit: r1904299 - /httpd/httpd/trunk/modules/http2/h2_workers.c
Le 27/09/2022 à 13:00, icing@apache.org a écrit :
> Author: icing
> Date: Tue Sep 27 11:00:10 2022
> New Revision: 1904299
>
> URL: http://svn.apache.org/viewvc?rev=1904299&view=rev
> Log:
> *) mod_http2: use proper apr_time_t where it is due, no (int) casting.
>
>
> Modified:
> httpd/httpd/trunk/modules/http2/h2_workers.c
>
> Modified: httpd/httpd/trunk/modules/http2/h2_workers.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_workers.c?rev=1904299&r1=1904298&r2=1904299&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/http2/h2_workers.c (original)
> +++ httpd/httpd/trunk/modules/http2/h2_workers.c Tue Sep 27 11:00:10 2022
> @@ -78,7 +78,7 @@ struct h2_workers {
>
> apr_uint32_t max_slots;
> apr_uint32_t min_active;
> - volatile int idle_limit;
> + volatile apr_time_t idle_limit;
> volatile int aborted;
> volatile int shutdown;
> int dynamic;
> @@ -315,7 +315,7 @@ static void* APR_THREAD_FUNC slot_run(ap
> APR_RING_INSERT_TAIL(&workers->idle, slot, h2_slot, link);
> ++workers->idle_slots;
> slot->is_idle = 1;
> - if (slot->id >= workers->min_active && workers->idle_limit) {
> + if (slot->id >= workers->min_active && workers->idle_limit > 0) {
> rv = apr_thread_cond_timedwait(slot->more_work, workers->lock,
> workers->idle_limit);
> if (APR_TIMEUP == rv) {
> @@ -416,7 +416,8 @@ static apr_status_t workers_pool_cleanup
> }
>
> h2_workers *h2_workers_create(server_rec *s, apr_pool_t *pchild,
> - int max_slots, int min_active, apr_time_t idle_limit)
> + int max_slots, int min_active,
> + apr_time_t idle_limit)
> {
> apr_status_t rv;
> h2_workers *workers;
> @@ -453,7 +454,7 @@ h2_workers *h2_workers_create(server_rec
> workers->pool = pool;
> workers->min_active = min_active;
> workers->max_slots = max_slots;
> - workers->idle_limit = (int)((idle_limit > 0)? idle_limit : apr_time_from_sec(10));
> + workers->idle_limit = (idle_limit > 0)? idle_limit : apr_time_from_sec(10);

Hi,

just wondering if this 10s is related to the default value of
H2MaxWorkerIdleSeconds?

If yes, the doc says 600 (unless I missed something obvious).

CJ


> workers->dynamic = (workers->min_active < workers->max_slots);
>
> ap_log_error(APLOG_MARK, APLOG_INFO, 0, s,
>
>
>
Re: svn commit: r1904299 - /httpd/httpd/trunk/modules/http2/h2_workers.c [ In reply to ]
> Am 30.09.2022 um 20:51 schrieb Christophe JAILLET <christophe.jaillet@wanadoo.fr>:
>
> Le 27/09/2022 à 13:00, icing@apache.org a écrit :
>> Author: icing
>> Date: Tue Sep 27 11:00:10 2022
>> New Revision: 1904299
>> URL: http://svn.apache.org/viewvc?rev=1904299&view=rev
>> Log:
>> *) mod_http2: use proper apr_time_t where it is due, no (int) casting.
>> Modified:
>> httpd/httpd/trunk/modules/http2/h2_workers.c
>> Modified: httpd/httpd/trunk/modules/http2/h2_workers.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_workers.c?rev=1904299&r1=1904298&r2=1904299&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/http2/h2_workers.c (original)
>> +++ httpd/httpd/trunk/modules/http2/h2_workers.c Tue Sep 27 11:00:10 2022
>> @@ -78,7 +78,7 @@ struct h2_workers {
>> apr_uint32_t max_slots;
>> apr_uint32_t min_active;
>> - volatile int idle_limit;
>> + volatile apr_time_t idle_limit;
>> volatile int aborted;
>> volatile int shutdown;
>> int dynamic;
>> @@ -315,7 +315,7 @@ static void* APR_THREAD_FUNC slot_run(ap
>> APR_RING_INSERT_TAIL(&workers->idle, slot, h2_slot, link);
>> ++workers->idle_slots;
>> slot->is_idle = 1;
>> - if (slot->id >= workers->min_active && workers->idle_limit) {
>> + if (slot->id >= workers->min_active && workers->idle_limit > 0) {
>> rv = apr_thread_cond_timedwait(slot->more_work, workers->lock,
>> workers->idle_limit);
>> if (APR_TIMEUP == rv) {
>> @@ -416,7 +416,8 @@ static apr_status_t workers_pool_cleanup
>> }
>> h2_workers *h2_workers_create(server_rec *s, apr_pool_t *pchild,
>> - int max_slots, int min_active, apr_time_t idle_limit)
>> + int max_slots, int min_active,
>> + apr_time_t idle_limit)
>> {
>> apr_status_t rv;
>> h2_workers *workers;
>> @@ -453,7 +454,7 @@ h2_workers *h2_workers_create(server_rec
>> workers->pool = pool;
>> workers->min_active = min_active;
>> workers->max_slots = max_slots;
>> - workers->idle_limit = (int)((idle_limit > 0)? idle_limit : apr_time_from_sec(10));
>> + workers->idle_limit = (idle_limit > 0)? idle_limit : apr_time_from_sec(10);
>
> Hi,
>
> just wondering if this 10s is related to the default value of H2MaxWorkerIdleSeconds?
>
> If yes, the doc says 600 (unless I missed something obvious).

The default in the config handling is indeed 600s and that is used when creating the workers. The code here is some leftover from earlier days.

That should be replaced with an assert and the config should only accept values > 0.

>
> CJ
>
>
>> workers->dynamic = (workers->min_active < workers->max_slots);
>> ap_log_error(APLOG_MARK, APLOG_INFO, 0, s,
>