GetSnapshotData never needs exclusive lock on ProcArrayLock

I was thinking about the business of capturing subtransaction XIDs
during GetSnapshotData so as to reduce the need for visits to
pg_subtrans, as implemented by Takahiro-san here:
http://archives.postgresql.org/pgsql-patches/2006-08/msg00401.php
The principal objection to this, I think, is the longer time spent
holding the ProcArrayLock while grabbing a snapshot. That wouldn't be
so bad if the lock were shared, but we hold it exclusive while taking
the first "serializable" snap of a transaction. It strikes me though
that this is excessive paranoia: it should be safe to take the lock
shared even while setting a serializable snap. I believe the reasoning
for using an exclusive lock was simply "we are changing a shared data
structure (by setting MyProc->xmin), therefore it oughta be exclusive".
However:

* No backend ever writes another's MyProc->xmin, so there is no danger
of write conflicts.

* We already assume that fetching/storing of TransactionId is atomic,
so there shouldn't be any problem with reading a partially updated
value. Onlooker backends should always read either zero or the correct
xmin, and they are already coded to do the right thing with zero ---
it just means "this xact hasn't set its xmin yet".

The only other assumption that I can see here is the one documented in
GetOldestXmin: if another backend has not yet set its xmin, then when it
does, it cannot set xmin less than the one we compute. This is
trivially true if backends are forced to set their serializable
snapshots one at a time, but if we allow two backends to execute
GetSnapshotData at the same time, ISTM that they must in fact compute
the same value of xmin, so there's no problem. Two backends concurrently
running GetSnapshotData must already have stored their own XIDs into
their PGPROC structures, so the xmins they compute will certainly be no
larger than the older of these XIDs. The only way they could compute
different xmins is if some even-older XID leaves the set of running
transactions meanwhile --- but this is impossible because we require
backends to take ProcArrayLock exclusively when removing an XID from the
proc array. The two will therefore compute identical xmins, whether or
not either one is slow enough to see the other's computed value appear
in the proc array.

I propose therefore that we make GetSnapshotData use a shared lock
always for ProcArrayLock.

Anyone see a hole in this analysis?

regards, tom lane