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(at)ak(dot)jp(dot)nec(dot)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(at)ak(dot)jp(dot)nec(dot)com>