Re: WAL format and API changes (9.5)

From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
Cc: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Subject: Re: WAL format and API changes (9.5)
Date: 2014-08-12 22:04:22
Message-ID: 20140812220422.GF14949@awork2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2014-08-12 12:44:22 +0300, Heikki Linnakangas wrote:
> Here's a new patch with those little things fixed.

I've so far ignored this thread... I'm now taking a look. Unfortunately
I have to admit I'm not unequivocally happy.

* XLogRegisterData/XLogRegisterRecData imo don't really form a
consistent API namewise. What does 'Rec' mean here?

* I'm distinctly not a fan of passing around both the LSN and the struct
XLogRecord to functions. We should bit the bullet and use something
encapsulating both.

* XLogReplayBuffer imo isn't a very good name - the buffer isn't
replayed. If anything it's sometimes replaced by the backup block, but
the real replay still happens outside of that function.

* Why do we need XLogBeginInsert(). Right now this leads to uglyness
like duplicated if (RelationNeedsWAL()) wal checks and
XLogCancelInsert() of inserts that haven't been started. And if we
have Begin, why do we also need Cancel?

* "XXX: do we need to do this for every page?" - yes, and it's every
tuple, not every page... And It needs to be done *after* the tuple has
been put on the page, not before. Otherwise the location will be wrong.

* The patch mixes the API changes around WAL records with changes of how
individual actions are logged. That makes it rather hard to review -
and it's a 500kb patch already.

I realize it's hard to avoid because the new API changes which
information about blocks is available, but it does make it really hard
to see whether the old/new code is doing something
equivalent. It's hard to find more critical code than this :/

Have you done any performance evaluation?

Greetings,

Andres Freund

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2014-08-12 22:52:44 Re: Minmax indexes
Previous Message Andres Freund 2014-08-12 21:02:24 Re: Scaling shared buffer eviction