Re: relaxing sync commit if no WAL written (was Re: unlogged tables)

On Wed, Dec 15, 2010 at 2:20 AM, Heikki Linnakangas
<heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
> Looks ok. I'd suggest rewording this comment though:
>
> [ the comment in question ]
>
> It's a bit hard to follow, as it first lists exceptions on when we must
> flush XLOG immediately, and then lists conditions on when we can skip it.

See if the attached looks better to you. I mostly adopted your
proposal, with a bit of additional wordsmithing, and I also added a
parenthetical comment about why we don't skip writing the commit
record altogether in this case, since that's come up twice now.

I've removed the references to unlogged tables for now, as I'm
thinking it makes sense to commit this part first.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Fri, 2010-12-17 at 13:35 -0500, Robert Haas wrote:

> I'm
> thinking it makes sense to commit this part first.

This will break Hot Standby, as previously explained. Don't.

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

On Sun, Dec 19, 2010 at 7:01 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> On Fri, 2010-12-17 at 13:35 -0500, Robert Haas wrote:
>
>> I'm
>> thinking it makes sense to commit this part first.
>
> This will break Hot Standby, as previously explained. Don't.

Uh, why? Skipping the commit record altogether would do that, but
this patch doesn't do that.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Sun, 2010-12-19 at 07:33 -0500, Robert Haas wrote:
> On Sun, Dec 19, 2010 at 7:01 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> > On Fri, 2010-12-17 at 13:35 -0500, Robert Haas wrote:
> >
> >> I'm
> >> thinking it makes sense to commit this part first.
> >
> > This will break Hot Standby, as previously explained. Don't.
>
> Uh, why? Skipping the commit record altogether would do that, but
> this patch doesn't do that.

I was looking for XLogStandbyInfoActive()

It isn't there, so you're either breaking HS or missing a possible
optimisation. Having said that, it would be useful to be able to assume
that all xids appear in the log, for diagnostic purposes.

So I now agree with the way you've coded it.

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

On Sun, Dec 19, 2010 at 3:08 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> On Sun, 2010-12-19 at 07:33 -0500, Robert Haas wrote:
>> On Sun, Dec 19, 2010 at 7:01 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>> > On Fri, 2010-12-17 at 13:35 -0500, Robert Haas wrote:
>> >
>> >> I'm
>> >> thinking it makes sense to commit this part first.
>> >
>> > This will break Hot Standby, as previously explained. Don't.
>>
>> Uh, why?  Skipping the commit record altogether would do that, but
>> this patch doesn't do that.
>
> I was looking for XLogStandbyInfoActive()
>
> It isn't there, so you're either breaking HS or missing a possible
> optimisation. Having said that, it would be useful to be able to assume
> that all xids appear in the log, for diagnostic purposes.
>
> So I now agree with the way you've coded it.

OK, thanks. Committed. Note that there is also a long comment in
there which includes a discussion of the issues relating to Hot
Standby. Hopefully that's clear enough to prevent anyone from getting
too clever with this in the future.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company