Re: Inaccuracy in VACUUM's tuple count estimates - Mailing list pgsql-hackers

Andres Freund <andres@2ndquadrant.com> wrote:
> On 2014-06-09 08:00:52 -0700, Kevin Grittner wrote:

> I tried to make things a bit clearer there - but I am not sure I've
> succeed. I'm certainly willing to explain things further if you can tell
> me which are is unclear.

Thanks!  IMO, something like this should be included in the code
comments.

>     HEAPTUPLE_INSERT_IN_PROGRESS,    /* inserting xact is still in progress */
>     HEAPTUPLE_DELETE_IN_PROGRESS    /* deleting xact is still in progress */
> the current code will return INSERT_IN_PROGRESS even if the tuple has
> *also* been deleted in another xact...
> I think the problem here is that there's simply no way to really
> represent that case accurately with the current API.

For purposes of predicate.c, if the "also deleted" activity might
be rolled back without rolling back the insert, INSERT_IN_PROGRESS
is the only correct value.  If they will either both commit or
neither will commit, predicate.c would be more efficient if
HEAPTUPLE_RECENTLY_DEAD was returned, but I
HEAPTUPLE_INSERT_IN_PROGRESS would be OK from a correctness PoV.

>> Perhaps it would be good if you could provide a concise description
>> of the conditions under which value could currently be returned on
>> this (or the related) thread before we talk about what changes
>> might be needed? Maybe this is clear to others involved in the
>> discussion, but I am not confident that I fully understand what
>> gets returned under what conditions.
>
>     HEAPTUPLE_DEAD,                /* tuple is dead and deletable */
> 1) xmin has committed, xmax has committed and wasn't just a locker. Xmax
> precedes OldestXmin.

Perfect.

>     HEAPTUPLE_LIVE,                /* tuple is live (committed, no deleter) */
> 1) xmin has committed, xmax unset
> 2) xmin has committed, xmax is locked only. Status of xmax is irrelevant
> 3) xmin has committed, xmax has aborted.

Perfect.

>     HEAPTUPLE_RECENTLY_DEAD,    /* tuple is dead, but not deletable yet */
> 1) xmin has committed, xmax has committed and wasn't only a locker. But
> xmax doesn't precede OldestXmin.

For my purposes, it would be better if this also included:
 2) xmin is in progress, xmax matches (or includes) xmin

... but that would be only a performance tweak.

>     HEAPTUPLE_INSERT_IN_PROGRESS,        /* inserting xact is still in progress
> */
> new:
> 1) xmin is in progress, xmin is the current backend, xmax is invalid
> 2) xmin is in progress, xmin is the current backend, xmax only a locker
> 3) xmin is in progress, xmin is the current backend, xmax aborted
> 4) xmin is in progress, xmin is *not* current backend, xmax is irrelevant
> old:
> 1) xmin is in progress, xmax is invalid
> 2) xmin is in progress, xmax is only a locker

I think this is OK from a correctness PoV.  There may be an
opportunity to optimize.  I will look more closely at whether it
seems likely to matter much, and what sort of change in the return
conditions or in predicate.c might be needed.

>     HEAPTUPLE_DELETE_IN_PROGRESS    /* deleting xact is still in progress */
> new:
> 1) xmin has committed, xmax is in progress, xmax is not just a locker
> 2) xmin is in progress, xmin is the current backend, xmax is not just a
>   locker and in progress.

I'm not clear on how 2) could happen unless xmax is the current
backend or a subtransaction thereof.  Could you clarify?

> old:
> 1) xmin has committed, xmax is in progress, xmax is not just a locker
> 2) xmin is in progress, xmax is set and not not just a locker
>
> Note that the 2) case here never checked xmax's status.

Again, I'm not sure how 2) could happen unless they involve the
same top-level transaction.  What am I missing?

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company