Re: ALTER TABLE lock strength reduction patch is unsafe

On Mon, Jan 2, 2012 at 9:17 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> On Mon, Jan 2, 2012 at 7:13 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
>>> On Mon, Jan 2, 2012 at 6:06 PM, Noah Misch <noah(at)leadboat(dot)com> wrote:
>>>> On Mon, Jan 02, 2012 at 05:09:16PM +0000, Simon Riggs wrote:
>>>>> Attached patch makes SnapshotNow into an MVCC snapshot,
>>
>>>> That's a neat trick.  However, if you start a new SnapshotNow scan while one is
>>>> ongoing, the primordial scan's snapshot will change mid-stream.
>>
>>> Do we ever do that?
>>
>> Almost certainly yes.  For example, a catcache load may invoke catcache
>> or relcache reload operations on its way to opening the table or index
>> needed to fetch the desired row.
>>
>> I think you can only safely do this if each caller has its own snapshot
>> variable, a la SnapshotDirty, and that's going to be hugely more
>> invasive.
>
>
> I think I can avoid doing things that way and keep it non-invasive.
>
> If we
>   #define  SnapshotNow   (GetSnapshotNow())
>
> and make GetSnapshotNow() reuse the static SnapshotNowData if possible.
> If not available we can allocate a new snapshot in TopTransactionContext.
>
> We can free the snapshot at the end of scan.
>
> That hides most of the complexity without causing any problems, ISTM.
>
> Working on this now.

Options for implementing this are

1) add FreeSnapshotNowIfNeeded() to every heap_endscan and index_endscan
Small code footprint, touches every scan

2) change all heap_scans that uses SnapshotNow into a systable_scan
and add FreeSnapshotNowIfNeeded() to systable_endscan
35 call points, touches only system table scans

3) code specific calls to create a snapshot for each SnapshotNow call,
similar to InitDirtySnapshot
185 call points, very fine grained control, but probably overkill

(2) seems the right way to do this, with selective use of (3) and has
the side benefit of a reasonable code rationalisation

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services