WAL format and API changes (9.5)
I'd like to do some changes to the WAL format in 9.5. I want to annotate
each WAL record with the blocks that they modify. Every WAL record
already includes that information, but it's done in an ad hoc way,
differently in every rmgr. The RelFileNode and block number are
currently part of the WAL payload, and it's the REDO routine's
responsibility to extract it. I want to include that information in a
common format for every WAL record type.
That makes life a lot easier for tools that are interested in knowing
which blocks a WAL record modifies. One such tool is pg_rewind; it
currently has to understand every WAL record the backend writes. There's
also a tool out there called pg_readahead, which does prefetching of
blocks accessed by WAL records, to speed up PITR. I don't think that
tool has been actively maintained, but at least part of the reason for
that is probably that it's a pain to maintain when it has to understand
the details of every WAL record type.
It'd also be nice for contrib/pg_xlogdump and backend code itself. The
boilerplate code in all WAL redo routines, and writing WAL records,
could be simplified.
So, here's my proposal:
Insertion
---------
The big change in creating WAL records is that the buffers involved in
the WAL-logged operation are explicitly registered, by calling a new
XLogRegisterBuffer function. Currently, buffers that need full-page
images are registered by including them in the XLogRecData chain, but
with the new system, you call the XLogRegisterBuffer() function instead.
And you call that function for every buffer involved, even if no
full-page image needs to be taken, e.g because the page is going to be
recreated from scratch at replay.
It is no longer necessary to include the RelFileNode and BlockNumber of
the modified pages in the WAL payload. That information is automatically
included in the WAL record, when XLogRegisterBuffer is called.
Currently, the backup blocks are implicitly numbered, in the order the
buffers appear in XLogRecData entries. With the new API, the blocks are
numbered explicitly. This is more convenient when a WAL record sometimes
modifies a buffer and sometimes not. For example, a B-tree split needs
to modify four pages: the original page, the new page, the right sibling
(unless it's the rightmost page) and if it's an internal page, the page
at the lower level whose split the insertion completes. So there are two
pages that are sometimes missing from the record. With the new API, you
can nevertheless always register e.g. original page as buffer 0, new
page as 1, right sibling as 2, even if some of them are actually
missing. SP-GiST contains even more complicated examples of that.
The new XLogRegisterBuffer would look like this:
void XLogRegisterBuffer(int blockref_id, Buffer buffer, bool buffer_std)
blockref_id: An arbitrary ID given to this block reference. It is used
in the redo routine to open/restore the same block.
buffer: the buffer involved
buffer_std: is the page in "standard" page layout?
That's for the normal cases. We'll need a couple of variants for also
registering buffers that don't need full-page images, and perhaps also a
function for registering a page that *always* needs a full-page image,
regardless of the LSN. A few existing WAL record types just WAL-log the
whole page, so those ad-hoc full-page images could be replaced with this.
With these changes, a typical WAL insertion would look like this:
/* register the buffer with the WAL record, with ID 0 */
XLogRegisterBuffer(0, buf, true);
rdata[0].data = (char *) &xlrec;
rdata[0].len = sizeof(BlahRecord);
rdata[0].buffer_id = -1; /* -1 means the data is always included */
rdata[0].next = &(rdata[1]);
rdata[1].data = (char *) mydata;
rdata[1].len = mydatalen;
rdata[1].buffer_id = 0; /* 0 here refers to the buffer registered above */
rdata[1].next = NULL
...
recptr = XLogInsert(RM_BLAH_ID, xlinfo, rdata);
PageSetLSN(buf, recptr);
(While we're at it, perhaps we should let XLogInsert set the LSN of all
the registered buffers, to reduce the amount of boilerplate code).
(Instead of using a new XLogRegisterBuffer() function to register the
buffers, perhaps they should be passed to XLogInsert as a separate list
or array. I'm not wedded on the details...)
Redo
----
There are four different states a block referenced by a typical WAL
record can be in:
1. The old page does not exist at all (because the relation was
truncated later)
2. The old page exists, but has an LSN higher than current WAL record,
so it doesn't need replaying.
3. The LSN is < current WAL record, so it needs to be replayed.
4. The WAL record contains a full-page image, which needs to be restored.
With the current API, that leads to a long boilerplate:
/* If we have a full-page image, restore it and we're done */
if (HasBackupBlock(record, 0))
{
(void) RestoreBackupBlock(lsn, record, 0, false, false);
return;
}
buffer = XLogReadBuffer(xlrec->node, xlrec->block, false);
/* If the page was truncated away, we're done */
if (!BufferIsValid(buffer))
return;
page = (Page) BufferGetPage(buffer);
/* Has this record already been replayed? */
if (lsn <= PageGetLSN(page))
{
UnlockReleaseBuffer(buffer);
return;
}
/* Modify the page */
...
PageSetLSN(page, lsn);
MarkBufferDirty(buffer);
UnlockReleaseBuffer(buffer);
Let's simplify that, and have one new function, XLogOpenBuffer, which
returns a return code that indicates which of the four cases we're
dealing with. A typical redo function looks like this:
if (XLogOpenBuffer(0, &buffer) == BLK_REPLAY)
{
/* Modify the page */
...
PageSetLSN(page, lsn);
MarkBufferDirty(buffer);
}
if (BufferIsValid(buffer))
UnlockReleaseBuffer(buffer);
The '0' in the XLogOpenBuffer call is the ID of the block reference
specified in the XLogRegisterBuffer call, when the WAL record was created.
WAL format
----------
The registered block references need to be included in the WAL record.
We already do that for backup blocks, so a naive implementation would be
to just include a BkpBlock struct for all the block references, even
those that don't need a full-page image. That would be rather bulky,
though, so that needs some optimization. Shouldn't be difficult to omit
duplicated/unnecessary information, and add a flags field indicating
which fields are present. Overall, I don't expect there to be any big
difference in the amount of WAL generated by a typical application.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Heikki Linnakangas <hlinnakangas@vmware.com> writes:
The big change in creating WAL records is that the buffers involved in
the WAL-logged operation are explicitly registered, by calling a new
XLogRegisterBuffer function.
Seems reasonable, especially if we can make the buffer numbering business
less error-prone.
void XLogRegisterBuffer(int blockref_id, Buffer buffer, bool buffer_std)
blockref_id: An arbitrary ID given to this block reference. It is used
in the redo routine to open/restore the same block.
buffer: the buffer involved
buffer_std: is the page in "standard" page layout?
That's for the normal cases. We'll need a couple of variants for also
registering buffers that don't need full-page images, and perhaps also a
function for registering a page that *always* needs a full-page image,
regardless of the LSN. A few existing WAL record types just WAL-log the
whole page, so those ad-hoc full-page images could be replaced with this.
Why not just one function with an additional flags argument?
Also, IIRC there are places that WAL-log full pages that aren't in a
shared buffer at all (btree build does this I think). How will that fit
into this model?
(While we're at it, perhaps we should let XLogInsert set the LSN of all
the registered buffers, to reduce the amount of boilerplate code).
Yeah, maybe so. I'm not sure why that was separate to begin with.
Let's simplify that, and have one new function, XLogOpenBuffer, which
returns a return code that indicates which of the four cases we're
dealing with. A typical redo function looks like this:
if (XLogOpenBuffer(0, &buffer) == BLK_REPLAY)
{
/* Modify the page */
...
PageSetLSN(page, lsn);
MarkBufferDirty(buffer);
}
if (BufferIsValid(buffer))
UnlockReleaseBuffer(buffer);
The '0' in the XLogOpenBuffer call is the ID of the block reference
specified in the XLogRegisterBuffer call, when the WAL record was created.
+1, but one important step here is finding the data to be replayed.
That is, a large part of the complexity of replay routines has to do
with figuring out which parts of the WAL record were elided due to
full-page-images, and locating the remaining parts. What can we do
to make that simpler?
Ideally, if XLogOpenBuffer (bad name BTW) returns BLK_REPLAY, it would
also calculate and hand back the address/size of the logged data that
had been pointed to by the associated XLogRecData chain item. The
trouble here is that there might've been multiple XLogRecData items
pointing to the same buffer. Perhaps the magic ID number you give to
XLogOpenBuffer should be thought of as identifying an XLogRecData chain
item, not so much a buffer? It's fairly easy to see what to do when
there's just one chain item per buffer, but I'm not sure what to do
if there's more than one.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 04/03/2014 06:37 PM, Tom Lane wrote:
Also, IIRC there are places that WAL-log full pages that aren't in a
shared buffer at all (btree build does this I think). How will that fit
into this model?
Hmm. We could provide a function for registering a block with given
content, without a Buffer. Something like:
XLogRegisterPage(int id, RelFileNode, BlockNumber, Page)
Let's simplify that, and have one new function, XLogOpenBuffer, which
returns a return code that indicates which of the four cases we're
dealing with. A typical redo function looks like this:if (XLogOpenBuffer(0, &buffer) == BLK_REPLAY)
{
/* Modify the page */
...PageSetLSN(page, lsn);
MarkBufferDirty(buffer);
}
if (BufferIsValid(buffer))
UnlockReleaseBuffer(buffer);The '0' in the XLogOpenBuffer call is the ID of the block reference
specified in the XLogRegisterBuffer call, when the WAL record was created.+1, but one important step here is finding the data to be replayed.
That is, a large part of the complexity of replay routines has to do
with figuring out which parts of the WAL record were elided due to
full-page-images, and locating the remaining parts. What can we do
to make that simpler?
We can certainly add more structure to the WAL records, but any extra
information you add will make the records larger. It might be worth it,
and would be lost in the noise for more complex records like page
splits, but we should keep frequently-used records like heap insertions
as lean as possible.
Ideally, if XLogOpenBuffer (bad name BTW) returns BLK_REPLAY, it would
also calculate and hand back the address/size of the logged data that
had been pointed to by the associated XLogRecData chain item. The
trouble here is that there might've been multiple XLogRecData items
pointing to the same buffer. Perhaps the magic ID number you give to
XLogOpenBuffer should be thought of as identifying an XLogRecData chain
item, not so much a buffer? It's fairly easy to see what to do when
there's just one chain item per buffer, but I'm not sure what to do
if there's more than one.
Hmm. You could register a separate XLogRecData chain for each buffer.
Along the lines of:
rdata[0].data = data for buffer
rdata[0].len = ...
rdata[0].next = &rdata[1];
rdata[1].data = more data for same buffer
rdata[1].len = ...
rdata[2].next = NULL;
XLogRegisterBuffer(0, buffer, &data[0]);
At replay:
if (XLogOpenBuffer(0, &buffer, &xldata, &len) == BLK_REPLAY)
{
/* xldata points to the data registered for this buffer */
}
Plus one more chain for the data not associated with a buffer.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Heikki Linnakangas <hlinnakangas@vmware.com> writes:
On 04/03/2014 06:37 PM, Tom Lane wrote:
+1, but one important step here is finding the data to be replayed.
That is, a large part of the complexity of replay routines has to do
with figuring out which parts of the WAL record were elided due to
full-page-images, and locating the remaining parts. What can we do
to make that simpler?
We can certainly add more structure to the WAL records, but any extra
information you add will make the records larger. It might be worth it,
and would be lost in the noise for more complex records like page
splits, but we should keep frequently-used records like heap insertions
as lean as possible.
Sure, but in simple WAL records there would just be a single data chunk
that would be present in the normal case and not present in the FPI case.
Seems like we ought to be able to handle that degenerate case with no
significant wastage (probably just a flag bit someplace).
More generally, I'm pretty sure that your proposal is already going to
involve some small growth of WAL records compared to today, but I think
that's probably all right; the benefits seem significant.
Hmm. You could register a separate XLogRecData chain for each buffer.
Plus one more chain for the data not associated with a buffer.
That would probably work.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 04/03/2014 07:11 PM, Tom Lane wrote:
More generally, I'm pretty sure that your proposal is already going to
involve some small growth of WAL records compared to today,
Quite possible.
but I think
that's probably all right; the benefits seem significant.
Yep.
OTOH, once we store the relfilenode+block in a common format, we can
then try to optimize that format more heavily. Just as an example, omit
the tablespace oid in the RelFileNode, when it's the default tablespace
(with a flag bit indicating we did that). Or use a variable-length
endoding for the block number, on the assumption that smaller numbers
are more common. Probably not be worth the extra complexity, but we can
easily experiment with that kind of stuff once we have the
infrastructure in place.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Apr 3, 2014 at 10:14 AM, Heikki Linnakangas
<hlinnakangas@vmware.com> wrote:
(Instead of using a new XLogRegisterBuffer() function to register the
buffers, perhaps they should be passed to XLogInsert as a separate list or
array. I'm not wedded on the details...)
That would have the advantage of avoiding the function-call overhead,
which seems appealing.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes:
On Thu, Apr 3, 2014 at 10:14 AM, Heikki Linnakangas
<hlinnakangas@vmware.com> wrote:(Instead of using a new XLogRegisterBuffer() function to register the
buffers, perhaps they should be passed to XLogInsert as a separate list or
array. I'm not wedded on the details...)
That would have the advantage of avoiding the function-call overhead,
which seems appealing.
You're kidding, right? One function call per buffer touched by an update
is going to be lost in the noise.
A somewhat more relevant concern is where are we going to keep the state
data involved in all this. Since this code is, by definition, going to be
called in critical sections, any solution involving internal pallocs is
right out. The existing arrangement keeps all its data in local variables
of the function doing the update, which is probably a nice property to
preserve if we can manage it. That might force us to do it as above.
I'd still like something that *looks* more like a list of function calls,
though, even if they're macros under the hood. The existing approach to
setting up the XLogRecData chains is awfully prone to bugs of omission.
There are a few places that went so far as to set up custom macros to help
with that. I'm not a fan of doing the same thing in randomly different
ways in different places; but if we did it that way uniformly, it might be
better/safer.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2014-04-03 19:08:27 -0400, Tom Lane wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Thu, Apr 3, 2014 at 10:14 AM, Heikki Linnakangas
<hlinnakangas@vmware.com> wrote:(Instead of using a new XLogRegisterBuffer() function to register the
buffers, perhaps they should be passed to XLogInsert as a separate list or
array. I'm not wedded on the details...)That would have the advantage of avoiding the function-call overhead,
which seems appealing.You're kidding, right? One function call per buffer touched by an update
is going to be lost in the noise.A somewhat more relevant concern is where are we going to keep the state
data involved in all this. Since this code is, by definition, going to be
called in critical sections, any solution involving internal pallocs is
right out.
We actually already allocate memory in XLogInsert() :(, although only in
the first XLogInsert() a backend does... I didn't remember it, but I
complained about it before:
http://archives.postgresql.org/message-id/4FEB223A.3060707%40enterprisedb.com
I didn't realize the implications for it back then and thus didn't press
hard for a change, but I think it should be fixed.
The existing arrangement keeps all its data in local variables
of the function doing the update, which is probably a nice property to
preserve if we can manage it. That might force us to do it as above.
We could simply allocate enough scratch space for the state at process
startup. I don't think there'll ever be a need for XLogInsert() to be
reentrant, so that should be fine.
I'd still like something that *looks* more like a list of function calls,
though, even if they're macros under the hood. The existing approach to
setting up the XLogRecData chains is awfully prone to bugs of
omission.
Agreed. There's some pretty ugly code around this.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andres Freund <andres@2ndquadrant.com> writes:
On 2014-04-03 19:08:27 -0400, Tom Lane wrote:
A somewhat more relevant concern is where are we going to keep the state
data involved in all this. Since this code is, by definition, going to be
called in critical sections, any solution involving internal pallocs is
right out.
We actually already allocate memory in XLogInsert() :(, although only in
the first XLogInsert() a backend does...
Ouch. I wonder if we should put an Assert(not-in-critical-section)
into MemoryContextAlloc.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2014-04-03 19:33:12 -0400, Tom Lane wrote:
Andres Freund <andres@2ndquadrant.com> writes:
On 2014-04-03 19:08:27 -0400, Tom Lane wrote:
A somewhat more relevant concern is where are we going to keep the state
data involved in all this. Since this code is, by definition, going to be
called in critical sections, any solution involving internal pallocs is
right out.We actually already allocate memory in XLogInsert() :(, although only in
the first XLogInsert() a backend does...Ouch. I wonder if we should put an Assert(not-in-critical-section)
into MemoryContextAlloc.
XLogInsert() is using malloc() directly, so that wouldn't detect this
case...
It's not a bad idea tho. I wonder how far the regression tests
get...
Not even through initdb. GetVirtualXIDsDelayingChkpt() is to blame.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 04/04/2014 02:40 AM, Andres Freund wrote:
On 2014-04-03 19:33:12 -0400, Tom Lane wrote:
Andres Freund <andres@2ndquadrant.com> writes:
On 2014-04-03 19:08:27 -0400, Tom Lane wrote:
A somewhat more relevant concern is where are we going to keep the state
data involved in all this. Since this code is, by definition, going to be
called in critical sections, any solution involving internal pallocs is
right out.We actually already allocate memory in XLogInsert() :(, although only in
the first XLogInsert() a backend does...Ouch. I wonder if we should put an Assert(not-in-critical-section)
into MemoryContextAlloc.
Good idea!
XLogInsert() is using malloc() directly, so that wouldn't detect this
case...It's not a bad idea tho. I wonder how far the regression tests
get...Not even through initdb. GetVirtualXIDsDelayingChkpt() is to blame.
Hmm, the checkpointer process seems to ignore the rule, and just hopes
for the best. The allocation in GetVirtualXIDsDelayingChkpt() was
probably just an oversight, and that call could be moved outside the
critical section, but we also have this in AbsortFsyncRequests():
/*
* We have to PANIC if we fail to absorb all the pending requests (eg,
* because our hashtable runs out of memory). This is because the system
* cannot run safely if we are unable to fsync what we have been told to
* fsync. Fortunately, the hashtable is so small that the problem is
* quite unlikely to arise in practice.
*/
START_CRIT_SECTION();
Perhaps we could fix that by just calling fsync() directly if the
allocation fails, like we do if ForwardFsyncRequest() fails.
But if we give the checkpointer process a free pass, running the
regression tests with an assertion in AllocSetAlloc catches five genuine
bugs:
1. _bt_newroot
2. XLogFileInit
3. spgPageIndexMultiDelete
4. PageRepairFragmentation
5. PageIndexMultiDelete
I'll commit the attached patch to fix those shortly.
- Heikki
Attachments:
avoid-allocations-in-crit-section.patchtext/x-diff; name=avoid-allocations-in-crit-section.patchDownload+48-54
Hi,
On 2014-04-04 10:48:32 +0300, Heikki Linnakangas wrote:
But if we give the checkpointer process a free pass, running the regression
tests with an assertion in AllocSetAlloc catches five genuine bugs:1. _bt_newroot
2. XLogFileInit
3. spgPageIndexMultiDelete
4. PageRepairFragmentation
5. PageIndexMultiDelete
Some of those, like PageRepairFragmentation, are somewhat bad...
@@ -484,10 +483,11 @@ PageRepairFragmentation(Page page) ((PageHeader) page)->pd_upper = pd_special; } else - { /* nstorage != 0 */ + { /* Need to compact the page the hard way */ - itemidbase = (itemIdSort) palloc(sizeof(itemIdSortData) * nstorage); - itemidptr = itemidbase; + itemIdSortData itemidbase[MaxHeapTuplesPerPage]; + itemIdSort itemidptr = itemidbase; +
That's a fair bit of stack, and it can be called somewhat deep on the
stack via heap_page_prune_opt(). I wonder if we ought to add a
check_stack_depth() somewhere.
Thanks,
Andres Freund
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 04/04/2014 11:41 AM, Andres Freund wrote:
On 2014-04-04 10:48:32 +0300, Heikki Linnakangas wrote:
@@ -484,10 +483,11 @@ PageRepairFragmentation(Page page) ((PageHeader) page)->pd_upper = pd_special; } else - { /* nstorage != 0 */ + { /* Need to compact the page the hard way */ - itemidbase = (itemIdSort) palloc(sizeof(itemIdSortData) * nstorage); - itemidptr = itemidbase; + itemIdSortData itemidbase[MaxHeapTuplesPerPage]; + itemIdSort itemidptr = itemidbase; +That's a fair bit of stack, and it can be called somewhat deep on the
stack via heap_page_prune_opt(). I wonder if we ought to add a
check_stack_depth() somewhere.
Hmm, on my 64-bit laptop, that array is 24*291=6984 bytes with 8k block
size. That's fairly large, but not unheard of; there are a few places
where we allocate a BLCKSZ-sized buffer from stack.
We could easily reduce the stack consumption here by changing itemIdSort
to use 16-bit ints; all the lengths and offsets that
PageRepairFragmentation deals with fit in 16-bits.
But overall I wouldn't worry about it. check_stack_depth() leaves a fair
amount of headroom: STACK_DEPTH_SLOP is 512kB. As long as we don't
recurse, that's plenty.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2014-04-04 12:50:25 +0300, Heikki Linnakangas wrote:
On 04/04/2014 11:41 AM, Andres Freund wrote:
On 2014-04-04 10:48:32 +0300, Heikki Linnakangas wrote:
@@ -484,10 +483,11 @@ PageRepairFragmentation(Page page) ((PageHeader) page)->pd_upper = pd_special; } else - { /* nstorage != 0 */ + { /* Need to compact the page the hard way */ - itemidbase = (itemIdSort) palloc(sizeof(itemIdSortData) * nstorage); - itemidptr = itemidbase; + itemIdSortData itemidbase[MaxHeapTuplesPerPage]; + itemIdSort itemidptr = itemidbase; +That's a fair bit of stack, and it can be called somewhat deep on the
stack via heap_page_prune_opt(). I wonder if we ought to add a
check_stack_depth() somewhere.Hmm, on my 64-bit laptop, that array is 24*291=6984 bytes with 8k block
size. That's fairly large, but not unheard of; there are a few places where
we allocate a BLCKSZ-sized buffer from stack.
Yea, I am not complaing about using so much stack. Seems sensible here.
But overall I wouldn't worry about it. check_stack_depth() leaves a fair
amount of headroom: STACK_DEPTH_SLOP is 512kB. As long as we don't recurse,
that's plenty.
Well, there's no checks at nearby afair. That's why I was
wondering... But I don't have a big problem with not checking, I just
wanted to bring it up.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Ok, I fixed the issues that the assertion fixed. I also committed a
patch to add the assertion itself; let's see if the buildfarm finds more
cases that violate the rule.
It ignores the checkpointer, because it's known to violate the rule, and
allocations in ErrorContext, which is used during error recovery, e.g if
you indeed PANIC while in a critical section for some other reason.
I didn't backpatch this. Although you shouldn't be running with
assertions enabled in production, it nevertheless seems too risky. There
might be some obscure cases where there is no real risk, e.g because the
current memory context always has enough free space because of a
previous pfree, and it doesn't seem worth tracking down and fixing such
issues in backbranches. You have to be pretty unlucky to run out of
memory in a critical section to begin with.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Heikki Linnakangas <hlinnakangas@vmware.com> writes:
Ok, I fixed the issues that the assertion fixed. I also committed a
patch to add the assertion itself; let's see if the buildfarm finds more
cases that violate the rule.
It ignores the checkpointer, because it's known to violate the rule,
... uh, isn't that a bug to be fixed?
and
allocations in ErrorContext, which is used during error recovery, e.g if
you indeed PANIC while in a critical section for some other reason.
Yeah, I realized we'd have to do something about elog's own allocations.
Not sure if a blanket exemption for ErrorContext is the best way. I'd
been thinking of having a way to turn off the complaint once processing
of an elog(PANIC) has started.
I didn't backpatch this.
Agreed.
BTW, I'm pretty sure you added some redundant assertions in mcxt.c.
eg, palloc does not need its own copy.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 04/04/2014 04:56 PM, Tom Lane wrote:
Heikki Linnakangas <hlinnakangas@vmware.com> writes:
Ok, I fixed the issues that the assertion fixed. I also committed a
patch to add the assertion itself; let's see if the buildfarm finds more
cases that violate the rule.It ignores the checkpointer, because it's known to violate the rule,
... uh, isn't that a bug to be fixed?
Yes. One step a time ;-).
and
allocations in ErrorContext, which is used during error recovery, e.g if
you indeed PANIC while in a critical section for some other reason.Yeah, I realized we'd have to do something about elog's own allocations.
Not sure if a blanket exemption for ErrorContext is the best way. I'd
been thinking of having a way to turn off the complaint once processing
of an elog(PANIC) has started.
Hmm. PANIC processing should avoid allocations too, except in
ErrorContext, because otherwise you might get an out-of-memory during
PANIC processing.
ErrorContext also covers elog(DEBUG2, ...). I presume we'll want to
ignore that too. Although I also tried without the exemption for
ErrorContext at first, and didn't get any failures from the regression
tests, so I guess we never do that in a critical section. I was a bit
surprised by that.
BTW, I'm pretty sure you added some redundant assertions in mcxt.c.
eg, palloc does not need its own copy.
palloc is copy-pasted from MemoryContextAlloc - it does need its own copy.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Apr 3, 2014 at 7:44 PM, Heikki Linnakangas
<hlinnakangas@vmware.com> wrote:
I'd like to do some changes to the WAL format in 9.5. I want to annotate
each WAL record with the blocks that they modify. Every WAL record already
includes that information, but it's done in an ad hoc way, differently in
every rmgr. The RelFileNode and block number are currently part of the WAL
payload, and it's the REDO routine's responsibility to extract it. I want to
include that information in a common format for every WAL record type.That makes life a lot easier for tools that are interested in knowing which
blocks a WAL record modifies. One such tool is pg_rewind; it currently has
to understand every WAL record the backend writes. There's also a tool out
there called pg_readahead, which does prefetching of blocks accessed by WAL
records, to speed up PITR. I don't think that tool has been actively
maintained, but at least part of the reason for that is probably that it's a
pain to maintain when it has to understand the details of every WAL record
type.It'd also be nice for contrib/pg_xlogdump and backend code itself. The
boilerplate code in all WAL redo routines, and writing WAL records, could be
simplified.
I think it will also be useful, if we want to implement table/tablespace
PITR.
That's for the normal cases. We'll need a couple of variants for also
registering buffers that don't need full-page images, and perhaps also a
function for registering a page that *always* needs a full-page image,
regardless of the LSN. A few existing WAL record types just WAL-log the
whole page, so those ad-hoc full-page images could be replaced with this.With these changes, a typical WAL insertion would look like this:
/* register the buffer with the WAL record, with ID 0 */
XLogRegisterBuffer(0, buf, true);rdata[0].data = (char *) &xlrec;
rdata[0].len = sizeof(BlahRecord);
rdata[0].buffer_id = -1; /* -1 means the data is always included */
rdata[0].next = &(rdata[1]);rdata[1].data = (char *) mydata;
rdata[1].len = mydatalen;
rdata[1].buffer_id = 0; /* 0 here refers to the buffer registered
above */
rdata[1].next = NULL...
recptr = XLogInsert(RM_BLAH_ID, xlinfo, rdata);PageSetLSN(buf, recptr);
If we do register buffer's (that require or don't require FPI) before
calling XLogInsert(), then will there be any impact to handle case
where we come to know that we need to backup the buffer after
taking WALInsertLock.. or will that part of code remains same as it is
today.
Redo
----There are four different states a block referenced by a typical WAL record
can be in:1. The old page does not exist at all (because the relation was truncated
later)
2. The old page exists, but has an LSN higher than current WAL record, so it
doesn't need replaying.
3. The LSN is < current WAL record, so it needs to be replayed.
4. The WAL record contains a full-page image, which needs to be restored.
I think there might be a need to have separate handling for some special
cases like Init Page which is used in few ops (Insert/Update/multi-insert).
Is there any criteria to decide if it needs to be a separate state or a special
handling for operations?
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 04/03/2014 06:37 PM, Tom Lane wrote:
Let's simplify that, and have one new function, XLogOpenBuffer, which
returns a return code that indicates which of the four cases we're
dealing with. A typical redo function looks like this:
if (XLogOpenBuffer(0, &buffer) == BLK_REPLAY)
{
/* Modify the page */
...
PageSetLSN(page, lsn);
MarkBufferDirty(buffer);
}
if (BufferIsValid(buffer))
UnlockReleaseBuffer(buffer);
The '0' in the XLogOpenBuffer call is the ID of the block reference
specified in the XLogRegisterBuffer call, when the WAL record was created.+1, but one important step here is finding the data to be replayed.
That is, a large part of the complexity of replay routines has to do
with figuring out which parts of the WAL record were elided due to
full-page-images, and locating the remaining parts. What can we do
to make that simpler?Ideally, if XLogOpenBuffer (bad name BTW) returns BLK_REPLAY, it would
also calculate and hand back the address/size of the logged data that
had been pointed to by the associated XLogRecData chain item. The
trouble here is that there might've been multiple XLogRecData items
pointing to the same buffer. Perhaps the magic ID number you give to
XLogOpenBuffer should be thought of as identifying an XLogRecData chain
item, not so much a buffer? It's fairly easy to see what to do when
there's just one chain item per buffer, but I'm not sure what to do
if there's more than one.
Ok, I wrote the first version of a patch for this. The bulk of the patch
is fairly mechanical changes to all the routines that generate WAL
records and their redo routines, to use the new facilities. I'm sure
we'll have to go through a few rounds of discussions and review on the
APIs and names, but I wanted to go and change all the rest of the code
now anyway, to make sure the APIs cover the needs of all the WAL records.
The interesting part is the new APIs for constructing a WAL record, and
replaying it. I haven't polished the implementation of the APIs, but I'm
fairly happy with the way the WAL generation and redo routines now look
like.
Constructing a WAL record is now more convenient; no more constructing
of XLogRecData structs. There's a function, XLogRegisterData(int id,
char *data, int len), that takes the pointer and length directly as
arguments. You can call it multiple times with the same 'id', and the
data will be appended. (although I kept an API function that still
allows you to pass a chain of XLogRecDatas too, for the odd case where
that's more convenient)
Internally, XLogRegisterData() still constructs a chain of XLogRecDatas,
using a small number of static XLogRecData structs. That's a tad messy,
and I hope there is a better way to do that, but I wanted to focus on
the API for now.
Here's some more details on the API (also included in the README in the
patch):
Constructing a WAL record
=========================
A WAL record consists of multiple chunks of data. Each chunk is
identified by an ID number. A WAL record also contains information about
the blocks that it applies to, and each block reference is also
identified by an ID number. If the same ID number is used for data and a
block reference, the data is left out of the WAL record if a full-page
image of the block is taken.
The API for constructing a WAL record consists of four functions:
XLogBeginInsert, XLogRegisterBuffer, XLogRegisterData, and XLogInsert.
First, call XLogBeginInsert(). Then register all the buffers modified,
and data needed to replay the changes, using XLogRegister* functions.
Finally, insert the constructed record to the WAL with XLogInsert(). For
example:
XLogBeginInsert();
/* register buffers modified as part of this action */
XLogRegisterBuffer(0, lbuffer, REGBUF_STANDARD);
XLogRegisterBuffer(1, rbuffer, REGBUF_STANDARD);
/* register data that is always included in the WAL record */
XLogRegisterData(-1, &xlrec, SizeOfFictionalAction);
/*
* register data associated with lbuffer. This will not be
* included in the record if a full-page image is taken.
*/
XLogRegisterData(0, footuple->data, footuple->len);
/* data associated with rbuffer */
XLogRegisterData(0, data2, len2);
/*
* Ok, all the data and buffers to include in the WAL record
* have been registered. Insert the record.
*/
recptr = XLogInsert(RM_FOO_ID, XLOG_FOOBAR_DOSTUFF);
Details of the API functions:
void XLogRegisterBuffer(int id, Buffer buf, int flags);
-----
XLogRegisterBuffer adds information about a data block to the WAL
record. 'id' is an arbitrary number used to identify this page reference
in the redo routine. The information needed to re-find the page at redo
- the relfilenode, fork, and block number - are included in the WAL record.
XLogInsert will automatically include a full copy of the page contents,
if this is the first modification of the buffer since the last
checkpoint. It is important to register every buffer modified by the
action with XLogRegisterBuffer, to avoid torn-page hazards.
The flags control when and how the buffer contents are included in the
WAL record. REGBUF_STANDARD means that the page follows the standard
page layout, and causes the area between pd_lower and pd_upper to be
left out from the image, reducing the WAL volume. Normally, a full-page
image is taken only if the page has not been modified since the last
checkpoint, and only if full_page_writes=on or an online backup is in
progress. The REGBUF_FORCE_IMAGE flag can be used to force a full-page
image to always be included. If the REGBUF_WILL_INIT flag is given, a
full-page image is never taken. The redo routine must completely
re-generate the page contents, using the data included in the WAL record.
void XLogRegisterData(int id, char *data, int len);
-----
XLogRegisterData is used to include arbitrary data in the WAL record. If
XLogRegisterData() is called multiple times with the same 'id', the data
are appended, and will be made available to the redo routine as one
contiguous chunk.
If the same 'id' is used in an XLogRegisterBuffer and XLogRegisterData()
call, the data is not included in the WAL record if a full-page image of
the page is taken. Data registered with id -1 is not related with a
buffer, and is always included.
Writing a REDO routine
======================
A REDO routine uses the data and page references included in the WAL
record to reconstruct the new state of the page. To access the data and
pages included in the WAL record, the following functions are available:
char *XLogRecGetData(XLogRecord *record)
-----
Returns the "main" chunk of data included in the WAL record. That is,
the data included in the record with XLogRegisterData(-1, ...).
XLogReplayResult XLogReplayBuffer(int id, Buffer *buf)
-----
Reads a block associated with the WAL record currently being replayed,
with the given 'id'. The shared buffer is returned in *buf, or
InvalidBuffer if the page could not be found. The block is read into a
shared buffer and locked in exclusive mode. Returns one of the following
result codes:
typedef enum
{
BLK_NEEDS_REDO, /* block needs to be replayed */
BLK_DONE, /* block was already replayed */
BLK_RESTORED, /* block was restored from a full-page image */
BLK_NOTFOUND /* block was not found (and hence does not need
* to be replayed) */
} XLogReplayResult;
The REDO routine must redo the actions to the page if XLogReplayBuffer
returns BLK_NEEDS_REDO. In other cases, no further action is no
required, although the result code can be used to distinguish the reason.
After modifying the page (if it was necessary), the REDO routine must
unlock and release the buffer. Note that the buffer must be unlocked and
released even if no action was required.
XLogReplayResult XLogReplayBufferExtended(int id, ReadBufferMode mode,
bool get_cleanup_lock, Buffer *buf)
-----
Like XLogReplayBuffer(), but with a few extra options.
mode' can be passed to e.g force the page to be zeroed, instead of
reading it from disk. This RBM_ZERO mode should be used to re-initialize
pages registered in the REGBUF_WILL_INIT mode in XLogRegisterBuffer().
if 'get_cleanup_lock' is TRUE, a stronger "cleanup" lock on the page is
acquired, instead of a reguler exclusive-lock.
char *XLogGetPayload(XLogRecord *record, int id, int *len)
-----
Returns a chunk of data included in the WAL record (with
XLogRegisterData(id, ...)). The length of the data is returned in *len.
This is typically used after XLogReplayBuffer() returned BLK_NEEDS_REDO,
with the same 'id', to get the information required to redo the actions
on the page. If no data with the given id is included, perhaps because a
full-page image of the associated buffer was taken instead, an error is
thrown.
void XLogBlockRefGetTag(XLogRecord *record, int id, RelFileNode *rnode,
ForkNumber *forknum, BlockNumber *blknum);
-----
Returns the relfilenode, fork number, and block number of the page
registered with the given ID.
Finally, here's the usual pattern for how a REDO routine works (taken
from btree_xlog_insert):
if (XLogReplayBuffer(0, &buffer) == BLK_NEEDS_REDO)
{
int datalen;
char *datapos = XLogGetPayload(record, 0, &datalen);
page = (Page) BufferGetPage(buffer);
if (PageAddItem(page, (Item) datapos, datalen, xlrec->offnum,
false, false) == InvalidOffsetNumber)
elog(PANIC, "btree_insert_redo: failed to add item");
PageSetLSN(page, lsn);
MarkBufferDirty(buffer);
}
if (BufferIsValid(buffer))
UnlockReleaseBuffer(buffer);
- Heikki
Attachments:
wal-format-and-api-changes-1.patch.gzapplication/gzip; name=wal-format-and-api-changes-1.patch.gzDownload+3-1
On 05/17/2014 02:54 PM, Heikki Linnakangas wrote:
On 04/03/2014 06:37 PM, Tom Lane wrote:
Let's simplify that, and have one new function, XLogOpenBuffer, which
returns a return code that indicates which of the four cases we're
dealing with. A typical redo function looks like this:
if (XLogOpenBuffer(0, &buffer) == BLK_REPLAY)
{
/* Modify the page */
...
PageSetLSN(page, lsn);
MarkBufferDirty(buffer);
}
if (BufferIsValid(buffer))
UnlockReleaseBuffer(buffer);
The '0' in the XLogOpenBuffer call is the ID of the block reference
specified in the XLogRegisterBuffer call, when the WAL record was created.+1, but one important step here is finding the data to be replayed.
That is, a large part of the complexity of replay routines has to do
with figuring out which parts of the WAL record were elided due to
full-page-images, and locating the remaining parts. What can we do
to make that simpler?Ideally, if XLogOpenBuffer (bad name BTW) returns BLK_REPLAY, it would
also calculate and hand back the address/size of the logged data that
had been pointed to by the associated XLogRecData chain item. The
trouble here is that there might've been multiple XLogRecData items
pointing to the same buffer. Perhaps the magic ID number you give to
XLogOpenBuffer should be thought of as identifying an XLogRecData chain
item, not so much a buffer? It's fairly easy to see what to do when
there's just one chain item per buffer, but I'm not sure what to do
if there's more than one.Ok, I wrote the first version of a patch for this.
Here's a new version, rebased against master. No other changes.
- Heikki
