Re: Improving connection scalability: GetSnapshotData()

Hi,

On 2020-04-08 09:26:42 -0400, Jonathan S. Katz wrote:
> On 4/8/20 8:59 AM, Alexander Korotkov wrote:
> > On Wed, Apr 8, 2020 at 3:43 PM Andres Freund <andres(at)anarazel(dot)de> wrote:
> >> Realistically it still 2-3 hours of proof-reading.
> >>
> >> This makes me sad :(
> >
> > Can we ask RMT to extend feature freeze for this particular patchset?
> > I think it's reasonable assuming extreme importance of this patchset.

> One of the features of RMT responsibilities[1] is to be "hands off" as
> much as possible, so perhaps a reverse ask: how would people feel about
> this patch going into PG13, knowing that the commit would come after the
> feature freeze date?

I'm obviously biased, so I don't think there's much point in responding
directly to that question. But I thought it could be helpful if I
described what my thoughts about where the patchset is:

What made me not commit it "earlier" yesterday was not that I had/have
any substantial concerns about the technical details of the patch. But
that there were a few too many comments that didn't yet sound quite
right, that the commit messages didn't yet explain the architecture
/ benefits well enough, and that I noticed that a few variable names
were too easy to be misunderstood by others.

By 5 AM I had addressed most of that, except that some technical details
weren't yet mentioned in the commit messages ([1], they are documented
in the code). I also produce enough typos / odd grammar when fully
awake, so even though I did proof read my changes, I thought that I need
to do that again while awake.

There have been no substantial code changes since yesterday. The
variable renaming prompted by Robert (which I agree is an improvement),
as well as reducing the diff size by deferring some readability
improvements (probably also a good idea) did however produce quite a few
conflicts in subsequent patches that I needed to resolve. Another awake
read-through to confirm that I resolved them correctly seemed the
responsible thing to do before a commit.

> Lastly, with the ongoing world events, perhaps time that could have been
> dedicated to this and other patches likely affected their completion. I
> know most things in my life take way longer than they used to (e.g.
> taking out the trash/recycles has gone from a 15s to 240s routine). The
> same could be said about other patches as well, but this one has a far
> greater impact (a double-edged sword, of course) given it's a feature
> that everyone uses in PostgreSQL ;)

I'm obviously not alone in that, so I agree that it's not an argument
pro/con anything.

But this definitely is the case for me. Leaving aside the general dread,
not having a quiet home-office, nor good exercise, is definitely not
helping.

I'm really bummed that I didn't have the cycles to help the shared
memory stats patch ready as well. It's clearly not yet there (but
improved a lot during the CF). But it's been around for so long, and
there's so many improvements blocked by the current stats
infrastructure...

[1] the "mirroring" of values beteween dense arrays and PGPROC, the
changed locking regimen for ProcArrayAdd/Remove, the widening of
lastCompletedXid to be a 64bit xid
[2] https://www.postgresql.org/message-id/20200407121503.zltbpqmdesurflnm%40alap3.anarazel.de

Greetings,

Andres Freund