Hi,
Patch 01: Fix a couple of embarassing typos. Most of them harmless, but
one isn't and can lead to crashing or decoding wrong data.
Patch 02: Don't crash with an Assert() failure if wal_level=logical but
max_replication_slots=0.
Patch 03: Add valgrind suppression for writing out padding bytes. That's
better than zeroing the data from the get go because unitialized
accesses are still detected.
Details are in the commit messages of the individual patches.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
>From 9987ff3fead5cee0b9c7da94c8d8a665015be7c2 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Thu, 8 May 2014 17:00:06 +0200
Subject: [PATCH 1/3] Fix a couple of brown paper bag typos in the logical
decoding code.
Most of these were fairly harmless but the typos in
ReorderBufferSerializeChange() could lead to crashes or wrong data
being decoded.
The ReorderBufferSerializeChange() bug was encountered by Christian
Kruse; the rest were found during a review for similar sillyness.
---
src/backend/replication/logical/reorderbuffer.c | 13 +++++--------
src/backend/replication/logical/snapbuild.c | 11 +++++------
2 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 7f2bbca..f96e3e1 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -2064,15 +2064,15 @@ ReorderBufferSerializeChange(ReorderBuffer *rb, ReorderBufferTXN *txn,
if (snap->xcnt)
{
memcpy(data, snap->xip,
- sizeof(TransactionId) + snap->xcnt);
- data += sizeof(TransactionId) + snap->xcnt;
+ sizeof(TransactionId) * snap->xcnt);
+ data += sizeof(TransactionId) * snap->xcnt;
}
if (snap->subxcnt)
{
memcpy(data, snap->subxip,
- sizeof(TransactionId) + snap->subxcnt);
- data += sizeof(TransactionId) + snap->subxcnt;
+ sizeof(TransactionId) * snap->subxcnt);
+ data += sizeof(TransactionId) * snap->subxcnt;
}
break;
}
@@ -2168,15 +2168,12 @@ ReorderBufferRestoreChanges(ReorderBuffer *rb, ReorderBufferTXN *txn,
}
- ReorderBufferSerializeReserve(rb, sizeof(ReorderBufferDiskChange));
-
-
/*
* Read the statically sized part of a change which has information
* about the total size. If we couldn't read a record, we're at the
* end of this file.
*/
-
+ ReorderBufferSerializeReserve(rb, sizeof(ReorderBufferDiskChange));
readBytes = read(*fd, rb->outbuf, sizeof(ReorderBufferDiskChange));
/* eof */
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index cb45f90..f00fd7d 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -307,8 +307,7 @@ AllocateSnapshotBuilder(ReorderBuffer *reorder,
builder->committed.xip =
palloc0(builder->committed.xcnt_space * sizeof(TransactionId));
builder->committed.includes_all_transactions = true;
- builder->committed.xip =
- palloc0(builder->committed.xcnt_space * sizeof(TransactionId));
+
builder->initial_xmin_horizon = xmin_horizon;
builder->transactions_after = start_lsn;
@@ -1691,7 +1690,7 @@ SnapBuildRestore(SnapBuild *builder, XLogRecPtr lsn)
/* restore running xacts information */
sz = sizeof(TransactionId) * ondisk.builder.running.xcnt_space;
- ondisk.builder.running.xip = MemoryContextAlloc(builder->context, sz);
+ ondisk.builder.running.xip = MemoryContextAllocZero(builder->context, sz);
readBytes = read(fd, ondisk.builder.running.xip, sz);
if (readBytes != sz)
{
@@ -1705,7 +1704,7 @@ SnapBuildRestore(SnapBuild *builder, XLogRecPtr lsn)
/* restore committed xacts information */
sz = sizeof(TransactionId) * ondisk.builder.committed.xcnt;
- ondisk.builder.committed.xip = MemoryContextAlloc(builder->context, sz);
+ ondisk.builder.committed.xip = MemoryContextAllocZero(builder->context, sz);
readBytes = read(fd, ondisk.builder.committed.xip, sz);
if (readBytes != sz)
{
@@ -1763,10 +1762,10 @@ SnapBuildRestore(SnapBuild *builder, XLogRecPtr lsn)
}
ondisk.builder.committed.xip = NULL;
- builder->running.xcnt = ondisk.builder.committed.xcnt;
+ builder->running.xcnt = ondisk.builder.running.xcnt;
if (builder->running.xip)
pfree(builder->running.xip);
- builder->running.xcnt_space = ondisk.builder.committed.xcnt_space;
+ builder->running.xcnt_space = ondisk.builder.running.xcnt_space;
builder->running.xip = ondisk.builder.running.xip;
/* our snapshot is not interesting anymore, build a new one */
--
1.8.5.rc2.dirty
>From 863589478ce185cba02e8ad5a4ff2a8b79588cb0 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Thu, 8 May 2014 18:02:20 +0200
Subject: [PATCH 2/3] Remove overeager assertion in wal_level=logical specific
code.
The assertion triggered when a relation was rewritten while
wal_level=logical but max_replication_slots=0 was set. That's not a
meaningful configuration, but that's not a excuse for crashing.
---
src/backend/access/heap/rewriteheap.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index 7b57911..687e76e 100644
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -812,8 +812,6 @@ logical_begin_heap_rewrite(RewriteState state)
if (!state->rs_logical_rewrite)
return;
- Assert(ReplicationSlotCtl != NULL);
-
ProcArrayGetReplicationSlotXmin(NULL, &logical_xmin);
/*
--
1.8.5.rc2.dirty
>From 2cc7da5accda01b579bb785b510bb16d133516d9 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Thu, 8 May 2014 18:16:34 +0200
Subject: [PATCH 3/3] Add valgrind suppression to ignore padding bytes in
reorderbuffer disk writes.
---
src/tools/valgrind.supp | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/src/tools/valgrind.supp b/src/tools/valgrind.supp
index d3447d7..b0ab53d 100644
--- a/src/tools/valgrind.supp
+++ b/src/tools/valgrind.supp
@@ -63,6 +63,16 @@
fun:write_relcache_init_file
}
+{
+ padding_reorderbuffer_serialize
+ Memcheck:Param
+ write(buf)
+
+ ...
+ fun:ReorderBufferSerializeChange
+}
+
+
# gcc on ppc64 can generate a four-byte read to fetch the final "char" fields
# of a FormData_pg_cast. This is valid compiler behavior, because a proper
--
1.8.5.rc2.dirty
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
