Re: contrib: auth_delay module

(2010/11/26 11:35), Fujii Masao wrote:
> On Thu, Nov 25, 2010 at 3:18 PM, KaiGai Kohei<kaigai@ak.jp.nec.com>  wrote:
>> The attached patch is revised version.
>>
>> - Logging part within auth_delay was removed. This module now focuses on
>>   injection of a few seconds delay on authentication failed.
>> - Documentation parts were added like any other contrib modules.
>
> Something like the following is not required? Though I'm not sure
> if there is the case where auth_delay is unload.
>
> ----------------------------
> void
> _PG_fini(void)
> {
>     /* Uninstall hooks. */
>          ClientAuthentication_hook = original_client_auth_hook;
> }
> ----------------------------
>
I'm not also sure whether we have situation libraries are unloaded.
Right now, internal_unload_library() is just a placeholder, so
it seems to me _PG_fini() is never invoked.

> +    if (status != STATUS_OK)
> +    {
> +        sleep(auth_delay_seconds);
> +    }
>
> We should use pg_usleep rather than sleep?
>
Indeed, pg_usleep() is mainly used rather than sleep().

> +    DefineCustomIntVariable("auth_delay.seconds",
> +                            "Seconds to be delayed on authentication failed",
> +                            NULL,
> +                            &auth_delay_seconds,
> +                            2,
> +                            0, INT_MAX,
> +                            PGC_POSTMASTER,
> +                            GUC_UNIT_S,
> +                            NULL,
> +                            NULL);
>
> Can we relax the context from PGC_POSTMASTER to PGC_SIGHUP?
>
It seems to me reasonable change.

I'll revise my patch. How about _PG_fini()?

Thanks,
-- 
KaiGai Kohei <kaigai@ak.jp.nec.com>