Thread: PushActiveSnapshot(GetTransactionSnapshot())

PushActiveSnapshot(GetTransactionSnapshot())

From
Simon Riggs
Date:
In common cases of snapshot use we run GetSnapshotData() into a
statically allocated snapshot, then immediately copy the static struct
into a dynamically allocated copy.

The static allocation was designed to remove the overhead of dynamic
allocation, but then we do it anyway.

The snapmgr code does this explicitly, but the reason isn't
documented, it just says we must do this.

Any one say why?

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


Re: PushActiveSnapshot(GetTransactionSnapshot())

From
Alvaro Herrera
Date:
Excerpts from Simon Riggs's message of dom ago 21 16:23:39 -0300 2011:
> In common cases of snapshot use we run GetSnapshotData() into a
> statically allocated snapshot, then immediately copy the static struct
> into a dynamically allocated copy.
> 
> The static allocation was designed to remove the overhead of dynamic
> allocation, but then we do it anyway.
> 
> The snapmgr code does this explicitly, but the reason isn't
> documented, it just says we must do this.

IIRC the active snapshot is scribbled onto by some operations, which is
why the copy is mandatory.  Maybe there's some way to optimize things so
that the copy is done only when necessary.  IIRC the copying of the
ActiveSnapshot was only introduced because some subtle bugs were
detected in the code without copy.  When I introduced the mandatory
copy, I don't remember thinking about the statically allocated struct.

The fact that PushActiveSnapshot and GetTransactionSnapshot are in two
completely separate modules complicates optimization.  Note I'm not
saying it's impossible -- I just didn't look into it.

-- 
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support


Re: PushActiveSnapshot(GetTransactionSnapshot())

From
Simon Riggs
Date:
On Mon, Aug 22, 2011 at 7:07 PM, Alvaro Herrera
<alvherre@commandprompt.com> wrote:
> Excerpts from Simon Riggs's message of dom ago 21 16:23:39 -0300 2011:
>> In common cases of snapshot use we run GetSnapshotData() into a
>> statically allocated snapshot, then immediately copy the static struct
>> into a dynamically allocated copy.
>>
>> The static allocation was designed to remove the overhead of dynamic
>> allocation, but then we do it anyway.
>>
>> The snapmgr code does this explicitly, but the reason isn't
>> documented, it just says we must do this.
>
> IIRC the active snapshot is scribbled onto by some operations, which is
> why the copy is mandatory.  Maybe there's some way to optimize things so
> that the copy is done only when necessary.  IIRC the copying of the
> ActiveSnapshot was only introduced because some subtle bugs were
> detected in the code without copy.  When I introduced the mandatory
> copy, I don't remember thinking about the statically allocated struct.

"Some operations", "subtle bugs".

Do you have any further information on those?

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


Re: PushActiveSnapshot(GetTransactionSnapshot())

From
Alvaro Herrera
Date:
Excerpts from Simon Riggs's message of mar ago 23 10:56:17 -0300 2011:
> On Mon, Aug 22, 2011 at 7:07 PM, Alvaro Herrera
> <alvherre@commandprompt.com> wrote:
> > Excerpts from Simon Riggs's message of dom ago 21 16:23:39 -0300 2011:
> >> In common cases of snapshot use we run GetSnapshotData() into a
> >> statically allocated snapshot, then immediately copy the static struct
> >> into a dynamically allocated copy.
> >>
> >> The static allocation was designed to remove the overhead of dynamic
> >> allocation, but then we do it anyway.
> >>
> >> The snapmgr code does this explicitly, but the reason isn't
> >> documented, it just says we must do this.
> >
> > IIRC the active snapshot is scribbled onto by some operations, which is
> > why the copy is mandatory.  Maybe there's some way to optimize things so
> > that the copy is done only when necessary.  IIRC the copying of the
> > ActiveSnapshot was only introduced because some subtle bugs were
> > detected in the code without copy.  When I introduced the mandatory
> > copy, I don't remember thinking about the statically allocated struct.
> 
> "Some operations", "subtle bugs".
> 
> Do you have any further information on those?

See commits 07cefdfb7a1c1a7ae96783c9723102250a4c3bad and
caa4cfa3697472a6673eb817eb34681684cba14f

-- 
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support