pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum"
16+ messages / 6 participants
[nested] [flat]
Commits from this thread:
2021-06-24 b6d8d2073f 2a0ab13f8d caac196507 c78bb32c19 a5eacd86b7 85ae8ccb2f Prevent race condition while reading relmapper file.
* BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum"
@ 2021-06-18 15:00 PG Bug reporting form <noreply@postgresql.org>
2021-06-22 09:30 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Heikki Linnakangas <hlinnaka@iki.fi>
0 siblings, 1 reply; 16+ messages in thread
From: PG Bug reporting form @ 2021-06-18 15:00 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: exclusion@gmail.com
The following bug has been logged on the website:
Bug reference: 17064
Logged by: Alexander Lakhin
Email address: exclusion@gmail.com
PostgreSQL version: 14beta1
Operating system: Ubuntu 20.04
Description:
The following script:
===
for i in `seq 100`; do
createdb db$i
done
# Based on the contents of the regression test "vacuum"
echo "
CREATE TABLE pvactst (i INT);
INSERT INTO pvactst SELECT i FROM generate_series(1,10000) i;
DELETE FROM pvactst;
VACUUM pvactst;
DROP TABLE pvactst;
VACUUM FULL pg_database;
" >/tmp/vacuum.sql
for n in `seq 10`; do
echo "iteration $n"
for i in `seq 100`; do
( { for f in `seq 100`; do cat /tmp/vacuum.sql; done } | psql -d db$i )
>psql-$i.log 2>&1 &
done
wait
grep -C5 FATAL psql*.log && break;
done
===
detects sporadic FATAL errors:
iteration 1
psql-56.log-DROP TABLE
psql-56.log-VACUUM
psql-56.log-CREATE TABLE
psql-56.log-INSERT 0 10000
psql-56.log-DELETE 10000
psql-56.log:FATAL: relation mapping file "global/pg_filenode.map" contains
incorrect checksum
psql-56.log-server closed the connection unexpectedly
psql-56.log- This probably means the server terminated abnormally
psql-56.log- before or while processing the request.
psql-56.log-connection to server was lost
(Discovered when running multiple installcheck's for a single instance.
(Except for this failure, installcheck x 100 (with several tests excluded)
is rather stable.))
Reproduced on REL9_6_STABLE .. master.
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum"
2021-06-18 15:00 BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" PG Bug reporting form <noreply@postgresql.org>
@ 2021-06-22 09:30 ` Heikki Linnakangas <hlinnaka@iki.fi>
2021-06-22 13:00 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 16+ messages in thread
From: Heikki Linnakangas @ 2021-06-22 09:30 UTC (permalink / raw)
To: exclusion@gmail.com; pgsql-bugs@lists.postgresql.org
On 18/06/2021 18:00, PG Bug reporting form wrote:
> The following bug has been logged on the website:
>
> Bug reference: 17064
> Logged by: Alexander Lakhin
> Email address: exclusion@gmail.com
> PostgreSQL version: 14beta1
> Operating system: Ubuntu 20.04
> Description:
>
> The following script:
> ===
> for i in `seq 100`; do
> createdb db$i
> done
>
> # Based on the contents of the regression test "vacuum"
> echo "
> CREATE TABLE pvactst (i INT);
> INSERT INTO pvactst SELECT i FROM generate_series(1,10000) i;
> DELETE FROM pvactst;
> VACUUM pvactst;
> DROP TABLE pvactst;
>
> VACUUM FULL pg_database;
> " >/tmp/vacuum.sql
>
> for n in `seq 10`; do
> echo "iteration $n"
> for i in `seq 100`; do
> ( { for f in `seq 100`; do cat /tmp/vacuum.sql; done } | psql -d db$i )
>> psql-$i.log 2>&1 &
> done
> wait
> grep -C5 FATAL psql*.log && break;
> done
> ===
> detects sporadic FATAL errors:
> iteration 1
> psql-56.log-DROP TABLE
> psql-56.log-VACUUM
> psql-56.log-CREATE TABLE
> psql-56.log-INSERT 0 10000
> psql-56.log-DELETE 10000
> psql-56.log:FATAL: relation mapping file "global/pg_filenode.map" contains
> incorrect checksum
> psql-56.log-server closed the connection unexpectedly
> psql-56.log- This probably means the server terminated abnormally
> psql-56.log- before or while processing the request.
> psql-56.log-connection to server was lost
Hmm, the simplest explanation would be that the read() or write() on the
relmapper file is not atomic. We assume that it is, and don't use a lock
in load_relmap_file() because of that. Is there anything unusual about
the filesystem, mount options or the kernel you're using? I could not
reproduce this on my laptop. Does the attached patch fix it for you?
If that's the cause, it is easy to fix by taking the RelationMappingLock
in load_relmap_file(), like in the attached patch. But if the write is
not atomic, you might have a bigger problem: we also rely on the
atomicity when writing the pg_control file. If that becomes corrupt
because of a partial write, the server won't start up. If it's just a
race condition between the read/write, or only the read() is not atomic,
maybe pg_control is OK, but I'd like to understand the issue better
before just adding a lock to load_relmap_file().
- Heikki
Attachments:
lock-load_relmap_file-1.patch (text/x-patch, 3.5K) download
diff --git a/src/backend/utils/cache/relmapper.c b/src/backend/utils/cache/relmapper.c
index 424624cf0d..d0986a59de 100644
--- a/src/backend/utils/cache/relmapper.c
+++ b/src/backend/utils/cache/relmapper.c
@@ -136,7 +136,7 @@ static void apply_map_update(RelMapFile *map, Oid relationId, Oid fileNode,
bool add_okay);
static void merge_map_updates(RelMapFile *map, const RelMapFile *updates,
bool add_okay);
-static void load_relmap_file(bool shared);
+static void load_relmap_file(bool shared, bool lock_held);
static void write_relmap_file(bool shared, RelMapFile *newmap,
bool write_wal, bool send_sinval, bool preserve_files,
Oid dbid, Oid tsid, const char *dbpath);
@@ -405,12 +405,12 @@ RelationMapInvalidate(bool shared)
if (shared)
{
if (shared_map.magic == RELMAPPER_FILEMAGIC)
- load_relmap_file(true);
+ load_relmap_file(true, false);
}
else
{
if (local_map.magic == RELMAPPER_FILEMAGIC)
- load_relmap_file(false);
+ load_relmap_file(false, false);
}
}
@@ -425,9 +425,9 @@ void
RelationMapInvalidateAll(void)
{
if (shared_map.magic == RELMAPPER_FILEMAGIC)
- load_relmap_file(true);
+ load_relmap_file(true, false);
if (local_map.magic == RELMAPPER_FILEMAGIC)
- load_relmap_file(false);
+ load_relmap_file(false, false);
}
/*
@@ -612,7 +612,7 @@ RelationMapInitializePhase2(void)
/*
* Load the shared map file, die on error.
*/
- load_relmap_file(true);
+ load_relmap_file(true, false);
}
/*
@@ -633,7 +633,7 @@ RelationMapInitializePhase3(void)
/*
* Load the local map file, die on error.
*/
- load_relmap_file(false);
+ load_relmap_file(false, false);
}
/*
@@ -695,7 +695,7 @@ RestoreRelationMap(char *startAddress)
* Note that the local case requires DatabasePath to be set up.
*/
static void
-load_relmap_file(bool shared)
+load_relmap_file(bool shared, bool lock_held)
{
RelMapFile *map;
char mapfilename[MAXPGPATH];
@@ -725,12 +725,18 @@ load_relmap_file(bool shared)
mapfilename)));
/*
- * Note: we could take RelationMappingLock in shared mode here, but it
- * seems unnecessary since our read() should be atomic against any
- * concurrent updater's write(). If the file is updated shortly after we
- * look, the sinval signaling mechanism will make us re-read it before we
- * are able to access any relation that's affected by the change.
+ * Grab the lock to prevent the file from being updated while we read it,
+ * unless the caller is already holding the lock. This shouldn't be
+ * necessary. The file is small so the read() should be atomic against any
+ * concurrent updater's write(). But better safe than sorry.
+ *
+ * If the file is updated shortly after we look, the sinval signaling
+ * mechanism will make us re-read it before we are able to access any
+ * relation that's affected by the change.
*/
+ if (!lock_held)
+ LWLockAcquire(RelationMappingLock, LW_SHARED);
+
pgstat_report_wait_start(WAIT_EVENT_RELATION_MAP_READ);
r = read(fd, map, sizeof(RelMapFile));
if (r != sizeof(RelMapFile))
@@ -747,6 +753,9 @@ load_relmap_file(bool shared)
}
pgstat_report_wait_end();
+ if (!lock_held)
+ LWLockRelease(RelationMappingLock);
+
if (CloseTransientFile(fd) != 0)
ereport(FATAL,
(errcode_for_file_access(),
@@ -969,7 +978,7 @@ perform_relmap_update(bool shared, const RelMapFile *updates)
LWLockAcquire(RelationMappingLock, LW_EXCLUSIVE);
/* Be certain we see any other updates just made */
- load_relmap_file(shared);
+ load_relmap_file(shared, true);
/* Prepare updated data in a local variable */
if (shared)
* Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum"
2021-06-18 15:00 BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" PG Bug reporting form <noreply@postgresql.org>
2021-06-22 09:30 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Heikki Linnakangas <hlinnaka@iki.fi>
@ 2021-06-22 13:00 ` Thomas Munro <thomas.munro@gmail.com>
2021-06-22 14:11 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Tom Lane <tgl@sss.pgh.pa.us>
2021-06-22 17:00 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Alexander Lakhin <exclusion@gmail.com>
0 siblings, 2 replies; 16+ messages in thread
From: Thomas Munro @ 2021-06-22 13:00 UTC (permalink / raw)
To: Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Tue, Jun 22, 2021 at 9:30 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> Hmm, the simplest explanation would be that the read() or write() on the
> relmapper file is not atomic. We assume that it is, and don't use a lock
> in load_relmap_file() because of that. Is there anything unusual about
> the filesystem, mount options or the kernel you're using? I could not
> reproduce this on my laptop. Does the attached patch fix it for you?
I have managed to reproduce this twice on a laptop running Linux
5.10.0-2-amd64, after trying many things for several hours. Both
times I was using ext4 in a loopback file (underlying is xfs, I had no
luck there hence hunch that I should try ext4, may not be significant
though) with fsync=off (ditto).
> If that's the cause, it is easy to fix by taking the RelationMappingLock
> in load_relmap_file(), like in the attached patch. But if the write is
> not atomic, you might have a bigger problem: we also rely on the
> atomicity when writing the pg_control file. If that becomes corrupt
> because of a partial write, the server won't start up. If it's just a
> race condition between the read/write, or only the read() is not atomic,
> maybe pg_control is OK, but I'd like to understand the issue better
> before just adding a lock to load_relmap_file().
Your analysis seems right to me. We have to worry about both things:
atomicity of writes on power failure (assumed to be sector-level,
hence our 512 byte struct -- all good), and atomicity of concurrent
reads and writes (we can't assume anything at all, so r/w locking is
the simplest way to get a consistent read). Shouldn't relmap_redo()
also acquire the lock exclusively?
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum"
2021-06-18 15:00 BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" PG Bug reporting form <noreply@postgresql.org>
2021-06-22 09:30 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Heikki Linnakangas <hlinnaka@iki.fi>
2021-06-22 13:00 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
@ 2021-06-22 14:11 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-06-23 00:33 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Michael Paquier <michael@paquier.xyz>
2021-06-23 00:50 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
1 sibling, 2 replies; 16+ messages in thread
From: Tom Lane @ 2021-06-22 14:11 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Thomas Munro <thomas.munro@gmail.com> writes:
> Your analysis seems right to me. We have to worry about both things:
> atomicity of writes on power failure (assumed to be sector-level,
> hence our 512 byte struct -- all good), and atomicity of concurrent
> reads and writes (we can't assume anything at all, so r/w locking is
> the simplest way to get a consistent read). Shouldn't relmap_redo()
> also acquire the lock exclusively?
Shouldn't we instead file a kernel bug report? I seem to recall that
POSIX guarantees atomicity of these things up to some operation size.
Or is that just for pipe I/O?
If we can't assume atomicity of relmapper file I/O, I wonder about
pg_control as well. But on the whole, what I'm smelling is a moderately
recently introduced kernel bug. We've been doing this this way for
years and heard no previous reports.
regards, tom lane
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum"
2021-06-18 15:00 BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" PG Bug reporting form <noreply@postgresql.org>
2021-06-22 09:30 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Heikki Linnakangas <hlinnaka@iki.fi>
2021-06-22 13:00 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
2021-06-22 14:11 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-06-23 00:33 ` Michael Paquier <michael@paquier.xyz>
2021-06-23 00:41 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 1 reply; 16+ messages in thread
From: Michael Paquier @ 2021-06-23 00:33 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Thomas Munro <thomas.munro@gmail.com>; Heikki Linnakangas <hlinnaka@iki.fi>; Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Tue, Jun 22, 2021 at 10:11:06AM -0400, Tom Lane wrote:
> Thomas Munro <thomas.munro@gmail.com> writes:
>> Your analysis seems right to me. We have to worry about both things:
>> atomicity of writes on power failure (assumed to be sector-level,
>> hence our 512 byte struct -- all good), and atomicity of concurrent
>> reads and writes (we can't assume anything at all, so r/w locking is
>> the simplest way to get a consistent read). Shouldn't relmap_redo()
>> also acquire the lock exclusively?
You are implying anything calling write_relmap_file(), right?
> Shouldn't we instead file a kernel bug report? I seem to recall that
> POSIX guarantees atomicity of these things up to some operation size.
> Or is that just for pipe I/O?
Even if this is recognized as a bug report, it seems to me that we'd
better cope with an extra lock for instances that may run into this
issue anyway in the future, no? Just to be on the safe side.
> If we can't assume atomicity of relmapper file I/O, I wonder about
> pg_control as well. But on the whole, what I'm smelling is a moderately
> recently introduced kernel bug. We've been doing this this way for
> years and heard no previous reports.
True. PG_CONTROL_MAX_SAFE_SIZE relies on that. Now, the only things
updating the control file are the startup process and the checkpointer
so that's less prone to conflicts contrary to the reported problem
here, and the code takes a ControlFileLock where necessary.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum"
2021-06-18 15:00 BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" PG Bug reporting form <noreply@postgresql.org>
2021-06-22 09:30 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Heikki Linnakangas <hlinnaka@iki.fi>
2021-06-22 13:00 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
2021-06-22 14:11 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Tom Lane <tgl@sss.pgh.pa.us>
2021-06-23 00:33 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Michael Paquier <michael@paquier.xyz>
@ 2021-06-23 00:41 ` Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 16+ messages in thread
From: Tom Lane @ 2021-06-23 00:41 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Thomas Munro <thomas.munro@gmail.com>; Heikki Linnakangas <hlinnaka@iki.fi>; Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Michael Paquier <michael@paquier.xyz> writes:
> On Tue, Jun 22, 2021 at 10:11:06AM -0400, Tom Lane wrote:
>> Shouldn't we instead file a kernel bug report? I seem to recall that
>> POSIX guarantees atomicity of these things up to some operation size.
>> Or is that just for pipe I/O?
> Even if this is recognized as a bug report, it seems to me that we'd
> better cope with an extra lock for instances that may run into this
> issue anyway in the future, no? Just to be on the safe side.
Based on the evidence at hand, we can't really say whether the writes
are non-atomic. If that's the case then we have much worse problems
than just needing to add a lock. Thus, I think we need to get some
kernel people involved.
regards, tom lane
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum"
2021-06-18 15:00 BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" PG Bug reporting form <noreply@postgresql.org>
2021-06-22 09:30 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Heikki Linnakangas <hlinnaka@iki.fi>
2021-06-22 13:00 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
2021-06-22 14:11 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-06-23 00:50 ` Thomas Munro <thomas.munro@gmail.com>
2021-06-23 02:29 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
2021-06-23 07:45 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Heikki Linnakangas <hlinnaka@iki.fi>
1 sibling, 2 replies; 16+ messages in thread
From: Thomas Munro @ 2021-06-23 00:50 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Wed, Jun 23, 2021 at 2:11 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Thomas Munro <thomas.munro@gmail.com> writes:
> > Your analysis seems right to me. We have to worry about both things:
> > atomicity of writes on power failure (assumed to be sector-level,
> > hence our 512 byte struct -- all good), and atomicity of concurrent
> > reads and writes (we can't assume anything at all, so r/w locking is
> > the simplest way to get a consistent read). Shouldn't relmap_redo()
> > also acquire the lock exclusively?
>
> Shouldn't we instead file a kernel bug report? I seem to recall that
> POSIX guarantees atomicity of these things up to some operation size.
> Or is that just for pipe I/O?
The spec doesn't cover us according to some opinions, at least:
https://utcc.utoronto.ca/~cks/space/blog/unix/WriteNotVeryAtomic
But at the same time, the behaviour seems quite surprising given the
parameters involved and how at least I thought this stuff worked in
practice (ie what the rules about the visibility of writes that
precede reads imply for the unspoken locking rule that must be the
obvious reasonable implementation, and the reality of the inode-level
read/write locking plainly visible in the source). It's possible that
it's not working as designed in some weird edge case. I guess the
next thing to do is write a minimal repro and find an expert to ask
about what it's supposed to do.
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum"
2021-06-18 15:00 BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" PG Bug reporting form <noreply@postgresql.org>
2021-06-22 09:30 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Heikki Linnakangas <hlinnaka@iki.fi>
2021-06-22 13:00 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
2021-06-22 14:11 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Tom Lane <tgl@sss.pgh.pa.us>
2021-06-23 00:50 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
@ 2021-06-23 02:29 ` Thomas Munro <thomas.munro@gmail.com>
2021-06-23 03:00 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Alexander Lakhin <exclusion@gmail.com>
1 sibling, 1 reply; 16+ messages in thread
From: Thomas Munro @ 2021-06-23 02:29 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Wed, Jun 23, 2021 at 12:50 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> repro
Here's a quick-and-dirty repro -- just touch test-file and then run
it. It fails about 0.1% of its read loops on ext4, for me, but
outputs nothing on other file systems and OSes I tried (XFS, FBSD UFS,
APFS). Seems a bit like there is a tendency for 64 byte (cache line?)
granularity in the mashed data it prints out.
Attachments:
concurrent-read-write.c (text/x-csrc, 1.5K) download
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <unistd.h>
#define LOOPS 1000000
#define BYTES 512
static void
writer(void)
{
char buffer1[BYTES];
char buffer2[BYTES];
int fd;
memset(buffer1, '1', sizeof(buffer1));
memset(buffer2, '2', sizeof(buffer2));
fd = open("test-file", O_WRONLY | O_CREAT, 0644);
if (fd < 0) {
perror("open");
exit(EXIT_FAILURE);
}
for (int i = 0; i < LOOPS; ++i) {
if (pwrite(fd, i % 2 == 0 ? buffer1 : buffer2, BYTES, 0) != 512) {
perror("pwrite");
break;
}
}
close(fd);
}
static void
reader(void)
{
char buffer[BYTES];
char buffer1[BYTES];
char buffer2[BYTES];
int fd;
memset(buffer1, '1', sizeof(buffer1));
memset(buffer2, '2', sizeof(buffer2));
fd = open("test-file", O_RDONLY);
if (fd < 0) {
perror("open");
return;
}
for (int i = 0; i < LOOPS; ++i) {
ssize_t r;
r = pread(fd, buffer, BYTES, 0);
if (r == 0)
continue;
else if (r != BYTES) {
perror("pread");
break;
}
if (memcmp(buffer, buffer1, BYTES) != 0 &&
memcmp(buffer, buffer2, BYTES) != 0) {
for (int i = 0; i < BYTES; ++i)
putchar(buffer[i]);
putchar('\n');
}
}
close(fd);
return;
}
int main(int argc, char *argv[])
{
int pid = fork();
if (pid < 0) {
perror("fork");
return EXIT_FAILURE;
} else if (pid == 0) {
reader();
} else {
int status;
writer();
waitpid(pid, &status, 0);
}
return EXIT_SUCCESS;
}
* Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum"
2021-06-18 15:00 BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" PG Bug reporting form <noreply@postgresql.org>
2021-06-22 09:30 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Heikki Linnakangas <hlinnaka@iki.fi>
2021-06-22 13:00 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
2021-06-22 14:11 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Tom Lane <tgl@sss.pgh.pa.us>
2021-06-23 00:50 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
2021-06-23 02:29 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
@ 2021-06-23 03:00 ` Alexander Lakhin <exclusion@gmail.com>
0 siblings, 0 replies; 16+ messages in thread
From: Alexander Lakhin @ 2021-06-23 03:00 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
23.06.2021 05:29, Thomas Munro wrote:
> On Wed, Jun 23, 2021 at 12:50 PM Thomas Munro <thomas.munro@gmail.com> wrote:
>> repro
> Here's a quick-and-dirty repro -- just touch test-file and then run
> it. It fails about 0.1% of its read loops on ext4, for me, but
> outputs nothing on other file systems and OSes I tried (XFS, FBSD UFS,
> APFS). Seems a bit like there is a tendency for 64 byte (cache line?)
> granularity in the mashed data it prints out.
Yes, this repro fails for me too (on ext4).
11111111111111111111111111111111222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222
11111111111111111111111111111111111111111111111111111111111111111111111111111111222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222221111111111111111111111111111111122222222222222222222222222222222111111111111111111111111111111112222222222222222222222222222222211111111111111111111111111111111222222222222222222222222222222222222222222222222
11111111111111111111111111111111111111111111111111111111111111112222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222
But granularity is 16-byte. (I've seen the fail count 19763 out of
1000000 loops.)
Best regards,
Alexander
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum"
2021-06-18 15:00 BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" PG Bug reporting form <noreply@postgresql.org>
2021-06-22 09:30 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Heikki Linnakangas <hlinnaka@iki.fi>
2021-06-22 13:00 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
2021-06-22 14:11 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Tom Lane <tgl@sss.pgh.pa.us>
2021-06-23 00:50 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
@ 2021-06-23 07:45 ` Heikki Linnakangas <hlinnaka@iki.fi>
2021-06-23 09:45 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
1 sibling, 1 reply; 16+ messages in thread
From: Heikki Linnakangas @ 2021-06-23 07:45 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On 23/06/2021 03:50, Thomas Munro wrote:
> On Wed, Jun 23, 2021 at 2:11 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Thomas Munro <thomas.munro@gmail.com> writes:
>>> Your analysis seems right to me. We have to worry about both things:
>>> atomicity of writes on power failure (assumed to be sector-level,
>>> hence our 512 byte struct -- all good), and atomicity of concurrent
>>> reads and writes (we can't assume anything at all, so r/w locking is
>>> the simplest way to get a consistent read). Shouldn't relmap_redo()
>>> also acquire the lock exclusively?
>>
>> Shouldn't we instead file a kernel bug report? I seem to recall that
>> POSIX guarantees atomicity of these things up to some operation size.
>> Or is that just for pipe I/O?
>
> The spec doesn't cover us according to some opinions, at least:
>
> https://utcc.utoronto.ca/~cks/space/blog/unix/WriteNotVeryAtomic
>
> But at the same time, the behaviour seems quite surprising given the
> parameters involved and how at least I thought this stuff worked in
> practice (ie what the rules about the visibility of writes that
> precede reads imply for the unspoken locking rule that must be the
> obvious reasonable implementation, and the reality of the inode-level
> read/write locking plainly visible in the source). It's possible that
> it's not working as designed in some weird edge case. I guess the
> next thing to do is write a minimal repro and find an expert to ask
> about what it's supposed to do.
That would be nice. At this point, though, I'm convinced at this point
that the POSIX doesn't give the guarantees we want, or even if it does,
there are a lot of systems out there that don't respect that. Do we rely
on that anywhere else than in load_relmap_file()? I don't think we do.
Let's just add the lock there.
Now, that leaves the question with pg_control. That's a different
situation. It doesn't rely on read() and write() being atomic across
processes, but on a 512 sector write not being torn on power failure.
How strong is that guarantee? It used to be common wisdom with hard
drives, and it was carried over to SSDs although I'm not sure if it was
ever strictly speaking guaranteed. What about the new kid on the block:
Persistent Memory? I found this article:
https://lwn.net/Articles/686150/. So at hardware level, Persistent
Memory only guarantees atomicity at cache line level (64 bytes). To
provide the traditional 512 byte sector atomicity, there's a feature in
Linux called BTT. Perhaps we should add a note to the docs that you
should enable that.
We haven't heard of broken control files from the field, so that doesn't
seem to be a problem in practice, at least not yet. Still, I would sleep
better if the control file had more redundancy. For example, have two
copies of it on disk. At startup, read both copies, and if they're both
valid, ignore the one with older timestamp. When updating it, write over
the older copy. That way, if you crash in the middle of updating it, the
old copy is still intact.
- Heikki
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum"
2021-06-18 15:00 BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" PG Bug reporting form <noreply@postgresql.org>
2021-06-22 09:30 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Heikki Linnakangas <hlinnaka@iki.fi>
2021-06-22 13:00 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
2021-06-22 14:11 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Tom Lane <tgl@sss.pgh.pa.us>
2021-06-23 00:50 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
2021-06-23 07:45 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Heikki Linnakangas <hlinnaka@iki.fi>
@ 2021-06-23 09:45 ` Thomas Munro <thomas.munro@gmail.com>
2021-06-24 07:52 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Heikki Linnakangas <hlinnaka@iki.fi>
0 siblings, 1 reply; 16+ messages in thread
From: Thomas Munro @ 2021-06-23 09:45 UTC (permalink / raw)
To: Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Wed, Jun 23, 2021 at 7:46 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> Let's just add the lock there.
+1, no doubt about that.
> Now, that leaves the question with pg_control. That's a different
> situation. It doesn't rely on read() and write() being atomic across
> processes, but on a 512 sector write not being torn on power failure.
> How strong is that guarantee? It used to be common wisdom with hard
> drives, and it was carried over to SSDs although I'm not sure if it was
> ever strictly speaking guaranteed. ...
Right, it's always been tacit, no standard relevant to userspace
mentions any of this AFAIK.
> ... What about the new kid on the block:
> Persistent Memory? I found this article:
> https://lwn.net/Articles/686150/. So at hardware level, Persistent
> Memory only guarantees atomicity at cache line level (64 bytes). To
> provide the traditional 512 byte sector atomicity, there's a feature in
> Linux called BTT. Perhaps we should add a note to the docs that you
> should enable that.
Right, also called sector mode. I don't know enough about that to
comment really, but... if my google-fu is serving me, you can't
actually use interesting sector sizes like 8KB (you have to choose 512
or 4096 bytes), so you'll have to pay for *two* synthetic atomic page
schemes: BTT and our full page writes. That makes me wonder... if you
need to leave full page writes on anyway, maybe it would be a better
trade-off to do double writes of our special atomic files (relmapper
files and control file) so that we could safely turn BTT off and avoid
double-taxation for relation data. Just a thought. No pmem
experience here, I could be way off.
> We haven't heard of broken control files from the field, so that doesn't
> seem to be a problem in practice, at least not yet. Still, I would sleep
> better if the control file had more redundancy. For example, have two
> copies of it on disk. At startup, read both copies, and if they're both
> valid, ignore the one with older timestamp. When updating it, write over
> the older copy. That way, if you crash in the middle of updating it, the
> old copy is still intact.
+1, with a flush in between so that only one can be borked no matter
how the storage works. It is interesting how few reports there are on
the mailing list of a control file CRC check failures though, if I'm
searching for the right thing[1].
[1] https://www.postgresql.org/search/?m=1&q=calculated+CRC+checksum+does+not+match+value+stored+in+...
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum"
2021-06-18 15:00 BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" PG Bug reporting form <noreply@postgresql.org>
2021-06-22 09:30 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Heikki Linnakangas <hlinnaka@iki.fi>
2021-06-22 13:00 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
2021-06-22 14:11 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Tom Lane <tgl@sss.pgh.pa.us>
2021-06-23 00:50 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
2021-06-23 07:45 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Heikki Linnakangas <hlinnaka@iki.fi>
2021-06-23 09:45 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
@ 2021-06-24 07:52 ` Heikki Linnakangas <hlinnaka@iki.fi>
2021-06-24 08:06 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 16+ messages in thread
From: Heikki Linnakangas @ 2021-06-24 07:52 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On 23/06/2021 12:45, Thomas Munro wrote:
> On Wed, Jun 23, 2021 at 7:46 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>> Let's just add the lock there.
>
> +1, no doubt about that.
Committed that. Thanks for the report, Alexander!
>> ... What about the new kid on the block:
>> Persistent Memory? I found this article:
>> https://lwn.net/Articles/686150/. So at hardware level, Persistent
>> Memory only guarantees atomicity at cache line level (64 bytes). To
>> provide the traditional 512 byte sector atomicity, there's a feature in
>> Linux called BTT. Perhaps we should add a note to the docs that you
>> should enable that.
>
> Right, also called sector mode. I don't know enough about that to
> comment really, but... if my google-fu is serving me, you can't
> actually use interesting sector sizes like 8KB (you have to choose 512
> or 4096 bytes), so you'll have to pay for *two* synthetic atomic page
> schemes: BTT and our full page writes. That makes me wonder... if you
> need to leave full page writes on anyway, maybe it would be a better
> trade-off to do double writes of our special atomic files (relmapper
> files and control file) so that we could safely turn BTT off and avoid
> double-taxation for relation data. Just a thought. No pmem
> experience here, I could be way off.
Yeah, you wouldn't want to turn on BTT for anything else than the
pg_control file. That's the only place where we rely on sector
atomicity, I believe. For everything else, it just adds overhead. Not
sure how much overhead; maybe it doesn't matter in practice.
>> We haven't heard of broken control files from the field, so that doesn't
>> seem to be a problem in practice, at least not yet. Still, I would sleep
>> better if the control file had more redundancy. For example, have two
>> copies of it on disk. At startup, read both copies, and if they're both
>> valid, ignore the one with older timestamp. When updating it, write over
>> the older copy. That way, if you crash in the middle of updating it, the
>> old copy is still intact.
>
> +1, with a flush in between so that only one can be borked no matter
> how the storage works. It is interesting how few reports there are on
> the mailing list of a control file CRC check failures though, if I'm
> searching for the right thing[1].
>
> [1] https://www.postgresql.org/search/?m=1&q=calculated+CRC+checksum+does+not+match+value+stored+in+...
If anyone wants a write a patch for that, I'd be happy to review it. And
if anyone has access to a system with pmem hardware, it would be
interesting to try to reproduce a torn sector and broken control file by
pulling the power plug.
- Heikki
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum"
2021-06-18 15:00 BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" PG Bug reporting form <noreply@postgresql.org>
2021-06-22 09:30 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Heikki Linnakangas <hlinnaka@iki.fi>
2021-06-22 13:00 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
2021-06-22 14:11 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Tom Lane <tgl@sss.pgh.pa.us>
2021-06-23 00:50 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
2021-06-23 07:45 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Heikki Linnakangas <hlinnaka@iki.fi>
2021-06-23 09:45 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
2021-06-24 07:52 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Heikki Linnakangas <hlinnaka@iki.fi>
@ 2021-06-24 08:06 ` Thomas Munro <thomas.munro@gmail.com>
2021-06-24 08:31 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Heikki Linnakangas <hlinnaka@iki.fi>
0 siblings, 1 reply; 16+ messages in thread
From: Thomas Munro @ 2021-06-24 08:06 UTC (permalink / raw)
To: Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Thu, Jun 24, 2021 at 7:52 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> On 23/06/2021 12:45, Thomas Munro wrote:
> > On Wed, Jun 23, 2021 at 7:46 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> >> Let's just add the lock there.
> >
> > +1, no doubt about that.
>
> Committed that. Thanks for the report, Alexander!
I think you missed relmap_redo (including a misleading comment).
> If anyone wants a write a patch for that, I'd be happy to review it. And
> if anyone has access to a system with pmem hardware, it would be
> interesting to try to reproduce a torn sector and broken control file by
> pulling the power plug.
I have been working on a kind of experimental file system for
simulating torn sectors (and other interesting file system phenomena)
as part of some work on recovery scenerio testing, not quite ready to
share yet but it can simulate that exact failure...
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum"
2021-06-18 15:00 BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" PG Bug reporting form <noreply@postgresql.org>
2021-06-22 09:30 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Heikki Linnakangas <hlinnaka@iki.fi>
2021-06-22 13:00 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
2021-06-22 14:11 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Tom Lane <tgl@sss.pgh.pa.us>
2021-06-23 00:50 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
2021-06-23 07:45 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Heikki Linnakangas <hlinnaka@iki.fi>
2021-06-23 09:45 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
2021-06-24 07:52 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Heikki Linnakangas <hlinnaka@iki.fi>
2021-06-24 08:06 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
@ 2021-06-24 08:31 ` Heikki Linnakangas <hlinnaka@iki.fi>
2021-06-28 02:45 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 16+ messages in thread
From: Heikki Linnakangas @ 2021-06-24 08:31 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On 24/06/2021 11:06, Thomas Munro wrote:
> On Thu, Jun 24, 2021 at 7:52 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>> On 23/06/2021 12:45, Thomas Munro wrote:
>>> On Wed, Jun 23, 2021 at 7:46 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>>>> Let's just add the lock there.
>>>
>>> +1, no doubt about that.
>>
>> Committed that. Thanks for the report, Alexander!
>
> I think you missed relmap_redo (including a misleading comment).
Fixed, thanks!
>> If anyone wants a write a patch for that, I'd be happy to review it. And
>> if anyone has access to a system with pmem hardware, it would be
>> interesting to try to reproduce a torn sector and broken control file by
>> pulling the power plug.
>
> I have been working on a kind of experimental file system for
> simulating torn sectors (and other interesting file system phenomena)
> as part of some work on recovery scenerio testing, not quite ready to
> share yet but it can simulate that exact failure...
Cool! We know what happens if pg_control file is torn, though. What I'd
like to know is whether it can happen in practice with pmem, an how
likely it is. For curiosity mostly, I think we have already established
that it can happen, and it would be nice to protect against it in any
case, even if it's rare.
- Heikki
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum"
2021-06-18 15:00 BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" PG Bug reporting form <noreply@postgresql.org>
2021-06-22 09:30 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Heikki Linnakangas <hlinnaka@iki.fi>
2021-06-22 13:00 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
2021-06-22 14:11 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Tom Lane <tgl@sss.pgh.pa.us>
2021-06-23 00:50 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
2021-06-23 07:45 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Heikki Linnakangas <hlinnaka@iki.fi>
2021-06-23 09:45 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
2021-06-24 07:52 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Heikki Linnakangas <hlinnaka@iki.fi>
2021-06-24 08:06 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
2021-06-24 08:31 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Heikki Linnakangas <hlinnaka@iki.fi>
@ 2021-06-28 02:45 ` Thomas Munro <thomas.munro@gmail.com>
0 siblings, 0 replies; 16+ messages in thread
From: Thomas Munro @ 2021-06-28 02:45 UTC (permalink / raw)
To: Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
> We haven't heard of broken control files from the field, so that doesn't
> seem to be a problem in practice, at least not yet. Still, I would sleep
> better if the control file had more redundancy. For example, have two
> copies of it on disk. At startup, read both copies, and if they're both
> valid, ignore the one with older timestamp. When updating it, write over
> the older copy. That way, if you crash in the middle of updating it, the
> old copy is still intact.
Seems like a good idea. I somehow doubt that accessing pmem through
old school read()/write() interfaces is the future of databases, but
ideally this should work correctly, and the dependency is indeed
unnecessary if we are prepared to jump through more hoops in just a
couple of places. There may also be other benefits. In hindsight,
it's a bit strange that we don't have explicit documentation of this
requirement. There is some related (and rather dated) discussion of
sectors in wal.sgml but nothing to say that we need 512 byte atomic
sectors for correct operation, unless I've managed to miss it (even
though it's well known among people who read the source code).
I experimented with a slightly different approach, attached, and a TAP
test to exercise it. Instead of alternating between two copies, I
tried writing out both copies every time with a synchronisation
barrier in between (the same double-write principle some other
database uses to deal with torn data pages). I think it's mostly
equivalent to your scheme, though the updates are of course slower. I
was thinking that there may be other benefits to having two copies of
the "current" version around, for resilience (though perhaps they
should be in separate files, not done here), and maybe it's better to
avoid having to invent a timestamp scheme. Or maybe the two ideas
should be combined: when both CRC checks pass, you could still be more
careful which one you choose than I have been here. Or maybe trying
to be resilient against handwavy unknown forms of corruption is a
waste of time. I'm not proposing anything here, I was just trying out
ideas, for discussion.
Attachments:
0001-Remove-control-file-dependency-on-512-byte-sectors.patch (text/x-patch, 14.6K) download
From 4240fd71ced2fca770b802cf08b03cab7a2ee89f Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Mon, 28 Jun 2021 12:03:02 +1200
Subject: [PATCH] Remove control file dependency on 512 byte sectors.
Instead of making requirements about sector-level atomicity on power
loss, use a double-write strategy. We write out two copies of the
control file data with a synchronization barrier in between. Only one
of them can be torn by power loss, on an overwrite file system.
XXX WIP
XXX pg_filenode.map has the same problem
Discussion: https://postgr.es/m/17064-bb0d7904ef72add3%40postgresql.org
---
src/backend/access/transam/xlog.c | 118 +++++++++++++++---
src/common/controldata_utils.c | 81 ++++++------
src/include/catalog/pg_control.h | 8 --
.../recovery/t/026_corrupted_control_file.pl | 94 ++++++++++++++
4 files changed, 236 insertions(+), 65 deletions(-)
create mode 100644 src/test/recovery/t/026_corrupted_control_file.pl
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 1b3a3d9bea..3442d5d95a 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -4660,10 +4660,8 @@ WriteControlFile(void)
* Ensure that the size of the pg_control data structure is sane. See the
* comments for these symbols in pg_control.h.
*/
- StaticAssertStmt(sizeof(ControlFileData) <= PG_CONTROL_MAX_SAFE_SIZE,
- "pg_control is too large for atomic disk writes");
- StaticAssertStmt(sizeof(ControlFileData) <= PG_CONTROL_FILE_SIZE,
- "sizeof(ControlFileData) exceeds PG_CONTROL_FILE_SIZE");
+ StaticAssertStmt(sizeof(ControlFileData) * 2 <= PG_CONTROL_FILE_SIZE,
+ "sizeof(ControlFileData) * 2 exceeds PG_CONTROL_FILE_SIZE");
/*
* Initialize version and compatibility-check fields
@@ -4704,6 +4702,14 @@ WriteControlFile(void)
memset(buffer, 0, PG_CONTROL_FILE_SIZE);
memcpy(buffer, ControlFile, sizeof(ControlFileData));
+ /*
+ * Make second copy of the control file data. No need to synchronize the
+ * file in between copies (as UpdateControlFile() does), because this path
+ * is used during bootstrapping only.
+ */
+ memcpy(buffer + sizeof(ControlFileData),
+ ControlFile, sizeof(ControlFileData));
+
fd = BasicOpenFile(XLOG_CONTROL_FILE,
O_RDWR | O_CREAT | O_EXCL | PG_BINARY);
if (fd < 0)
@@ -4744,6 +4750,8 @@ WriteControlFile(void)
static void
ReadControlFile(void)
{
+ ControlFileData control_file_array[2];
+ bool crc_good[2];
pg_crc32c crc;
int fd;
static char wal_segsz_str[20];
@@ -4761,8 +4769,8 @@ ReadControlFile(void)
XLOG_CONTROL_FILE)));
pgstat_report_wait_start(WAIT_EVENT_CONTROL_FILE_READ);
- r = read(fd, ControlFile, sizeof(ControlFileData));
- if (r != sizeof(ControlFileData))
+ r = read(fd, control_file_array, sizeof(control_file_array));
+ if (r != sizeof(control_file_array))
{
if (r < 0)
ereport(PANIC,
@@ -4773,12 +4781,98 @@ ReadControlFile(void)
ereport(PANIC,
(errcode(ERRCODE_DATA_CORRUPTED),
errmsg("could not read file \"%s\": read %d of %zu",
- XLOG_CONTROL_FILE, r, sizeof(ControlFileData))));
+ XLOG_CONTROL_FILE, r, sizeof(control_file_array))));
}
pgstat_report_wait_end();
close(fd);
+ /*
+ * We read two copies. Usually they are identical, but they might not be
+ * if there was a crash while UpdateControlFile() was writing them out.
+ * They were written with a synchronization barrier between them, so we can
+ * assume that at least one of them was written out fully, without having
+ * to make any assumptions about things like sectors and atomicity on
+ * overwrite filesystems.
+ */
+ for (int i = 0; i < 2; ++i)
+ {
+ INIT_CRC32C(crc);
+ COMP_CRC32C(crc, &control_file_array[i],
+ offsetof(ControlFileData, crc));
+ FIN_CRC32C(crc);
+ crc_good[i] = EQ_CRC32C(crc, control_file_array[i].crc);
+ }
+
+ if (crc_good[0])
+ {
+ /* The first copy is good, so that's the one we'll use to start up. */
+ *ControlFile = control_file_array[0];
+
+ /*
+ * There is a hazard if the second copy is corrupted, though:
+ * UpdateControlFile() writes first then second. If we leave the
+ * second copy corrupted now, another power loss event could corrupt
+ * the first copy, leaving no good copies on the disk. So, if the
+ * second copy is corrupted, fix it now.
+ */
+ if (!crc_good[1])
+ {
+ elog(WARNING, "incorrect checksum for second copy of control data, correcting");
+ fd = BasicOpenFile(XLOG_CONTROL_FILE, O_RDWR | PG_BINARY);
+ if (fd < 0)
+ ereport(PANIC,
+ (errcode_for_file_access(),
+ errmsg("could not open file \"%s\": %m",
+ XLOG_CONTROL_FILE)));
+ r = pg_pwrite(fd, ControlFile, sizeof(*ControlFile),
+ sizeof(*ControlFile));
+ if (r != sizeof(*ControlFile))
+ {
+ if (r < 0)
+ ereport(PANIC,
+ (errcode_for_file_access(),
+ errmsg("could not write file \"%s\": %m",
+ XLOG_CONTROL_FILE)));
+ else
+ ereport(PANIC,
+ (errcode(ERRCODE_DATA_CORRUPTED),
+ errmsg("could not write file \"%s\": wrote %d of %zu",
+ XLOG_CONTROL_FILE, r, sizeof(*ControlFile))));
+ }
+ if (pg_fsync(openLogFile) != 0)
+ ereport(PANIC,
+ (errcode_for_file_access(),
+ errmsg("could not fsync file \"%s\": %m",
+ XLOG_CONTROL_FILE)));
+ close(fd);
+ }
+ }
+ else if (crc_good[1])
+ {
+ /* The second copy is good, so that's the one we'll use to start up. */
+ *ControlFile = control_file_array[1];
+ elog(WARNING, "incorrect checksum for first copy of control data, using second copy");
+
+ /*
+ * We don't have to worry about correcting the corrupted first copy.
+ * The next call to UpdateControlFile() will overwrite the first copy
+ * first, so there won't be a risk of both copies being corrupted on
+ * overwrite filesystems.
+ */
+ }
+ else
+ {
+ /*
+ * Both copies failed CRC checks, so we can't start up. We'll use the
+ * first one for the checks on file format version that we perform
+ * next, because only its version is expected to be at the same offset
+ * in all versions.
+ */
+ *ControlFile = control_file_array[0];
+ elog(WARNING, "incorrect checksum for both copies of control data");
+ }
+
/*
* Check for expected pg_control format version. If this is wrong, the
* CRC check will likely fail because we'll be checking the wrong number
@@ -4803,14 +4897,8 @@ ReadControlFile(void)
ControlFile->pg_control_version, PG_CONTROL_VERSION),
errhint("It looks like you need to initdb.")));
- /* Now check the CRC. */
- INIT_CRC32C(crc);
- COMP_CRC32C(crc,
- (char *) ControlFile,
- offsetof(ControlFileData, crc));
- FIN_CRC32C(crc);
-
- if (!EQ_CRC32C(crc, ControlFile->crc))
+ /* Now report CRC failure. */
+ if (!crc_good[0] && !crc_good[1])
ereport(FATAL,
(errmsg("incorrect checksum in control file")));
diff --git a/src/common/controldata_utils.c b/src/common/controldata_utils.c
index 7d4af7881e..844f1639fa 100644
--- a/src/common/controldata_utils.c
+++ b/src/common/controldata_utils.c
@@ -44,6 +44,10 @@
* Get controlfile values. The result is returned as a palloc'd copy of the
* control file data.
*
+ * XXX Unlike the backend's ReadControlFile(), this function does not currently
+ * consider the second copy of ControlFileData and will simply report CRC check
+ * failure if the first copy is corrupted.
+ *
* crc_ok_p can be used by the caller to see whether the CRC of the control
* file data is correct.
*/
@@ -157,16 +161,13 @@ update_controlfile(const char *DataDir,
ControlFileData *ControlFile, bool do_sync)
{
int fd;
- char buffer[PG_CONTROL_FILE_SIZE];
char ControlFilePath[MAXPGPATH];
/*
* Apply the same static assertions as in backend's WriteControlFile().
*/
- StaticAssertStmt(sizeof(ControlFileData) <= PG_CONTROL_MAX_SAFE_SIZE,
- "pg_control is too large for atomic disk writes");
- StaticAssertStmt(sizeof(ControlFileData) <= PG_CONTROL_FILE_SIZE,
- "sizeof(ControlFileData) exceeds PG_CONTROL_FILE_SIZE");
+ StaticAssertStmt(sizeof(ControlFileData) * 2 <= PG_CONTROL_FILE_SIZE,
+ "sizeof(ControlFileData) * 2 exceeds PG_CONTROL_FILE_SIZE");
/* Recalculate CRC of control file */
INIT_CRC32C(ControlFile->crc);
@@ -175,14 +176,6 @@ update_controlfile(const char *DataDir,
offsetof(ControlFileData, crc));
FIN_CRC32C(ControlFile->crc);
- /*
- * Write out PG_CONTROL_FILE_SIZE bytes into pg_control by zero-padding
- * the excess over sizeof(ControlFileData), to avoid premature EOF related
- * errors when reading it.
- */
- memset(buffer, 0, PG_CONTROL_FILE_SIZE);
- memcpy(buffer, ControlFile, sizeof(ControlFileData));
-
snprintf(ControlFilePath, sizeof(ControlFilePath), "%s/%s", DataDir, XLOG_CONTROL_FILE);
#ifndef FRONTEND
@@ -205,47 +198,51 @@ update_controlfile(const char *DataDir,
}
#endif
- errno = 0;
-#ifndef FRONTEND
- pgstat_report_wait_start(WAIT_EVENT_CONTROL_FILE_WRITE_UPDATE);
-#endif
- if (write(fd, buffer, PG_CONTROL_FILE_SIZE) != PG_CONTROL_FILE_SIZE)
+ /* Write out two copies with a sychronization barrier in between. */
+ for (int i = 0; i < 2; ++i)
{
- /* if write didn't set errno, assume problem is no disk space */
- if (errno == 0)
- errno = ENOSPC;
-
-#ifndef FRONTEND
- ereport(PANIC,
- (errcode_for_file_access(),
- errmsg("could not write file \"%s\": %m",
- ControlFilePath)));
-#else
- pg_log_fatal("could not write file \"%s\": %m", ControlFilePath);
- exit(EXIT_FAILURE);
-#endif
- }
+ errno = 0;
#ifndef FRONTEND
- pgstat_report_wait_end();
+ pgstat_report_wait_start(WAIT_EVENT_CONTROL_FILE_WRITE_UPDATE);
#endif
+ if (write(fd, ControlFile, sizeof(*ControlFile)) != sizeof(*ControlFile))
+ {
+ /* if write didn't set errno, assume problem is no disk space */
+ if (errno == 0)
+ errno = ENOSPC;
- if (do_sync)
- {
#ifndef FRONTEND
- pgstat_report_wait_start(WAIT_EVENT_CONTROL_FILE_SYNC_UPDATE);
- if (pg_fsync(fd) != 0)
ereport(PANIC,
(errcode_for_file_access(),
- errmsg("could not fsync file \"%s\": %m",
+ errmsg("could not write file \"%s\": %m",
ControlFilePath)));
- pgstat_report_wait_end();
#else
- if (fsync(fd) != 0)
- {
- pg_log_fatal("could not fsync file \"%s\": %m", ControlFilePath);
+ pg_log_fatal("could not write file \"%s\": %m", ControlFilePath);
exit(EXIT_FAILURE);
+#endif
}
+#ifndef FRONTEND
+ pgstat_report_wait_end();
#endif
+
+ if (do_sync)
+ {
+#ifndef FRONTEND
+ pgstat_report_wait_start(WAIT_EVENT_CONTROL_FILE_SYNC_UPDATE);
+ if (pg_fsync(fd) != 0)
+ ereport(PANIC,
+ (errcode_for_file_access(),
+ errmsg("could not fsync file \"%s\": %m",
+ ControlFilePath)));
+ pgstat_report_wait_end();
+#else
+ if (fsync(fd) != 0)
+ {
+ pg_log_fatal("could not fsync file \"%s\": %m", ControlFilePath);
+ exit(EXIT_FAILURE);
+ }
+#endif
+ }
}
if (close(fd) != 0)
diff --git a/src/include/catalog/pg_control.h b/src/include/catalog/pg_control.h
index e3f48158ce..24d98441d1 100644
--- a/src/include/catalog/pg_control.h
+++ b/src/include/catalog/pg_control.h
@@ -230,14 +230,6 @@ typedef struct ControlFileData
pg_crc32c crc;
} ControlFileData;
-/*
- * Maximum safe value of sizeof(ControlFileData). For reliability's sake,
- * it's critical that pg_control updates be atomic writes. That generally
- * means the active data can't be more than one disk sector, which is 512
- * bytes on common hardware. Be very careful about raising this limit.
- */
-#define PG_CONTROL_MAX_SAFE_SIZE 512
-
/*
* Physical size of the pg_control file. Note that this is considerably
* bigger than the actually used size (ie, sizeof(ControlFileData)).
diff --git a/src/test/recovery/t/026_corrupted_control_file.pl b/src/test/recovery/t/026_corrupted_control_file.pl
new file mode 100644
index 0000000000..03a42c79d0
--- /dev/null
+++ b/src/test/recovery/t/026_corrupted_control_file.pl
@@ -0,0 +1,94 @@
+
+# Copyright (c) 2021, PostgreSQL Global Development Group
+
+use strict;
+use warnings;
+use PostgresNode;
+use TestLib;
+use Test::More;
+use Config;
+
+plan tests => 8;
+
+# find $pat in logfile of $node after $off-th byte
+# XXX: function copied from t/019_repslot_limit.pl
+sub find_in_log
+{
+ my ($node, $pat, $off) = @_;
+
+ $off = 0 unless defined $off;
+ my $log = TestLib::slurp_file($node->logfile);
+ return 0 if (length($log) <= $off);
+
+ $log = substr($log, $off);
+
+ return $log =~ m/$pat/;
+}
+
+my $psql_timeout = IPC::Run::timer(60);
+
+my $node = get_new_node('test');
+my $control_file = $node->data_dir . "/global/pg_control";
+$node->init();
+
+# We start up without warnings if there is no corruption.
+my $logstart = -s $node->logfile;
+$node->start();
+$node->stop();
+ok(!find_in_log($node, "incorrect checksum"),
+ "both copies ok #1");
+
+# Corrupt the first copy of the control data... we can still start up, using
+# the second copy.
+open(my $file, '+<', $control_file);
+print $file "................";
+close($file);
+$logstart = -s $node->logfile;
+$node->start();
+$node->stop();
+ok(find_in_log($node, "incorrect checksum for first copy of control data", $logstart),
+ "first copy bad");
+
+# There should be no corruption now.
+$logstart = -s $node->logfile;
+$node->start();
+$node->stop();
+ok(!find_in_log($node, "incorrect checksum", $logstart),
+ "both copies ok #2");
+
+# Corrupt the second copy of the control data. We don't want to hard-code its
+# location in this test, so we'll just grab a chunk of bytes from the front of
+# the control file, and then search for the second instance of that string and
+# corrupt it.
+my $control_file_contents = slurp_file($control_file);
+my $header = substr $control_file_contents, 0, 64;
+my $index = rindex $control_file_contents, $header;
+ok($index > 0, "second copy exists");
+open($file, '+<', $control_file);
+seek($file, $index, 0);
+print $file "................";
+close($file);
+$logstart = -s $node->logfile;
+$node->start();
+$node->stop();
+ok(find_in_log($node, "incorrect checksum for second copy of control data, correcting", $logstart),
+ "second copy bad");
+
+# There should be no corruption now.
+$logstart = -s $node->logfile;
+$node->start();
+$node->stop();
+ok(!find_in_log($node, "incorrect checksum", $logstart),
+ "both copies ok #3");
+
+# Corrupt both copies. Now we can't start up!
+my $control_file_size = -s $control_file;
+open($file, '+<', $control_file);
+print $file ("." x $control_file_size);
+close($file);
+$logstart = -s $node->logfile;
+$node->start(fail_ok => 1);
+ok(find_in_log($node, "incorrect checksum for both copies of control data", $logstart),
+ "both copies bad 1/2");
+ok(find_in_log($node, "FATAL: database files are incompatible with server", $logstart),
+ "both copies bad 2/2");
--
2.30.2
* Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum"
2021-06-18 15:00 BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" PG Bug reporting form <noreply@postgresql.org>
2021-06-22 09:30 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Heikki Linnakangas <hlinnaka@iki.fi>
2021-06-22 13:00 ` Re: BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" Thomas Munro <thomas.munro@gmail.com>
@ 2021-06-22 17:00 ` Alexander Lakhin <exclusion@gmail.com>
1 sibling, 0 replies; 16+ messages in thread
From: Alexander Lakhin @ 2021-06-22 17:00 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Hello,
22.06.2021 16:00, Thomas Munro wrote:
> On Tue, Jun 22, 2021 at 9:30 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>> Hmm, the simplest explanation would be that the read() or write() on the
>> relmapper file is not atomic. We assume that it is, and don't use a lock
>> in load_relmap_file() because of that. Is there anything unusual about
>> the filesystem, mount options or the kernel you're using? I could not
>> reproduce this on my laptop. Does the attached patch fix it for you?
> I have managed to reproduce this twice on a laptop running Linux
> 5.10.0-2-amd64, after trying many things for several hours. Both
> times I was using ext4 in a loopback file (underlying is xfs, I had no
> luck there hence hunch that I should try ext4, may not be significant
> though) with fsync=off (ditto).
I'm sorry, I forgot that I've set "fsync=off" in my postgresql.conf (to
avoid NVME-specific slowdown on fsyncs).
It really does matter. With fsync=on the demo script passes 20
iterations successfully.
I reproduce the issue on Ubuntu 20.04 with the kernel 5.9.15, ext4
(without any specific options) on NVME storage, and Ryzen 3700x.
It was first encountered on Debian 10 with the kernel 4.19.0, ext4 on
software RAID built on NVME storage too, and Xeon 5220.
The attached patch fixes it for me (with fsync=off). 3 runs by 20
iterations completed without the error (without the patch I get the
error on the first iteration).
Best regards,
Alexander
^ permalink raw reply [nested|flat] 16+ messages in thread
end of thread, other threads:[~2021-06-28 02:45 UTC | newest]
Thread overview: 16+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-06-18 15:00 BUG #17064: Parallel VACUUM operations cause the error "global/pg_filenode.map contains incorrect checksum" PG Bug reporting form <noreply@postgresql.org>
2021-06-22 09:30 ` Heikki Linnakangas <hlinnaka@iki.fi>
2021-06-22 13:00 ` Thomas Munro <thomas.munro@gmail.com>
2021-06-22 14:11 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-06-23 00:33 ` Michael Paquier <michael@paquier.xyz>
2021-06-23 00:41 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-06-23 00:50 ` Thomas Munro <thomas.munro@gmail.com>
2021-06-23 02:29 ` Thomas Munro <thomas.munro@gmail.com>
2021-06-23 03:00 ` Alexander Lakhin <exclusion@gmail.com>
2021-06-23 07:45 ` Heikki Linnakangas <hlinnaka@iki.fi>
2021-06-23 09:45 ` Thomas Munro <thomas.munro@gmail.com>
2021-06-24 07:52 ` Heikki Linnakangas <hlinnaka@iki.fi>
2021-06-24 08:06 ` Thomas Munro <thomas.munro@gmail.com>
2021-06-24 08:31 ` Heikki Linnakangas <hlinnaka@iki.fi>
2021-06-28 02:45 ` Thomas Munro <thomas.munro@gmail.com>
2021-06-22 17:00 ` Alexander Lakhin <exclusion@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox