Re: snapshot too old, configured by time

On Wed, Mar 30, 2016 at 3:26 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> Kevin Grittner wrote:
>> On Wed, Mar 30, 2016 at 2:22 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>
>> > I said that we should change BufferGetPage into having the snapshot
>> > check built-in, except in the cases where a flag is passed; and the flag
>> > would be passed in all cases except those 30-something you identified.
>> > In other words, the behavior in all the current callsites would be
>> > identical to what's there today; we could have a macro do the first
>> > check so that we don't introduce the overhead of a function call in the
>> > 450 cases where it's not needed.
>>
>> In many of the places that BufferGetPage is called there is not a
>> snapshot available.  I assume that you would be OK with an Assert
>> that the flag was passed if the snapshot is NULL?
>
> Sure, that's fine.
>
> BTW I said "a macro" but I was forgetting that we have static inline
> functions in headers now, which means you can avoid the horrors of
> actually writing a macro.

Attached is what I think you're talking about for the first patch.
AFAICS this should generate identical executable code to unpatched.
Then the patch to actually implement the feature would, instead
of adding 30-some lines with TestForOldSnapshot() would implement
that as the behavior for the other enum value, and alter those
30-some BufferGetPage() calls.

Álvaro and Michael, is this what you were looking for?

Is everyone else OK with this approach?

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