Tom Lane wrote:
> Heikki Linnakangas <heikki@enterprisedb.com> writes:
>> BTW: Should we do the synchronization in the non-page-at-a-time mode?
>> It's not many lines of code to do so, but IIRC that codepath is only
>> used for catalog access. System tables really shouldn't grow that big,
>> and if they do we shouldn't be doing seq scans for them anyway.
>
> It occurs to me that there's an actual bug here for catalog access.
> The code assumes that it can measure rs_nblocks only once and not worry
> about tuples added beyond that endpoint. But this is only true when
> using an MVCC-safe snapshot. In SnapshotNow mode, in particular, this
> could easily lead to missing a valid tuple. I'm not sure such a bug
> has ever been seen in the wild, because we don't do that many heapscans
> on system catalogs (and most of the ones we do do have some amount of
> higher-level locking preventing a concurrent update). But we ought to
> fix it while we're touching the code now. That will take re-measuring
> rs_nblocks each time we consider stopping, and I think we definitely
> will NOT want the syncscan code getting involved.
You would only miss tuples inserted after you began the seqscan. After
you've began the scan, you're going to miss any tuples that are inserted
before the current position anyway, so stopping the scan early shouldn't
do any real harm. It would only be a problem if you do something like:
heap_beginscan(...)
Lock
while(heap_getnext) ...
Unlock
And expect to see all tuples inserted before acquiring the lock.
Good catch, though. Fixing it is probably a good idea even if it's not a
problem for any existing code.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com