On Fri, Mar 30, 2018 at 10:06:46AM +0900, Kyotaro HORIGUCHI wrote: > Hello. I found that c203d6cf81 hit this and this is the rebased > version on the current master.
Okay, as this is visibly the oldest item in this commit fest, Andrew has asked me to look at a solution which would allow us to definitely close the loop for all maintained branches. In consequence, I have been looking at this problem. Here are my thoughts: - The set of errors reported on this thread are alarming, depending on the scenarios used, we could have "could not read file" stuff, or even data loss after WAL replay comes and wipes out everything. - Disabling completely the TRUNCATE optimization is definitely not cool, as there could be an impact for users. - Removing wal_level = minimal is not acceptable as well, as some people rely on this feature. - Rewriting the sync handling of heap relation files in an invasive way may be something to investigate and improve on HEAD (I am not really convinced about that actually for the optimizations discussed on this thread as this may result in more bugs than actual fixes), but that would do nothing for back-branches. Hence I propose the patch attached which disables the TRUNCATE and COPY optimizations for two cases, which are the ones actually causing problems. One solution has been presented by Simon here for COPY, which is to disable the optimization when there are no blocks on a relation with wal_level = minimal: https://www.postgresql.org/message-id/CANP8+jKN4V4MJEzFN_iEtdZ+1oM=yetxvmuu1yk4umxqy2g...@mail.gmail.com For back-patching, I find that really appealing. The second thing that the patch attached does is to tweak ExecuteTruncateGuts so as the TRUNCATE optimization never runs for wal_level = minimal. Another thing that this patch adds is a set of regression tests to stress all the various scenarios presented on this thread with table creation, INSERT, COPY and TRUNCATE running in the same transactions for both wal_level = minimal and replica, which make sure that there are no failures and no actual data loss. The test is useful anyway, as any patch presented did not present a way to test easily all the scenarios, except for a bash script present upthread, but this discarded some of the cases. I would propose that for a back-patch, except for the test which can go down easily to 9.6 but I have not tested that yet. Thoughts? -- Michael
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 3a66cb5025..78f7db07f0 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -42,6 +42,7 @@
#include "parser/parse_relation.h"
#include "port/pg_bswap.h"
#include "rewrite/rewriteHandler.h"
+#include "storage/bufmgr.h"
#include "storage/fd.h"
#include "tcop/tcopprot.h"
#include "utils/builtins.h"
@@ -2404,7 +2405,16 @@ CopyFrom(CopyState cstate)
cstate->rel->rd_newRelfilenodeSubid != InvalidSubTransactionId)
{
hi_options |= HEAP_INSERT_SKIP_FSM;
- if (!XLogIsNeeded())
+
+ /*
+ * Skip writing WAL if there have been no actions that write an init
+ * block for any of the buffers that will be touched during COPY.
+ * Since there is no way of knowing at present which ones these are,
+ * we must use a simple but effective heuristic to ensure safety of
+ * the COPY operation for all cases, which is in this case to check
+ * that the relation copied to has zero blocks.
+ */
+ if (!XLogIsNeeded() && RelationGetNumberOfBlocks(cstate->rel) == 0)
hi_options |= HEAP_INSERT_SKIP_WAL;
}
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7c0cf0d7ee..90fe27fbf9 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1562,10 +1562,15 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged,
* the table was either created in the current (sub)transaction or has
* a new relfilenode in the current (sub)transaction, then we can just
* truncate it in-place, because a rollback would cause the whole
- * table or the current physical file to be thrown away anyway.
+ * table or the current physical file to be thrown away anyway. This
+ * optimization is not safe with wal_level = minimal as there is no
+ * actual way to know which are the blocks that could have been
+ * touched by another operation done within this same transaction, be
+ * it INSERT or COPY.
*/
- if (rel->rd_createSubid == mySubid ||
- rel->rd_newRelfilenodeSubid == mySubid)
+ if (XLogIsNeeded() &&
+ (rel->rd_createSubid == mySubid ||
+ rel->rd_newRelfilenodeSubid == mySubid))
{
/* Immediate, non-rollbackable truncation is OK */
heap_truncate_one_rel(rel);
diff --git a/src/test/recovery/t/015_wal_optimize.pl b/src/test/recovery/t/015_wal_optimize.pl
new file mode 100644
index 0000000000..98a410b125
--- /dev/null
+++ b/src/test/recovery/t/015_wal_optimize.pl
@@ -0,0 +1,120 @@
+# Test WAL replay for optimized TRUNCATE and COPY records
+#
+# WAL truncation is optimized in some cases with TRUNCATE and COPY queries
+# which sometimes interact badly with the other optimizations in line with
+# several setting values of wal_level, particularly when using "minimal" or
+# "replica". The optimization may be enabled or disabled depending on the
+# scenarios dealt here, and should never result in any type of failures or
+# data loss.
+use strict;
+use warnings;
+
+use PostgresNode;
+use TestLib;
+use Test::More tests => 10;
+
+# Wrapper routine tunable for wal_level.
+sub run_wal_optimize
+{
+ my $wal_level = shift;
+
+ # Primary needs to have wal_level = minimal here
+ my $node = get_new_node("node_$wal_level");
+ $node->init;
+ $node->append_conf('postgresql.conf', qq(
+wal_level = $wal_level
+));
+ $node->start;
+
+ # Test direct truncation optimization. No tuples
+ $node->safe_psql('postgres', "
+ BEGIN;
+ CREATE TABLE test1 (id serial PRIMARY KEY);
+ TRUNCATE test1;
+ COMMIT;");
+
+ $node->stop('immediate');
+ $node->start;
+
+ my $result = $node->safe_psql('postgres', "SELECT count(*) FROM test1;");
+ is($result, qq(0),
+ "wal_level = $wal_level, optimized truncation with empty table");
+
+ # Test truncation with inserted tuples within the same transaction.
+ # Tuples inserted after the truncation should be seen.
+ $node->safe_psql('postgres', "
+ BEGIN;
+ CREATE TABLE test2 (id serial PRIMARY KEY);
+ INSERT INTO test2 VALUES (DEFAULT);
+ TRUNCATE test2;
+ INSERT INTO test2 VALUES (DEFAULT);
+ COMMIT;");
+
+ $node->stop('immediate');
+ $node->start;
+
+ $result = $node->safe_psql('postgres', "SELECT count(*) FROM test2;");
+ is($result, qq(1),
+ "wal_level = $wal_level, optimized truncation with inserted table");
+
+ # Data file for COPY query in follow-up tests.
+ my $basedir = $node->basedir;
+ my $copy_file = "$basedir/copy_data.txt";
+ TestLib::append_to_file($copy_file, qq(20000,30000
+20001,30001
+20002,30002));
+
+ # Test truncation with inserted tuples using COPY. Tuples copied after the
+ # truncation should be seen.
+ $node->safe_psql('postgres', "
+ BEGIN;
+ CREATE TABLE test3 (id serial PRIMARY KEY, id2 int);
+ INSERT INTO test3 (id, id2) VALUES (DEFAULT, generate_series(1,10000));
+ TRUNCATE test3;
+ COPY test3 FROM '$copy_file' DELIMITER ',';
+ COMMIT;");
+ $node->stop('immediate');
+ $node->start;
+ $result = $node->safe_psql('postgres', "SELECT count(*) FROM test3;");
+ is($result, qq(3),
+ "wal_level = $wal_level, optimized truncation with copied table");
+
+ # Test truncation with inserted tuples using both INSERT and COPY. Tuples
+ # inserted after the truncation should be seen.
+ $node->safe_psql('postgres', "
+ BEGIN;
+ CREATE TABLE test4 (id serial PRIMARY KEY, id2 int);
+ INSERT INTO test4 (id, id2) VALUES (DEFAULT, generate_series(1,10000));
+ TRUNCATE test4;
+ INSERT INTO test4 (id, id2) VALUES (DEFAULT, 10000);
+ COPY test4 FROM '$copy_file' DELIMITER ',';
+ INSERT INTO test4 (id, id2) VALUES (DEFAULT, 10000);
+ COMMIT;");
+
+ $node->stop('immediate');
+ $node->start;
+ $result = $node->safe_psql('postgres', "SELECT count(*) FROM test4;");
+ is($result, qq(5),
+ "wal_level = $wal_level, optimized truncation with inserted/copied table");
+
+ # Test consistency of COPY with INSERT for table created in the same
+ # transaction.
+ $node->safe_psql('postgres', "
+ BEGIN;
+ CREATE TABLE test5 (id serial PRIMARY KEY, id2 int);
+ INSERT INTO test5 VALUES (DEFAULT, 1);
+ COPY test5 FROM '$copy_file' DELIMITER ',';
+ COMMIT;");
+ $node->stop('immediate');
+ $node->start;
+ $result = $node->safe_psql('postgres', "SELECT count(*) FROM test5;");
+ is($result, qq(4),
+ "wal_level = $wal_level, replay of optimized copy with inserted table");
+
+ $node->teardown_node;
+ $node->clean_node;
+}
+
+# Run same test suite for multiple wal_level values.
+run_wal_optimize("minimal");
+run_wal_optimize("replica");
signature.asc
Description: PGP signature
