date:20120303

Re: [HACKERS] RFC: Making TRUNCATE more MVCC-safe

2012-03-03 Thread Simon Riggs

On Sat, Feb 11, 2012 at 4:46 AM, Noah Misch n...@leadboat.com wrote:

 But I have to admit I'm intrigued by the idea of extending this to
 other cases, if there's a reasonable way to do that.  For example, if
 we could fix things up so that we don't see a table at all if it was
 created after we took our snapshot, that would remove one of the
 obstacles to marking pages bulk-loaded into that table with FrozenXID
 and PD_ALL_VISIBLE from the get-go.  I think a lot of people would be
 mighty happy about that.

 Exactly.

 But the necessary semantics seem somewhat different.  For TRUNCATE,
 you want to throw a serialization error; but is that also what you
 want for CREATE TABLE?  Or do you just want it to appear empty?

 I think an error helps just as much there.  If you create a table and populate
 it with data in the same transaction, letting some snapshot see an empty table
 is an atomicity failure.

 Your comment illustrates a helpful point: this is just another kind of
 ordinary serialization failure, one that can happen at any isolation level.
 No serial transaction sequence can yield one of these errors.

Thanks Noah for drawing attention to this thread. I hadn't been
watching. As you say, this work would allow me to freeze rows at load
time and avoid the overhead of hint bit setting, which avoids
performance issues from hint bit setting in checksum patch.

I've reviewed this patch and it seems OK to me. Good work Marti.

v2 patch attached, updated to latest HEAD. Patch adds
* a GUC called strict_mvcc to isolate the new behaviour if required
* relvalidxid is reset by VACUUM/ANALYZE to avoid wraparound failure

At present this lacks docs for strict_mvcc and doesn't attempt to
handle CREATE TABLE case yet, but should be straightforward to do so.

Hint bit setting is in separate patch on other thread.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index c910863..4387f49 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -73,7 +73,7 @@
 
 /* GUC variable */
 bool		synchronize_seqscans = true;
-
+bool		StrictMVCC = true;
 
 static HeapScanDesc heap_beginscan_internal(Relation relation,
 		Snapshot snapshot,
@@ -1175,6 +1175,24 @@ heap_beginscan_internal(Relation relation, Snapshot snapshot,
 	HeapScanDesc scan;
 
 	/*
+	 * If the snapshot is older than relvalidxmin then that either a table
+	 * has only recently been created or that a TRUNCATE has removed data
+	 * that we should still be able to see. Either way, we aren't allowed
+	 * to view the rows in StrictMVCC mode.
+	 */
+	if (StrictMVCC 
+		TransactionIdIsValid(relation-rd_rel-relvalidxmin) 
+		TransactionIdIsValid(snapshot-xmax) 
+		NormalTransactionIdPrecedes(snapshot-xmax, relation-rd_rel-relvalidxmin))
+	{
+		ereport(ERROR,
+(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
+ errmsg(canceling statement due to conflict with TRUNCATE TABLE on %s,
+		 NameStr(relation-rd_rel-relname)),
+ errdetail(User query attempting to see row versions that have been removed.)));
+	}
+
+	/*
 	 * increment relation ref count while scanning relation
 	 *
 	 * This is just to make really sure the relcache entry won't go away while
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index aef410a..3f9bd5d 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -787,6 +787,7 @@ InsertPgClassTuple(Relation pg_class_desc,
 	values[Anum_pg_class_relhastriggers - 1] = BoolGetDatum(rd_rel-relhastriggers);
 	values[Anum_pg_class_relhassubclass - 1] = BoolGetDatum(rd_rel-relhassubclass);
 	values[Anum_pg_class_relfrozenxid - 1] = TransactionIdGetDatum(rd_rel-relfrozenxid);
+	values[Anum_pg_class_relvalidxmin - 1] = TransactionIdGetDatum(rd_rel-relvalidxmin);
 	if (relacl != (Datum) 0)
 		values[Anum_pg_class_relacl - 1] = relacl;
 	else
@@ -884,6 +885,7 @@ AddNewRelationTuple(Relation pg_class_desc,
 		new_rel_reltup-relfrozenxid = InvalidTransactionId;
 	}
 
+	new_rel_reltup-relvalidxmin = InvalidTransactionId;
 	new_rel_reltup-relowner = relowner;
 	new_rel_reltup-reltype = new_type_oid;
 	new_rel_reltup-reloftype = reloftype;
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index bfbe642..0d96a6a 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -2854,7 +2854,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks)
 		}
 
 		/* We'll build a new physical relation for the index */
-		RelationSetNewRelfilenode(iRel, InvalidTransactionId);
+		RelationSetNewRelfilenode(iRel, InvalidTransactionId, InvalidTransactionId);
 
 		/* Initialize the index and rebuild */
 		/* Note: we do not need to re-establish pkey setting */
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index b40e57b..0578241 100644
--- a/src/backend/commands/analyze.c
+++

Re: [HACKERS] COPY with hints, rebirth

2012-03-03 Thread Simon Riggs

On Fri, Mar 2, 2012 at 8:58 PM, Noah Misch n...@leadboat.com wrote:
On Fri, Mar 02, 2012 at 08:46:45AM +, Simon Riggs wrote:
On Thu, Mar 1, 2012 at 8:49 PM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
It's still broken:

[BEGIN;TRUNCATE;SAVEPOINT;COPY;ROLLBACK TO]

So this approach isn't the one...

The COPY FREEZE patch provides a way for the user to say explicitly
that they don't really care about these MVCC corner cases and as a
result allows us to avoid touching XidInMVCCSnapshot() at all. So
there is still a patch on the table.

You can salvage the optimization by tightening its prerequisite: use it when
the current subtransaction or a child thereof created or truncated the table.
A parent subtransaction having done so is acceptable for the WAL avoidance
optimization but not for this.

I misread your earlier comment. Yes, that will make this work correctly.

Incidentally, I contend that we should write frozen tuples to new/truncated
tables unconditionally. The current behavior of making old snapshots see the
table as empty violates atomicity at least as badly as letting those snapshots
see the future-snapshot contents. But Marti has a sound proposal that would
interact with your efforts here to avoid violating atomicity at all:
http://archives.postgresql.org/message-id/cabrt9rbrmdsoz8kxgehfb4lg-ev9u67-6dlqvoiibpkkhtl...@mail.gmail.com

Thank you for bringing this to my attention.

This will make this optimisation work correctly without adding
anything to the main code path of XidInMVCCSnapshot() and without the
annoying FREEZE syntax. So this puts the patch squarely back on the
table.

I'll do another version of this later today designed to work with the
StrictMVCC patch.

--
Simon Riggs 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

37 matches

Mail list logo