Date: Fri, 19 Jan 2024 20:11:16 +0530
From: Abhijit Menon-Sen <ams@toroid.org>
To: Michael Banck <mbanck@gmx.net>
Cc: pgsql-hackers@lists.postgresql.org,
	=?utf-8?B?5oiQ5LmL54SV?= <zhcheng@ceresdata.com>
Subject: Re: [PATCH] Exponential backoff for auth_delay
Message-ID: <ZaqKDGxIJ39Mklyy@toroid.org>
References: <658c4eab.050a0220.731ff.011e@mx.google.com>
 <65965e9d.050a0220.919a6.f9e1@mx.google.com>
 <ZaYimdRlGS5tdzjy@toroid.org>
 <65aa8265.170a0220.1ecef.91df@mx.google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <65aa8265.170a0220.1ecef.91df@mx.google.com>
Archived-At: 
 <https://www.postgresql.org/message-id/ZaqKDGxIJ39Mklyy%40toroid.org>
Precedence: bulk

At 2024-01-19 15:08:36 +0100, mbanck@gmx.net wrote:
>
> I wonder whether maybe auth_delay.max_seconds should either be renamed
> to auth_delay.exponential_backoff_max_seconds (but then it is rather
> long) in order to make it clearer it only applies in that context or
> alternatively just apply to auth_delay.milliseconds as well (though
> that would be somewhat weird).

I think it's OK as-is. The description/docs are pretty clear.

> I wonder though whether this might be a useful message to have at some
> more standard level like INFO?

I don't have a strong opinion about this, but I suspect anyone who is
annoyed enough by repeated authentication failures to use auth_delay
will also be happy to have less noise in the logs about it.

> You mean something like "after 5 minutes, reset the delay to 0 again"?
> I agree that this would probably be useful, but would also make the
> change more complex.

Yes, that's the kind of thing I meant.

I agree that it would make this patch more complex, and I don't think
it's necessary to implement. However, since it's a feature that seems to
go hand-in-hand with exponential backoff in general, it _may_ be good to
mention in the docs that the sleep time for a host is reset only by
successful authentication, not by any timeout. Not sure.

> What I had in mind is that admins would lower auth_delay.milliseconds to
> something like 100 or 125 when exponential_backoff is on

Ah, that makes a lot of sense. Thanks for explaining.

Your new v3 patch looks fine to me. I'm marking it as ready for
committer.

-- Abhijit