Re: Improvement of procArray.xmin for VACUUM - Mailing list pgsql-patches

Bruce Momjian <bruce@momjian.us> writes:
> This is an optimization so I was thinking of something simpler, like
> incrementing/decrementing a counter every time we allocate/free a
> snapshot (like in the patch), and updating MyProc->xmin only if there
> are no open snapshots.  I don't think it is worth tracking anything more
> complicated than that.  Is that now possible to do cleanly?  I am having
> trouble getting this to work in the attached patch.

No kidding.

Even if you get it to pass the regression tests, it'll be a constant
source of bugs.  IMHO the *only* way to do this reliably is to have
a centralized reference-count management module.

I've been thinking more about the avoid-copy idea and I think it will
indeed work, and be simpler than what we have now, to boot.  Ideas:

* There is only one (pair of) statically allocated, max-size XID arrays
for GetSnapshotData to read into.  When we want to acquire a new snap
from GetSnapshotData, we palloc a new SnapshotData struct (and no more),
make it point at the statically allocated arrays, fill and return it.

* There can be at most one SnapshotData struct using the statically
allocated arrays.  We remember its address in a static variable (which
goes NULL when there is no such struct).  If we have to acquire a new
snapshot before the old one's refcount has gone to zero, then at that
time we palloc arrays just big enough for the old one, copy the static
data to there, insert the palloc'd array pointers into the old one,
and clear the static variable (or more likely, immediately set it to
point to the new one).

* All the current CopySnapshot operations are replaced by refcount
increments.  We need explicit refcount-decrement calls added, and
ResourceOwner support to make sure they happen.  (I'm imagining that
snapshots will now live in a dedicated context, so they don't go away
implicitly but need explicit release operations.  The ResourceOwner
infrastructure will help us find any missed releases.)

* Decrementing the refcount on a SnapshotData pfree's it if the refcount
has gone to zero, and also clears the aforementioned static pointer
variable if it happens to point at that selfsame struct.  Note that for
a serializable transaction, or a "simple" read-committed transaction
that doesn't use more than one snap at a time, this will always happen
and so there will be zero copying cost.

* In this scheme the distinction between SerializableSnapshot and
LatestSnapshot vanishes, at least at the level of memory management.
Probably SerializableSnapshot would translate into a refcount held by
xact.c or some other upper-level module.

* We keep all the live MVCC snapshots in a linked list managed by
a "snapshot cache" module.  We can traverse this list to determine
the minimum xmin at any instant.  Freeing a snap whose xmin equals
the current MyProc->xmin causes us to traverse the list, determine
the new backend-wide xmin, and update MyProc->xmin.  Freeing the
last snap causes us to reset MyProc->xmin to 0.  (I'm not completely
sure, but I think that we can just set MyProc->xmin in these cases,
without bothering to lock the ProcArray --- any comments on that?)

* GetSnapshotData no longer cares about serializable vs nonserializable
snaps, either.  Its rule is just "if MyProc->xmin is 0, then set it".
This can happen only when creating the first snap in a transaction that
currently has no live snaps.

* The above two rules clearly suffice to maintain the invariant
"MyProc->xmin is the minimum of the xmins of the live snapshots in the
transaction, or zero when there are none".  Which is what we want.


On the whole though I think we should let this idea go till 8.4; we have
a lot to deal with for 8.3 and a definite shortage of evidence that
advancing xmin will buy much.  Mu gut feeling is that the above design
would save about enough in snapshot-copying costs to pay for its extra
management logic, but we won't come out ahead unless advancing xmin
intra-transaction really helps, and I'm not sure I believe that (except
in the special case of VACUUM, and we already have a solution for that,
which would be independent of this).

            regards, tom lane