Hard coded time value in apr_memcache.c

[Ruediger Pluem <rp...@apache.org>]

I am a little bit bothered by the following hardcoded time in memcache/apr_memcache.c::apr_memcache_find_server_hash_default

            if (curtime - ms->btime >  apr_time_from_sec(5)) {
                ms->btime = curtime;
                if (mc_version_ping(ms) == APR_SUCCESS) {
                    make_server_live(mc, ms);
#if APR_HAS_THREADS
                    apr_thread_mutex_unlock(ms->lock);
#endif
                    break;
                }
            }

I want to make this configurable. I can think of the following three options:

1. Create an apr_memcache_create_ex that allows to set this value.
2. Create a new setter / getter function to allow to set / get this for a previously created apr_memcache_t.
3. Encourage people to directly change the field that holds the time information in the public struct apr_memcache_t for
   a previously created apr_memcache_t.

What makes most sense from an API point of view?

Regards

Rüdiger

Re: Hard coded time value in apr_memcache.c

[Christophe JAILLET <ch...@wanadoo.fr>]

Le 10/05/2023 à 17:20, Ruediger Pluem a écrit :
> I am a little bit bothered by the following hardcoded time in memcache/apr_memcache.c::apr_memcache_find_server_hash_default
> 
>              if (curtime - ms->btime >  apr_time_from_sec(5)) {
>                  ms->btime = curtime;
>                  if (mc_version_ping(ms) == APR_SUCCESS) {
>                      make_server_live(mc, ms);
> #if APR_HAS_THREADS
>                      apr_thread_mutex_unlock(ms->lock);
> #endif
>                      break;
>                  }
>              }
> 
> I want to make this configurable. I can think of the following three options:
> 
> 1. Create an apr_memcache_create_ex that allows to set this value.
> 2. Create a new setter / getter function to allow to set / get this for a previously created apr_memcache_t.
> 3. Encourage people to directly change the field that holds the time information in the public struct apr_memcache_t for
>     a previously created apr_memcache_t.
> 
> What makes most sense from an API point of view?
> 
> Regards
> 
> Rüdiger
> 
> 

Hi,

personally, I think that 2. is the cleaner way to go.

CJ