On Wed, Jan 18, 2023 at 2:00 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
> On Wed, Jan 18, 2023 at 11:28:57AM -0500, Robert Haas wrote:
> > In general, looks good. I think this will often call HaveNFreeProcs
> > twice, though, and that would be better to avoid, e.g.
>
> I should have thought of this. This is fixed in v2.
Should (nfree < SuperuserReservedBackends) be using <=, or am I confused?
> > In the common case where we hit neither limit, this only counts free
> > connection slots once. We could do even better by making
> > HaveNFreeProcs have an out parameter for the number of free procs
> > actually found when it returns false, but that's probably not
> > important.
>
> Actually, I think it might be important. IIUC the separate calls to
> HaveNFreeProcs might return different values for the same input, which
> could result in incorrect error messages (e.g., you might get the
> reserved_connections message despite setting reserved_connections to 0).
> So, I made this change in v2, too.
I thought of that briefly and it didn't seem that important, but the
way you did it seems fine, so let's go with that.
What's the deal with removing "and no new replication connections will
be accepted" from the documentation? Is the existing documentation
just wrong? If so, should we fix that first? And maybe delete
"non-replication" from the error message that says "remaining
connection slots are reserved for non-replication superuser
connections"? It seems like right now the comments say that
replication connections are a completely separate pool of connections,
but the documentation and the error message make it sound otherwise.
If that's true, then one of them is wrong, and I think it's the
docs/error message. Or am I just misreading it?
--
Robert Haas
EDB: http://www.enterprisedb.com