use less space in xl_xact_commit patch

Started by Leonardo Francalanciover 14 years ago32 messages
#1Leonardo Francalanci
m_lists@yahoo.it
1 attachment(s)

Hi,

following the conversation at

http://postgresql.1045698.n5.nabble.com/switch-UNLOGGED-to-LOGGED-tp4290461p4382333.html

I tried to remove some bytes from xl_xact_commit.

The way I did it needs palloc+memcpy. I guess it could be done
reusing the memory for smgrGetPendingDeletes. But I don't
think it's that important.

I guess there are other ways of doing it; let me know what
you think.

Leonardo

Attachments:

commitlog_firstv.patchapplication/octet-stream; name=commitlog_firstv.patchDownload
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
new file mode 100644
index 2812681..f552800
*** a/src/backend/access/transam/twophase.c
--- b/src/backend/access/transam/twophase.c
*************** RecordTransactionCommitPrepared(Transact
*** 1977,2014 ****
  	xlrec.xid = xid;
  	xlrec.crec.xact_time = GetCurrentTimestamp();
  	xlrec.crec.xinfo = initfileinval ? XACT_COMPLETION_UPDATE_RELCACHE_FILE : 0;
- 	xlrec.crec.nmsgs = 0;
- 	xlrec.crec.nrels = nrels;
- 	xlrec.crec.nsubxacts = nchildren;
- 	xlrec.crec.nmsgs = ninvalmsgs;
- 
  	rdata[0].data = (char *) (&xlrec);
  	rdata[0].len = MinSizeOfXactCommitPrepared;
  	rdata[0].buffer = InvalidBuffer;
  	/* dump rels to delete */
  	if (nrels > 0)
  	{
  		rdata[0].next = &(rdata[1]);
! 		rdata[1].data = (char *) rels;
! 		rdata[1].len = nrels * sizeof(RelFileNode);
  		rdata[1].buffer = InvalidBuffer;
  		lastrdata = 1;
  	}
  	/* dump committed child Xids */
  	if (nchildren > 0)
  	{
  		rdata[lastrdata].next = &(rdata[2]);
! 		rdata[2].data = (char *) children;
! 		rdata[2].len = nchildren * sizeof(TransactionId);
  		rdata[2].buffer = InvalidBuffer;
  		lastrdata = 2;
  	}
! 	/* dump cache invalidation messages */
  	if (ninvalmsgs > 0)
  	{
  		rdata[lastrdata].next = &(rdata[3]);
! 		rdata[3].data = (char *) invalmsgs;
! 		rdata[3].len = ninvalmsgs * sizeof(SharedInvalidationMessage);
  		rdata[3].buffer = InvalidBuffer;
  		lastrdata = 3;
  	}
--- 1977,2034 ----
  	xlrec.xid = xid;
  	xlrec.crec.xact_time = GetCurrentTimestamp();
  	xlrec.crec.xinfo = initfileinval ? XACT_COMPLETION_UPDATE_RELCACHE_FILE : 0;
  	rdata[0].data = (char *) (&xlrec);
  	rdata[0].len = MinSizeOfXactCommitPrepared;
  	rdata[0].buffer = InvalidBuffer;
  	/* dump rels to delete */
  	if (nrels > 0)
  	{
+ 		xl_xact_commit_opt *opt_rels = (xl_xact_commit_opt *)
+ 										palloc(MinSizeOfXactCommitOpt +
+ 											nrels * sizeof(RelFileNode));
  		rdata[0].next = &(rdata[1]);
! 		xlrec.crec.xinfo |= XACT_COMPLETION_DROP_REL_NODES_PRESENT;
! 		opt_rels->nopts = nrels;
! 		memcpy(&(opt_rels->opts), rels, nrels * sizeof(RelFileNode));
! 		rdata[1].data = (char *) opt_rels;
! 		rdata[1].len = MinSizeOfXactCommitOpt +	nrels * sizeof(RelFileNode);
  		rdata[1].buffer = InvalidBuffer;
  		lastrdata = 1;
  	}
  	/* dump committed child Xids */
  	if (nchildren > 0)
  	{
+ 		xl_xact_commit_opt *opt_child =
+ 					(xl_xact_commit_opt *)
+ 									palloc(MinSizeOfXactCommitOpt +
+ 									nchildren * sizeof(TransactionId));
  		rdata[lastrdata].next = &(rdata[2]);
! 		xlrec.crec.xinfo |= XACT_COMPLETION_COMMITTED_SUB_PRESENT;
! 
! 		opt_child->nopts = nchildren;
! 		memcpy(&(opt_child->opts), children,
! 									nchildren * sizeof(TransactionId));
! 		rdata[2].data = (char *) opt_child;
! 		rdata[2].len = MinSizeOfXactCommitOpt +
! 									nchildren * sizeof(TransactionId);
  		rdata[2].buffer = InvalidBuffer;
  		lastrdata = 2;
  	}
! 	/* dump shared cache invalidation messages */
  	if (ninvalmsgs > 0)
  	{
+ 		xl_xact_commit_opt *opt_shr =
+ 							(xl_xact_commit_opt *)
+ 								palloc(MinSizeOfXactCommitOpt +
+ 								ninvalmsgs * sizeof(SharedInvalidationMessage));
  		rdata[lastrdata].next = &(rdata[3]);
! 		xlrec.crec.xinfo |= XACT_COMPLETION_SHARED_INV_PRESENT;
! 		opt_shr->nopts = ninvalmsgs;
! 		memcpy(&(opt_shr->opts), invalmsgs,
! 							ninvalmsgs * sizeof(SharedInvalidationMessage));
! 		rdata[3].data = (char *) opt_shr;
! 		rdata[3].len = MinSizeOfXactCommitOpt +
! 								ninvalmsgs * sizeof(SharedInvalidationMessage);
  		rdata[3].buffer = InvalidBuffer;
  		lastrdata = 3;
  	}
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
new file mode 100644
index 2ca1c14..730f0de
*** a/src/backend/access/transam/xact.c
--- b/src/backend/access/transam/xact.c
*************** RecordTransactionCommit(void)
*** 1003,1038 ****
  
  		SetCurrentTransactionStopTimestamp();
  		xlrec.xact_time = xactStopTimestamp;
- 		xlrec.nrels = nrels;
- 		xlrec.nsubxacts = nchildren;
- 		xlrec.nmsgs = nmsgs;
  		rdata[0].data = (char *) (&xlrec);
  		rdata[0].len = MinSizeOfXactCommit;
  		rdata[0].buffer = InvalidBuffer;
  		/* dump rels to delete */
  		if (nrels > 0)
  		{
  			rdata[0].next = &(rdata[1]);
! 			rdata[1].data = (char *) rels;
! 			rdata[1].len = nrels * sizeof(RelFileNode);
  			rdata[1].buffer = InvalidBuffer;
  			lastrdata = 1;
  		}
  		/* dump committed child Xids */
  		if (nchildren > 0)
  		{
  			rdata[lastrdata].next = &(rdata[2]);
! 			rdata[2].data = (char *) children;
! 			rdata[2].len = nchildren * sizeof(TransactionId);
  			rdata[2].buffer = InvalidBuffer;
  			lastrdata = 2;
  		}
  		/* dump shared cache invalidation messages */
  		if (nmsgs > 0)
  		{
  			rdata[lastrdata].next = &(rdata[3]);
! 			rdata[3].data = (char *) invalMessages;
! 			rdata[3].len = nmsgs * sizeof(SharedInvalidationMessage);
  			rdata[3].buffer = InvalidBuffer;
  			lastrdata = 3;
  		}
--- 1003,1060 ----
  
  		SetCurrentTransactionStopTimestamp();
  		xlrec.xact_time = xactStopTimestamp;
  		rdata[0].data = (char *) (&xlrec);
  		rdata[0].len = MinSizeOfXactCommit;
  		rdata[0].buffer = InvalidBuffer;
  		/* dump rels to delete */
  		if (nrels > 0)
  		{
+ 			xl_xact_commit_opt *opt_rels = (xl_xact_commit_opt *)
+ 											palloc(MinSizeOfXactCommitOpt +
+ 												nrels * sizeof(RelFileNode));
  			rdata[0].next = &(rdata[1]);
! 			xlrec.xinfo |= XACT_COMPLETION_DROP_REL_NODES_PRESENT;
! 			opt_rels->nopts = nrels;
! 			memcpy(&(opt_rels->opts), rels, nrels * sizeof(RelFileNode));
! 			rdata[1].data = (char *) opt_rels;
! 			rdata[1].len = MinSizeOfXactCommitOpt +	nrels * sizeof(RelFileNode);
  			rdata[1].buffer = InvalidBuffer;
  			lastrdata = 1;
  		}
  		/* dump committed child Xids */
  		if (nchildren > 0)
  		{
+ 			xl_xact_commit_opt *opt_child =
+ 						(xl_xact_commit_opt *)
+ 										palloc(MinSizeOfXactCommitOpt +
+ 										nchildren * sizeof(TransactionId));
  			rdata[lastrdata].next = &(rdata[2]);
! 			xlrec.xinfo |= XACT_COMPLETION_COMMITTED_SUB_PRESENT;
! 
! 			opt_child->nopts = nchildren;
! 			memcpy(&(opt_child->opts), children,
! 										nchildren * sizeof(TransactionId));
! 			rdata[2].data = (char *) opt_child;
! 			rdata[2].len = MinSizeOfXactCommitOpt +
! 										nchildren * sizeof(TransactionId);
  			rdata[2].buffer = InvalidBuffer;
  			lastrdata = 2;
  		}
  		/* dump shared cache invalidation messages */
  		if (nmsgs > 0)
  		{
+ 			xl_xact_commit_opt *opt_shr =
+ 								(xl_xact_commit_opt *)
+ 										palloc(MinSizeOfXactCommitOpt +
+ 										nmsgs * sizeof(SharedInvalidationMessage));
  			rdata[lastrdata].next = &(rdata[3]);
! 			xlrec.xinfo |= XACT_COMPLETION_SHARED_INV_PRESENT;
! 			opt_shr->nopts = nmsgs;
! 			memcpy(&(opt_shr->opts), invalMessages,
! 									nmsgs * sizeof(SharedInvalidationMessage));
! 			rdata[3].data = (char *) opt_shr;
! 			rdata[3].len = MinSizeOfXactCommitOpt +
! 								nmsgs * sizeof(SharedInvalidationMessage);
  			rdata[3].buffer = InvalidBuffer;
  			lastrdata = 3;
  		}
*************** xactGetCommittedChildren(TransactionId *
*** 4436,4441 ****
--- 4458,4500 ----
   *	XLOG support routines
   */
  
+ 
+ static void
+ xact_read_commit(xl_xact_commit *xlrec,
+ 					RelFileNode **drop_rels,
+ 					TransactionId **sub_xids,
+ 					SharedInvalidationMessage **inval_msgs,
+ 					int *nrels,
+ 					int *nsubxacts,
+ 					int *nmsgs)
+ {
+ 	xl_xact_commit_opt		*nextOpts = &xlrec->xnodes;
+ 	*nrels = 0;
+ 	*nsubxacts = 0;
+ 	*nmsgs = 0;
+ 
+ 	if (XactCompletionDropRelPresent(xlrec))
+ 	{
+ 		*drop_rels = (RelFileNode *) xlrec->xnodes.opts;
+ 		*nrels = xlrec->xnodes.nopts;
+ 		nextOpts = (xl_xact_commit_opt *)
+ 								&(xlrec->xnodes.opts[*nrels]);
+ 	}
+ 
+ 	if (XactCompletionCommittedSubPresent(xlrec))
+ 	{
+ 		*sub_xids = (TransactionId *) nextOpts->opts;
+ 		*nsubxacts = nextOpts->nopts;
+ 		nextOpts = (xl_xact_commit_opt *) &(nextOpts->opts[*nsubxacts]);
+ 	}
+ 
+ 	if (XactCompletionSharedInvPresent(xlrec))
+ 	{
+ 		*inval_msgs = (SharedInvalidationMessage *) nextOpts->opts;
+ 		*nmsgs = nextOpts->nopts;
+ 	}
+ }
+ 
  /*
   * Before 9.0 this was a fairly short function, but now it performs many
   * actions for which the order of execution is critical.
*************** xactGetCommittedChildren(TransactionId *
*** 4443,4459 ****
  static void
  xact_redo_commit(xl_xact_commit *xlrec, TransactionId xid, XLogRecPtr lsn)
  {
  	TransactionId *sub_xids;
  	SharedInvalidationMessage *inval_msgs;
  	TransactionId max_xid;
  	int			i;
  
! 	/* subxid array follows relfilenodes */
! 	sub_xids = (TransactionId *) &(xlrec->xnodes[xlrec->nrels]);
! 	/* invalidation messages array follows subxids */
! 	inval_msgs = (SharedInvalidationMessage *) &(sub_xids[xlrec->nsubxacts]);
! 
! 	max_xid = TransactionIdLatest(xid, xlrec->nsubxacts, sub_xids);
  
  	/*
  	 * Make sure nextXid is beyond any XID mentioned in the record.
--- 4502,4519 ----
  static void
  xact_redo_commit(xl_xact_commit *xlrec, TransactionId xid, XLogRecPtr lsn)
  {
+ 	RelFileNode		*drop_rels;
  	TransactionId *sub_xids;
  	SharedInvalidationMessage *inval_msgs;
  	TransactionId max_xid;
  	int			i;
+ 	int 		nrels = 0;
+ 	int 		nsubxacts = 0;
+ 	int 		nmsgs = 0;
  
! 	xact_read_commit(xlrec, &drop_rels, &sub_xids, &inval_msgs, &nrels,
! 						&nsubxacts, &nmsgs);
! 	max_xid = TransactionIdLatest(xid, nsubxacts, sub_xids);
  
  	/*
  	 * Make sure nextXid is beyond any XID mentioned in the record.
*************** xact_redo_commit(xl_xact_commit *xlrec, 
*** 4476,4482 ****
  		/*
  		 * Mark the transaction committed in pg_clog.
  		 */
! 		TransactionIdCommitTree(xid, xlrec->nsubxacts, sub_xids);
  	}
  	else
  	{
--- 4536,4542 ----
  		/*
  		 * Mark the transaction committed in pg_clog.
  		 */
! 		TransactionIdCommitTree(xid, nsubxacts, sub_xids);
  	}
  	else
  	{
*************** xact_redo_commit(xl_xact_commit *xlrec, 
*** 4500,4518 ****
  		 * bits set on changes made by transactions that haven't yet
  		 * recovered. It's unlikely but it's good to be safe.
  		 */
! 		TransactionIdAsyncCommitTree(xid, xlrec->nsubxacts, sub_xids, lsn);
  
  		/*
  		 * We must mark clog before we update the ProcArray.
  		 */
! 		ExpireTreeKnownAssignedTransactionIds(xid, xlrec->nsubxacts, sub_xids, max_xid);
  
  		/*
  		 * Send any cache invalidations attached to the commit. We must
  		 * maintain the same order of invalidation then release locks as
  		 * occurs in CommitTransaction().
  		 */
! 		ProcessCommittedInvalidationMessages(inval_msgs, xlrec->nmsgs,
  								  XactCompletionRelcacheInitFileInval(xlrec),
  											 xlrec->dbId, xlrec->tsId);
  
--- 4560,4578 ----
  		 * bits set on changes made by transactions that haven't yet
  		 * recovered. It's unlikely but it's good to be safe.
  		 */
! 		TransactionIdAsyncCommitTree(xid, nsubxacts, sub_xids, lsn);
  
  		/*
  		 * We must mark clog before we update the ProcArray.
  		 */
! 		ExpireTreeKnownAssignedTransactionIds(xid, nsubxacts, sub_xids, max_xid);
  
  		/*
  		 * Send any cache invalidations attached to the commit. We must
  		 * maintain the same order of invalidation then release locks as
  		 * occurs in CommitTransaction().
  		 */
! 		ProcessCommittedInvalidationMessages(inval_msgs, nmsgs,
  								  XactCompletionRelcacheInitFileInval(xlrec),
  											 xlrec->dbId, xlrec->tsId);
  
*************** xact_redo_commit(xl_xact_commit *xlrec, 
*** 4521,4540 ****
  		 * phase transactions. In effect we are ignoring the prepare phase and
  		 * just going straight to lock release.
  		 */
! 		StandbyReleaseLockTree(xid, xlrec->nsubxacts, sub_xids);
  	}
  
  	/* Make sure files supposed to be dropped are dropped */
! 	for (i = 0; i < xlrec->nrels; i++)
  	{
! 		SMgrRelation srel = smgropen(xlrec->xnodes[i], InvalidBackendId);
  		ForkNumber	fork;
  
  		for (fork = 0; fork <= MAX_FORKNUM; fork++)
  		{
  			if (smgrexists(srel, fork))
  			{
! 				XLogDropRelation(xlrec->xnodes[i], fork);
  				smgrdounlink(srel, fork, true);
  			}
  		}
--- 4581,4600 ----
  		 * phase transactions. In effect we are ignoring the prepare phase and
  		 * just going straight to lock release.
  		 */
! 		StandbyReleaseLockTree(xid, nsubxacts, sub_xids);
  	}
  
  	/* Make sure files supposed to be dropped are dropped */
! 	for (i = 0; i < nrels; i++)
  	{
! 		SMgrRelation srel = smgropen(drop_rels[i], InvalidBackendId);
  		ForkNumber	fork;
  
  		for (fork = 0; fork <= MAX_FORKNUM; fork++)
  		{
  			if (smgrexists(srel, fork))
  			{
! 				XLogDropRelation(drop_rels[i], fork);
  				smgrdounlink(srel, fork, true);
  			}
  		}
*************** xact_desc_commit(StringInfo buf, xl_xact
*** 4704,4745 ****
  {
  	int			i;
  	TransactionId *xacts;
  
! 	xacts = (TransactionId *) &xlrec->xnodes[xlrec->nrels];
  
  	appendStringInfoString(buf, timestamptz_to_str(xlrec->xact_time));
  
! 	if (xlrec->nrels > 0)
  	{
  		appendStringInfo(buf, "; rels:");
! 		for (i = 0; i < xlrec->nrels; i++)
  		{
! 			char	   *path = relpathperm(xlrec->xnodes[i], MAIN_FORKNUM);
  
  			appendStringInfo(buf, " %s", path);
  			pfree(path);
  		}
  	}
! 	if (xlrec->nsubxacts > 0)
  	{
  		appendStringInfo(buf, "; subxacts:");
! 		for (i = 0; i < xlrec->nsubxacts; i++)
  			appendStringInfo(buf, " %u", xacts[i]);
  	}
! 	if (xlrec->nmsgs > 0)
  	{
- 		SharedInvalidationMessage *msgs;
- 
- 		msgs = (SharedInvalidationMessage *) &xacts[xlrec->nsubxacts];
- 
  		if (XactCompletionRelcacheInitFileInval(xlrec))
  			appendStringInfo(buf, "; relcache init file inval dbid %u tsid %u",
  							 xlrec->dbId, xlrec->tsId);
  
  		appendStringInfo(buf, "; inval msgs:");
! 		for (i = 0; i < xlrec->nmsgs; i++)
  		{
! 			SharedInvalidationMessage *msg = &msgs[i];
  
  			if (msg->id >= 0)
  				appendStringInfo(buf, " catcache %d", msg->id);
--- 4764,4807 ----
  {
  	int			i;
  	TransactionId *xacts;
+ 	RelFileNode		*drop_rels;
+ 	SharedInvalidationMessage *inval_msgs;
+ 	int 		nrels = 0;
+ 	int 		nsubxacts = 0;
+ 	int 		nmsgs = 0;
  
! 	xact_read_commit(xlrec, &drop_rels, &xacts, &inval_msgs, &nrels,
! 						&nsubxacts, &nmsgs);
  
  	appendStringInfoString(buf, timestamptz_to_str(xlrec->xact_time));
  
! 	if (nrels > 0)
  	{
  		appendStringInfo(buf, "; rels:");
! 		for (i = 0; i < nrels; i++)
  		{
! 			char	   *path = relpathperm(drop_rels[i], MAIN_FORKNUM);
  
  			appendStringInfo(buf, " %s", path);
  			pfree(path);
  		}
  	}
! 	if (nsubxacts > 0)
  	{
  		appendStringInfo(buf, "; subxacts:");
! 		for (i = 0; i < nsubxacts; i++)
  			appendStringInfo(buf, " %u", xacts[i]);
  	}
! 	if (nmsgs > 0)
  	{
  		if (XactCompletionRelcacheInitFileInval(xlrec))
  			appendStringInfo(buf, "; relcache init file inval dbid %u tsid %u",
  							 xlrec->dbId, xlrec->tsId);
  
  		appendStringInfo(buf, "; inval msgs:");
! 		for (i = 0; i < nmsgs; i++)
  		{
! 			SharedInvalidationMessage *msg = &inval_msgs[i];
  
  			if (msg->id >= 0)
  				appendStringInfo(buf, " catcache %d", msg->id);
diff --git a/src/include/access/xact.h b/src/include/access/xact.h
new file mode 100644
index cb440d4..9a6a7a2
*** a/src/include/access/xact.h
--- b/src/include/access/xact.h
*************** typedef struct xl_xact_assignment
*** 116,134 ****
  
  #define MinSizeOfXactAssignment offsetof(xl_xact_assignment, xsub)
  
  typedef struct xl_xact_commit
  {
  	TimestampTz xact_time;		/* time of commit */
  	uint32		xinfo;			/* info flags */
- 	int			nrels;			/* number of RelFileNodes */
- 	int			nsubxacts;		/* number of subtransaction XIDs */
- 	int			nmsgs;			/* number of shared inval msgs */
  	Oid			dbId;			/* MyDatabaseId */
  	Oid			tsId;			/* MyDatabaseTableSpace */
! 	/* Array of RelFileNode(s) to drop at commit */
! 	RelFileNode xnodes[1];		/* VARIABLE LENGTH ARRAY */
! 	/* ARRAY OF COMMITTED SUBTRANSACTION XIDs FOLLOWS */
! 	/* ARRAY OF SHARED INVALIDATION MESSAGES FOLLOWS */
  } xl_xact_commit;
  
  #define MinSizeOfXactCommit offsetof(xl_xact_commit, xnodes)
--- 116,141 ----
  
  #define MinSizeOfXactAssignment offsetof(xl_xact_assignment, xsub)
  
+ typedef struct xl_xact_commit_opt
+ {
+ 	int 	nopts;
+ 	char	opts[1];
+ } xl_xact_commit_opt;
+ 
+ #define MinSizeOfXactCommitOpt offsetof(xl_xact_commit_opt, opts)
+ 
  typedef struct xl_xact_commit
  {
  	TimestampTz xact_time;		/* time of commit */
  	uint32		xinfo;			/* info flags */
  	Oid			dbId;			/* MyDatabaseId */
  	Oid			tsId;			/* MyDatabaseTableSpace */
! 	/* Array of RelFileNode(s) to drop at commit (optional) */
! 	xl_xact_commit_opt xnodes;
! 	/* ARRAY OF OPTIONAL xl_xact_commit_opt COMMITTED SUBTRANSACTION XIDs
! 	 * FOLLOWS */
! 	/* ARRAY OF OPTIONAL xl_xact_commit_opt SHARED INVALIDATION MESSAGES
! 	 * FOLLOWS */
  } xl_xact_commit;
  
  #define MinSizeOfXactCommit offsetof(xl_xact_commit, xnodes)
*************** typedef struct xl_xact_commit
*** 148,153 ****
--- 155,174 ----
  #define XactCompletionRelcacheInitFileInval(xlrec)	((xlrec)->xinfo & XACT_COMPLETION_UPDATE_RELCACHE_FILE)
  #define XactCompletionForceSyncCommit(xlrec)		((xlrec)->xinfo & XACT_COMPLETION_FORCE_SYNC_COMMIT)
  
+ /*
+  * These flags are set in the xinfo fields of WAL commit records,
+  * indicating the presence of optionals xl_xact_commit_opt structures
+  * (RelFileNodes to drop at commit, subtransaction XIDs, shared inval msgs)
+  */
+ #define XACT_COMPLETION_DROP_REL_NODES_PRESENT	0x04
+ #define XACT_COMPLETION_COMMITTED_SUB_PRESENT	0x08
+ #define XACT_COMPLETION_SHARED_INV_PRESENT		0x10
+ 
+ /* Access macros for above flags */
+ #define XactCompletionDropRelPresent(xlrec)	((xlrec)->xinfo & XACT_COMPLETION_DROP_REL_NODES_PRESENT)
+ #define XactCompletionCommittedSubPresent(xlrec)		((xlrec)->xinfo & XACT_COMPLETION_COMMITTED_SUB_PRESENT)
+ #define XactCompletionSharedInvPresent(xlrec)		((xlrec)->xinfo & XACT_COMPLETION_SHARED_INV_PRESENT)
+ 
  typedef struct xl_xact_abort
  {
  	TimestampTz xact_time;		/* time of abort */
#2Robert Haas
robertmhaas@gmail.com
In reply to: Leonardo Francalanci (#1)
Re: use less space in xl_xact_commit patch

On Mon, May 16, 2011 at 11:20 AM, Leonardo Francalanci <m_lists@yahoo.it> wrote:

following the conversation at

http://postgresql.1045698.n5.nabble.com/switch-UNLOGGED-to-LOGGED-tp4290461p4382333.html

I tried to remove some bytes from xl_xact_commit.

The way I did it needs palloc+memcpy. I guess it could be done
reusing the memory for smgrGetPendingDeletes. But I don't
think it's that important.

I guess there are other ways of doing it; let me know what
you think.

I don't think there's much point to the xl_xact_commit_opt structure;
it doesn't really do anything. What I would do is end the
xl_xact_commit structure with something like:

int counts[1]; /* variable-length array of counts, xinfo flags define
length of array and meaning of counts */

Then, I'd make macros like this:

#define XactCommitNumberOfDroppedRelFileNodes(xlrec) \
((xlref->xinfo & XACT_COMMIT_DROPPED_RELFILENODES) ? xlrec->counts[0] : 0)
#define XactCommitNumberOfCommittedSubXids(xlrec) \
((xlref->xinfo & XACT_COMMITED_SUBXDIDS) ?
xlrec->counts[(xlrec->xinfo & XACT_COMMIT_DROPPED_RELFILENODES) ? 1 :
0] : 0)
...etc...

...and a similar set of macros that will return a pointer to the
beginning of the corresponding array, if it's present. I'd lay out
the record like this:

- main record
- array of counts (might be zero-length)
- array of dropped relfilnodes (if any)
- array of committed subxids (if any)
- array of sinval messages (if any)

Also, it's important not to confuse xact completion with xact commit,
as I think some of your naming does. Completion could perhaps be
thought to include abort.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#3Leonardo Francalanci
m_lists@yahoo.it
In reply to: Robert Haas (#2)
Re: use less space in xl_xact_commit patch

int counts[1]; /* variable-length array of counts, xinfo flags define
length of array and meaning of counts */

Damn, that's much cleaner than what I did. I don't know why
I stuck with the idea that it had to be:

int
array
int
array
...

instead of:
int
int
...
array
array
...

which makes much more sense.

Then, I'd make macros like this:

#define XactCommitNumberOfDroppedRelFileNodes(xlrec) \
((xlref->xinfo & XACT_COMMIT_DROPPED_RELFILENODES) ? xlrec->counts[0] : 0)
#define XactCommitNumberOfCommittedSubXids(xlrec) \
((xlref->xinfo & XACT_COMMITED_SUBXDIDS) ?
xlrec->counts[(xlrec->xinfo & XACT_COMMIT_DROPPED_RELFILENODES) ? 1 :
0] : 0)
...etc...

ehm I don't know if macros would be enough; that's ok
for the first 2, then I think it would become a mess...
Maybe I'll use a simple function that gets all "ints" at once.

...and a similar set of macros that will return a pointer to the
beginning of the corresponding array, if it's present. I'd lay out
the record like this:

- main record
- array of counts (might be zero-length)
- array of dropped relfilnodes (if any)
- array of committed subxids (if any)
- array of sinval messages (if any)

ok

Also, it's important not to confuse xact completion with xact commit,
as I think some of your naming does. Completion could perhaps be
thought to include abort.

mmh... I don't know if I got it... but I'll give a look, and ask questions...

Thank you very much for the help

Leonardo

#4Leonardo Francalanci
m_lists@yahoo.it
In reply to: Leonardo Francalanci (#3)
1 attachment(s)
Re: use less space in xl_xact_commit patch

this is a second version: now using

int counts[1]; /* variable-length array of counts */

in xl_xact_commit to keep track of number of

different arrays at the end of the struct.

Waiting for feedbacks...

Leonardo

Attachments:

commitlog_lessbytes00.patchapplication/octet-stream; name=commitlog_lessbytes00.patchDownload
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
new file mode 100644
index 2812681..f2b88a5
*** a/src/backend/access/transam/twophase.c
--- b/src/backend/access/transam/twophase.c
*************** RecordTransactionCommitPrepared(Transact
*** 1964,1972 ****
  								bool initfileinval)
  {
  	XLogRecData rdata[4];
- 	int			lastrdata = 0;
- 	xl_xact_commit_prepared xlrec;
  	XLogRecPtr	recptr;
  
  	START_CRIT_SECTION();
  
--- 1964,1987 ----
  								bool initfileinval)
  {
  	XLogRecData rdata[4];
  	XLogRecPtr	recptr;
+ 	int			lastrdata = 0;
+ 	xl_xact_commit_prepared *xlrec;
+ 	int xlrec_len = MinSizeOfXactCommitPrepared;
+ 	if (nrels > 0)
+ 	{
+ 		xlrec_len += sizeof(int);
+ 	}
+ 	if (nchildren > 0)
+ 	{
+ 		xlrec_len += sizeof(int);
+ 	}
+ 	if (ninvalmsgs > 0)
+ 	{
+ 		xlrec_len += sizeof(int);
+ 	}
+ 
+ 	xlrec = (xl_xact_commit_prepared *)palloc(xlrec_len);
  
  	START_CRIT_SECTION();
  
*************** RecordTransactionCommitPrepared(Transact
*** 1974,1994 ****
  	MyProc->inCommit = true;
  
  	/* Emit the XLOG commit record */
! 	xlrec.xid = xid;
! 	xlrec.crec.xact_time = GetCurrentTimestamp();
! 	xlrec.crec.xinfo = initfileinval ? XACT_COMPLETION_UPDATE_RELCACHE_FILE : 0;
! 	xlrec.crec.nmsgs = 0;
! 	xlrec.crec.nrels = nrels;
! 	xlrec.crec.nsubxacts = nchildren;
! 	xlrec.crec.nmsgs = ninvalmsgs;
! 
! 	rdata[0].data = (char *) (&xlrec);
! 	rdata[0].len = MinSizeOfXactCommitPrepared;
  	rdata[0].buffer = InvalidBuffer;
  	/* dump rels to delete */
  	if (nrels > 0)
  	{
  		rdata[0].next = &(rdata[1]);
  		rdata[1].data = (char *) rels;
  		rdata[1].len = nrels * sizeof(RelFileNode);
  		rdata[1].buffer = InvalidBuffer;
--- 1989,2006 ----
  	MyProc->inCommit = true;
  
  	/* Emit the XLOG commit record */
! 	xlrec->xid = xid;
! 	xlrec->crec.xact_time = GetCurrentTimestamp();
! 	xlrec->crec.xinfo = initfileinval ? XACT_COMPLETION_UPDATE_RELCACHE_FILE : 0;
! 	rdata[0].data = (char *) (xlrec);
! 	rdata[0].len = xlrec_len;
  	rdata[0].buffer = InvalidBuffer;
  	/* dump rels to delete */
  	if (nrels > 0)
  	{
  		rdata[0].next = &(rdata[1]);
+ 		xlrec->crec.xinfo |= XACT_COMPLETION_DROP_REL_NODES_PRESENT;
+ 		xlrec->crec.counts[0] = nrels;
  		rdata[1].data = (char *) rels;
  		rdata[1].len = nrels * sizeof(RelFileNode);
  		rdata[1].buffer = InvalidBuffer;
*************** RecordTransactionCommitPrepared(Transact
*** 1998,2012 ****
  	if (nchildren > 0)
  	{
  		rdata[lastrdata].next = &(rdata[2]);
  		rdata[2].data = (char *) children;
  		rdata[2].len = nchildren * sizeof(TransactionId);
  		rdata[2].buffer = InvalidBuffer;
  		lastrdata = 2;
  	}
! 	/* dump cache invalidation messages */
  	if (ninvalmsgs > 0)
  	{
  		rdata[lastrdata].next = &(rdata[3]);
  		rdata[3].data = (char *) invalmsgs;
  		rdata[3].len = ninvalmsgs * sizeof(SharedInvalidationMessage);
  		rdata[3].buffer = InvalidBuffer;
--- 2010,2028 ----
  	if (nchildren > 0)
  	{
  		rdata[lastrdata].next = &(rdata[2]);
+ 		xlrec->crec.xinfo |= XACT_COMPLETION_COMMITTED_SUB_PRESENT;
+ 		xlrec->crec.counts[lastrdata] = nchildren;
  		rdata[2].data = (char *) children;
  		rdata[2].len = nchildren * sizeof(TransactionId);
  		rdata[2].buffer = InvalidBuffer;
  		lastrdata = 2;
  	}
! 	/* dump shared cache invalidation messages */
  	if (ninvalmsgs > 0)
  	{
  		rdata[lastrdata].next = &(rdata[3]);
+ 		xlrec->crec.counts[lastrdata] = ninvalmsgs;
+ 		xlrec->crec.xinfo |= XACT_COMPLETION_SHARED_INV_PRESENT;
  		rdata[3].data = (char *) invalmsgs;
  		rdata[3].len = ninvalmsgs * sizeof(SharedInvalidationMessage);
  		rdata[3].buffer = InvalidBuffer;
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
new file mode 100644
index 2ca1c14..6e750cc
*** a/src/backend/access/transam/xact.c
--- b/src/backend/access/transam/xact.c
*************** RecordTransactionCommit(void)
*** 964,970 ****
  		 */
  		XLogRecData rdata[4];
  		int			lastrdata = 0;
! 		xl_xact_commit xlrec;
  
  		/* Tell bufmgr and smgr to prepare for commit */
  		BufmgrCommit();
--- 964,985 ----
  		 */
  		XLogRecData rdata[4];
  		int			lastrdata = 0;
! 		xl_xact_commit *xlrec;
! 		int xlrec_len = MinSizeOfXactCommit;
! 		if (nrels > 0)
! 		{
! 			xlrec_len += sizeof(int);
! 		}
! 		if (nchildren > 0)
! 		{
! 			xlrec_len += sizeof(int);
! 		}
! 		if (nmsgs > 0)
! 		{
! 			xlrec_len += sizeof(int);
! 		}
! 
! 		xlrec = (xl_xact_commit *)palloc(xlrec_len);
  
  		/* Tell bufmgr and smgr to prepare for commit */
  		BufmgrCommit();
*************** RecordTransactionCommit(void)
*** 972,985 ****
  		/*
  		 * Set flags required for recovery processing of commits.
  		 */
! 		xlrec.xinfo = 0;
  		if (RelcacheInitFileInval)
! 			xlrec.xinfo |= XACT_COMPLETION_UPDATE_RELCACHE_FILE;
  		if (forceSyncCommit)
! 			xlrec.xinfo |= XACT_COMPLETION_FORCE_SYNC_COMMIT;
  
! 		xlrec.dbId = MyDatabaseId;
! 		xlrec.tsId = MyDatabaseTableSpace;
  
  		/*
  		 * Mark ourselves as within our "commit critical section".	This
--- 987,1000 ----
  		/*
  		 * Set flags required for recovery processing of commits.
  		 */
! 		xlrec->xinfo = 0;
  		if (RelcacheInitFileInval)
! 			xlrec->xinfo |= XACT_COMPLETION_UPDATE_RELCACHE_FILE;
  		if (forceSyncCommit)
! 			xlrec->xinfo |= XACT_COMPLETION_FORCE_SYNC_COMMIT;
  
! 		xlrec->dbId = MyDatabaseId;
! 		xlrec->tsId = MyDatabaseTableSpace;
  
  		/*
  		 * Mark ourselves as within our "commit critical section".	This
*************** RecordTransactionCommit(void)
*** 1002,1018 ****
  		MyProc->inCommit = true;
  
  		SetCurrentTransactionStopTimestamp();
! 		xlrec.xact_time = xactStopTimestamp;
! 		xlrec.nrels = nrels;
! 		xlrec.nsubxacts = nchildren;
! 		xlrec.nmsgs = nmsgs;
! 		rdata[0].data = (char *) (&xlrec);
! 		rdata[0].len = MinSizeOfXactCommit;
  		rdata[0].buffer = InvalidBuffer;
  		/* dump rels to delete */
  		if (nrels > 0)
  		{
  			rdata[0].next = &(rdata[1]);
  			rdata[1].data = (char *) rels;
  			rdata[1].len = nrels * sizeof(RelFileNode);
  			rdata[1].buffer = InvalidBuffer;
--- 1017,1032 ----
  		MyProc->inCommit = true;
  
  		SetCurrentTransactionStopTimestamp();
! 		xlrec->xact_time = xactStopTimestamp;
! 		rdata[0].data = (char *) (xlrec);
! 		rdata[0].len = xlrec_len;
  		rdata[0].buffer = InvalidBuffer;
  		/* dump rels to delete */
  		if (nrels > 0)
  		{
  			rdata[0].next = &(rdata[1]);
+ 			xlrec->xinfo |= XACT_COMPLETION_DROP_REL_NODES_PRESENT;
+ 			xlrec->counts[0] = nrels;
  			rdata[1].data = (char *) rels;
  			rdata[1].len = nrels * sizeof(RelFileNode);
  			rdata[1].buffer = InvalidBuffer;
*************** RecordTransactionCommit(void)
*** 1022,1027 ****
--- 1036,1043 ----
  		if (nchildren > 0)
  		{
  			rdata[lastrdata].next = &(rdata[2]);
+ 			xlrec->xinfo |= XACT_COMPLETION_COMMITTED_SUB_PRESENT;
+ 			xlrec->counts[lastrdata] = nchildren;
  			rdata[2].data = (char *) children;
  			rdata[2].len = nchildren * sizeof(TransactionId);
  			rdata[2].buffer = InvalidBuffer;
*************** RecordTransactionCommit(void)
*** 1031,1036 ****
--- 1047,1054 ----
  		if (nmsgs > 0)
  		{
  			rdata[lastrdata].next = &(rdata[3]);
+ 			xlrec->counts[lastrdata] = nmsgs;
+ 			xlrec->xinfo |= XACT_COMPLETION_SHARED_INV_PRESENT;
  			rdata[3].data = (char *) invalMessages;
  			rdata[3].len = nmsgs * sizeof(SharedInvalidationMessage);
  			rdata[3].buffer = InvalidBuffer;
*************** xactGetCommittedChildren(TransactionId *
*** 4436,4441 ****
--- 4454,4497 ----
   *	XLOG support routines
   */
  
+ 
+ static void
+ xact_read_commit(xl_xact_commit *xlrec,
+ 					RelFileNode **drop_rels,
+ 					TransactionId **sub_xids,
+ 					SharedInvalidationMessage **inval_msgs,
+ 					int *nrels,
+ 					int *nsubxacts,
+ 					int *nmsgs)
+ {
+ 	int lastpos = 0;
+ 	*nrels = 0;
+ 	*nsubxacts = 0;
+ 	*nmsgs = 0;
+ 	if (XactCompletionDropRelPresent(xlrec))
+ 	{
+ 		*nrels = xlrec->counts[lastpos];
+ 		lastpos++;
+ 	}
+ 
+ 	if (XactCompletionCommittedSubPresent(xlrec))
+ 	{
+ 		*nsubxacts = xlrec->counts[lastpos];
+ 		lastpos++;
+ 	}
+ 
+ 	if (XactCompletionSharedInvPresent(xlrec))
+ 	{
+ 		*nmsgs = xlrec->counts[lastpos];
+ 		lastpos++;
+ 	}
+ 
+ 
+ 	*drop_rels = (RelFileNode *) &xlrec->counts[lastpos];
+ 	*sub_xids = (TransactionId *) &drop_rels[*nrels];
+ 	*inval_msgs = (SharedInvalidationMessage *) &sub_xids[*nsubxacts];
+ }
+ 
  /*
   * Before 9.0 this was a fairly short function, but now it performs many
   * actions for which the order of execution is critical.
*************** xactGetCommittedChildren(TransactionId *
*** 4443,4459 ****
  static void
  xact_redo_commit(xl_xact_commit *xlrec, TransactionId xid, XLogRecPtr lsn)
  {
  	TransactionId *sub_xids;
  	SharedInvalidationMessage *inval_msgs;
  	TransactionId max_xid;
  	int			i;
  
! 	/* subxid array follows relfilenodes */
! 	sub_xids = (TransactionId *) &(xlrec->xnodes[xlrec->nrels]);
! 	/* invalidation messages array follows subxids */
! 	inval_msgs = (SharedInvalidationMessage *) &(sub_xids[xlrec->nsubxacts]);
! 
! 	max_xid = TransactionIdLatest(xid, xlrec->nsubxacts, sub_xids);
  
  	/*
  	 * Make sure nextXid is beyond any XID mentioned in the record.
--- 4499,4516 ----
  static void
  xact_redo_commit(xl_xact_commit *xlrec, TransactionId xid, XLogRecPtr lsn)
  {
+ 	RelFileNode		*drop_rels;
  	TransactionId *sub_xids;
  	SharedInvalidationMessage *inval_msgs;
  	TransactionId max_xid;
  	int			i;
+ 	int 		nrels = 0;
+ 	int 		nsubxacts = 0;
+ 	int 		nmsgs = 0;
  
! 	xact_read_commit(xlrec, &drop_rels, &sub_xids, &inval_msgs, &nrels,
! 						&nsubxacts, &nmsgs);
! 	max_xid = TransactionIdLatest(xid, nsubxacts, sub_xids);
  
  	/*
  	 * Make sure nextXid is beyond any XID mentioned in the record.
*************** xact_redo_commit(xl_xact_commit *xlrec, 
*** 4476,4482 ****
  		/*
  		 * Mark the transaction committed in pg_clog.
  		 */
! 		TransactionIdCommitTree(xid, xlrec->nsubxacts, sub_xids);
  	}
  	else
  	{
--- 4533,4539 ----
  		/*
  		 * Mark the transaction committed in pg_clog.
  		 */
! 		TransactionIdCommitTree(xid, nsubxacts, sub_xids);
  	}
  	else
  	{
*************** xact_redo_commit(xl_xact_commit *xlrec, 
*** 4500,4518 ****
  		 * bits set on changes made by transactions that haven't yet
  		 * recovered. It's unlikely but it's good to be safe.
  		 */
! 		TransactionIdAsyncCommitTree(xid, xlrec->nsubxacts, sub_xids, lsn);
  
  		/*
  		 * We must mark clog before we update the ProcArray.
  		 */
! 		ExpireTreeKnownAssignedTransactionIds(xid, xlrec->nsubxacts, sub_xids, max_xid);
  
  		/*
  		 * Send any cache invalidations attached to the commit. We must
  		 * maintain the same order of invalidation then release locks as
  		 * occurs in CommitTransaction().
  		 */
! 		ProcessCommittedInvalidationMessages(inval_msgs, xlrec->nmsgs,
  								  XactCompletionRelcacheInitFileInval(xlrec),
  											 xlrec->dbId, xlrec->tsId);
  
--- 4557,4575 ----
  		 * bits set on changes made by transactions that haven't yet
  		 * recovered. It's unlikely but it's good to be safe.
  		 */
! 		TransactionIdAsyncCommitTree(xid, nsubxacts, sub_xids, lsn);
  
  		/*
  		 * We must mark clog before we update the ProcArray.
  		 */
! 		ExpireTreeKnownAssignedTransactionIds(xid, nsubxacts, sub_xids, max_xid);
  
  		/*
  		 * Send any cache invalidations attached to the commit. We must
  		 * maintain the same order of invalidation then release locks as
  		 * occurs in CommitTransaction().
  		 */
! 		ProcessCommittedInvalidationMessages(inval_msgs, nmsgs,
  								  XactCompletionRelcacheInitFileInval(xlrec),
  											 xlrec->dbId, xlrec->tsId);
  
*************** xact_redo_commit(xl_xact_commit *xlrec, 
*** 4521,4540 ****
  		 * phase transactions. In effect we are ignoring the prepare phase and
  		 * just going straight to lock release.
  		 */
! 		StandbyReleaseLockTree(xid, xlrec->nsubxacts, sub_xids);
  	}
  
  	/* Make sure files supposed to be dropped are dropped */
! 	for (i = 0; i < xlrec->nrels; i++)
  	{
! 		SMgrRelation srel = smgropen(xlrec->xnodes[i], InvalidBackendId);
  		ForkNumber	fork;
  
  		for (fork = 0; fork <= MAX_FORKNUM; fork++)
  		{
  			if (smgrexists(srel, fork))
  			{
! 				XLogDropRelation(xlrec->xnodes[i], fork);
  				smgrdounlink(srel, fork, true);
  			}
  		}
--- 4578,4597 ----
  		 * phase transactions. In effect we are ignoring the prepare phase and
  		 * just going straight to lock release.
  		 */
! 		StandbyReleaseLockTree(xid, nsubxacts, sub_xids);
  	}
  
  	/* Make sure files supposed to be dropped are dropped */
! 	for (i = 0; i < nrels; i++)
  	{
! 		SMgrRelation srel = smgropen(drop_rels[i], InvalidBackendId);
  		ForkNumber	fork;
  
  		for (fork = 0; fork <= MAX_FORKNUM; fork++)
  		{
  			if (smgrexists(srel, fork))
  			{
! 				XLogDropRelation(drop_rels[i], fork);
  				smgrdounlink(srel, fork, true);
  			}
  		}
*************** xact_desc_commit(StringInfo buf, xl_xact
*** 4704,4745 ****
  {
  	int			i;
  	TransactionId *xacts;
  
! 	xacts = (TransactionId *) &xlrec->xnodes[xlrec->nrels];
  
  	appendStringInfoString(buf, timestamptz_to_str(xlrec->xact_time));
  
! 	if (xlrec->nrels > 0)
  	{
  		appendStringInfo(buf, "; rels:");
! 		for (i = 0; i < xlrec->nrels; i++)
  		{
! 			char	   *path = relpathperm(xlrec->xnodes[i], MAIN_FORKNUM);
  
  			appendStringInfo(buf, " %s", path);
  			pfree(path);
  		}
  	}
! 	if (xlrec->nsubxacts > 0)
  	{
  		appendStringInfo(buf, "; subxacts:");
! 		for (i = 0; i < xlrec->nsubxacts; i++)
  			appendStringInfo(buf, " %u", xacts[i]);
  	}
! 	if (xlrec->nmsgs > 0)
  	{
- 		SharedInvalidationMessage *msgs;
- 
- 		msgs = (SharedInvalidationMessage *) &xacts[xlrec->nsubxacts];
- 
  		if (XactCompletionRelcacheInitFileInval(xlrec))
  			appendStringInfo(buf, "; relcache init file inval dbid %u tsid %u",
  							 xlrec->dbId, xlrec->tsId);
  
  		appendStringInfo(buf, "; inval msgs:");
! 		for (i = 0; i < xlrec->nmsgs; i++)
  		{
! 			SharedInvalidationMessage *msg = &msgs[i];
  
  			if (msg->id >= 0)
  				appendStringInfo(buf, " catcache %d", msg->id);
--- 4761,4804 ----
  {
  	int			i;
  	TransactionId *xacts;
+ 	RelFileNode		*drop_rels;
+ 	SharedInvalidationMessage *inval_msgs;
+ 	int 		nrels = 0;
+ 	int 		nsubxacts = 0;
+ 	int 		nmsgs = 0;
  
! 	xact_read_commit(xlrec, &drop_rels, &xacts, &inval_msgs, &nrels,
! 						&nsubxacts, &nmsgs);
  
  	appendStringInfoString(buf, timestamptz_to_str(xlrec->xact_time));
  
! 	if (nrels > 0)
  	{
  		appendStringInfo(buf, "; rels:");
! 		for (i = 0; i < nrels; i++)
  		{
! 			char	   *path = relpathperm(drop_rels[i], MAIN_FORKNUM);
  
  			appendStringInfo(buf, " %s", path);
  			pfree(path);
  		}
  	}
! 	if (nsubxacts > 0)
  	{
  		appendStringInfo(buf, "; subxacts:");
! 		for (i = 0; i < nsubxacts; i++)
  			appendStringInfo(buf, " %u", xacts[i]);
  	}
! 	if (nmsgs > 0)
  	{
  		if (XactCompletionRelcacheInitFileInval(xlrec))
  			appendStringInfo(buf, "; relcache init file inval dbid %u tsid %u",
  							 xlrec->dbId, xlrec->tsId);
  
  		appendStringInfo(buf, "; inval msgs:");
! 		for (i = 0; i < nmsgs; i++)
  		{
! 			SharedInvalidationMessage *msg = &inval_msgs[i];
  
  			if (msg->id >= 0)
  				appendStringInfo(buf, " catcache %d", msg->id);
diff --git a/src/include/access/xact.h b/src/include/access/xact.h
new file mode 100644
index cb440d4..63fe1ca
*** a/src/include/access/xact.h
--- b/src/include/access/xact.h
*************** typedef struct xl_xact_commit
*** 120,137 ****
  {
  	TimestampTz xact_time;		/* time of commit */
  	uint32		xinfo;			/* info flags */
- 	int			nrels;			/* number of RelFileNodes */
- 	int			nsubxacts;		/* number of subtransaction XIDs */
- 	int			nmsgs;			/* number of shared inval msgs */
  	Oid			dbId;			/* MyDatabaseId */
  	Oid			tsId;			/* MyDatabaseTableSpace */
! 	/* Array of RelFileNode(s) to drop at commit */
! 	RelFileNode xnodes[1];		/* VARIABLE LENGTH ARRAY */
! 	/* ARRAY OF COMMITTED SUBTRANSACTION XIDs FOLLOWS */
! 	/* ARRAY OF SHARED INVALIDATION MESSAGES FOLLOWS */
  } xl_xact_commit;
  
! #define MinSizeOfXactCommit offsetof(xl_xact_commit, xnodes)
  
  /*
   * These flags are set in the xinfo fields of WAL commit records,
--- 120,135 ----
  {
  	TimestampTz xact_time;		/* time of commit */
  	uint32		xinfo;			/* info flags */
  	Oid			dbId;			/* MyDatabaseId */
  	Oid			tsId;			/* MyDatabaseTableSpace */
! 	int  		counts[1];  	/* variable-length array of counts, xinfo flags */
! 								/* define length of array and meaning of counts */
! 	/* ARRAY OF OPTIONAL RELATIONS TO DROP FOLLOWS */
! 	/* ARRAY OF OPTIONAL COMMITTED SUBTRANSACTION XIDs FOLLOWS */
! 	/* ARRAY OF OPTIONAL SHARED INVALIDATION MESSAGES FOLLOWS */
  } xl_xact_commit;
  
! #define MinSizeOfXactCommit offsetof(xl_xact_commit, counts)
  
  /*
   * These flags are set in the xinfo fields of WAL commit records,
*************** typedef struct xl_xact_commit
*** 148,153 ****
--- 146,165 ----
  #define XactCompletionRelcacheInitFileInval(xlrec)	((xlrec)->xinfo & XACT_COMPLETION_UPDATE_RELCACHE_FILE)
  #define XactCompletionForceSyncCommit(xlrec)		((xlrec)->xinfo & XACT_COMPLETION_FORCE_SYNC_COMMIT)
  
+ /*
+  * These flags are set in the xinfo fields of WAL commit records,
+  * indicating the presence of optionals xl_xact_commit_opt structures
+  * (RelFileNodes to drop at commit, subtransaction XIDs, shared inval msgs)
+  */
+ #define XACT_COMPLETION_DROP_REL_NODES_PRESENT	0x04
+ #define XACT_COMPLETION_COMMITTED_SUB_PRESENT	0x08
+ #define XACT_COMPLETION_SHARED_INV_PRESENT		0x10
+ 
+ /* Access macros for above flags */
+ #define XactCompletionDropRelPresent(xlrec)	((xlrec)->xinfo & XACT_COMPLETION_DROP_REL_NODES_PRESENT)
+ #define XactCompletionCommittedSubPresent(xlrec)		((xlrec)->xinfo & XACT_COMPLETION_COMMITTED_SUB_PRESENT)
+ #define XactCompletionSharedInvPresent(xlrec)		((xlrec)->xinfo & XACT_COMPLETION_SHARED_INV_PRESENT)
+ 
  typedef struct xl_xact_abort
  {
  	TimestampTz xact_time;		/* time of abort */
*************** typedef struct xl_xact_commit_prepared
*** 176,182 ****
  	/* MORE DATA FOLLOWS AT END OF STRUCT */
  } xl_xact_commit_prepared;
  
! #define MinSizeOfXactCommitPrepared offsetof(xl_xact_commit_prepared, crec.xnodes)
  
  typedef struct xl_xact_abort_prepared
  {
--- 188,194 ----
  	/* MORE DATA FOLLOWS AT END OF STRUCT */
  } xl_xact_commit_prepared;
  
! #define MinSizeOfXactCommitPrepared offsetof(xl_xact_commit_prepared, crec.counts)
  
  typedef struct xl_xact_abort_prepared
  {
#5Robert Haas
robertmhaas@gmail.com
In reply to: Leonardo Francalanci (#4)
Re: use less space in xl_xact_commit patch

On Wed, May 18, 2011 at 9:11 AM, Leonardo Francalanci <m_lists@yahoo.it> wrote:

this is a second version: now using

 int            counts[1];      /* variable-length array of counts */

in xl_xact_commit to keep track of number of

different arrays at the end of the struct.

Waiting for feedbacks...

Looks reasonable on a quick once-over; please add it to the next
CommitFest so that it gets a more detailed review.

https://commitfest.postgresql.org/action/commitfest_view/open

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#6Simon Riggs
simon@2ndQuadrant.com
In reply to: Leonardo Francalanci (#4)
Re: use less space in xl_xact_commit patch

On Wed, May 18, 2011 at 2:11 PM, Leonardo Francalanci <m_lists@yahoo.it> wrote:

this is a second version: now using

 int            counts[1];      /* variable-length array of counts */

in xl_xact_commit to keep track of number of

different arrays at the end of the struct.

Waiting for feedbacks...

I can't find a clear discussion of what you are trying to do, and how,
just a URL back to a complex discussion on another topic.

I can't see any analysis that shows whether this would be effective in
reducing space of WAL records and what the impacts might be.

The patch contains very close to zero comments.

Please explain what you are trying to do, and what the likely benefits
of doing it will be. Add comments to the patch to make that very
clear, e.g. "As of 9.2, the record format changed to reduce space..."

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

#7Leonardo Francalanci
m_lists@yahoo.it
In reply to: Simon Riggs (#6)
1 attachment(s)
Re: use less space in xl_xact_commit patch

Da: Simon Riggs <simon@2ndQuadrant.com>
I can't find a clear discussion of what you are trying to do, and how,
just a URL back to a complex discussion on another topic.

While trying to write a patch to allow changing an unlogged table into
a logged one, I had to add another int field to xl_xact_commit.
Robert Haas said:

"I have to admit I don't like this approach very much. I can't see
adding 4 bytes to every commit record for this feature."

which is a correct remark.

xl_xact_commit can contain some arrays (relation to drops,
committed sub-trans, shared invalidation msgs). The length of
these arrays is specified using 3 ints in the struct.

So, to avoid adding more ints to the struct, I've been suggested to
remove all the ints, and use xl_xact_commit.xinfo to flag which
arrays are, in fact, present.

So the whole idea is:

- remove nrels, nsubxacts and nmsgs from xl_xact_commit
- use bits in xinfo to signal which arrays are present at the end
of xl_xact_commit
- for each present array, add the length of the array (as int) at
the end of xl_xact_commit
- add each present array after all the lengths

I can't see any analysis that shows whether this would be effective in
reducing space of WAL records and what the impacts might be.

The space of WAL records should always be <= than without the patch:
in the worst case scenario, all ints are present (as they would without
the patch). In the best case, which I guess is the more common, each
xl_xact_commit will be 3 ints shorter.

I don't think the effect will be "perceivable": the whole idea is to allow
more variable-length structures in xl_xact_commit in the future,
without incrementing the space on disk.

The patch contains very close to zero comments.

I tried to add some.

Leonardo

Attachments:

commitlog_lessbytes01.patchapplication/octet-stream; name=commitlog_lessbytes01.patchDownload
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
new file mode 100644
index 2812681..f2b88a5
*** a/src/backend/access/transam/twophase.c
--- b/src/backend/access/transam/twophase.c
*************** RecordTransactionCommitPrepared(Transact
*** 1964,1972 ****
  								bool initfileinval)
  {
  	XLogRecData rdata[4];
- 	int			lastrdata = 0;
- 	xl_xact_commit_prepared xlrec;
  	XLogRecPtr	recptr;
  
  	START_CRIT_SECTION();
  
--- 1964,1987 ----
  								bool initfileinval)
  {
  	XLogRecData rdata[4];
  	XLogRecPtr	recptr;
+ 	int			lastrdata = 0;
+ 	xl_xact_commit_prepared *xlrec;
+ 	int xlrec_len = MinSizeOfXactCommitPrepared;
+ 	if (nrels > 0)
+ 	{
+ 		xlrec_len += sizeof(int);
+ 	}
+ 	if (nchildren > 0)
+ 	{
+ 		xlrec_len += sizeof(int);
+ 	}
+ 	if (ninvalmsgs > 0)
+ 	{
+ 		xlrec_len += sizeof(int);
+ 	}
+ 
+ 	xlrec = (xl_xact_commit_prepared *)palloc(xlrec_len);
  
  	START_CRIT_SECTION();
  
*************** RecordTransactionCommitPrepared(Transact
*** 1974,1994 ****
  	MyProc->inCommit = true;
  
  	/* Emit the XLOG commit record */
! 	xlrec.xid = xid;
! 	xlrec.crec.xact_time = GetCurrentTimestamp();
! 	xlrec.crec.xinfo = initfileinval ? XACT_COMPLETION_UPDATE_RELCACHE_FILE : 0;
! 	xlrec.crec.nmsgs = 0;
! 	xlrec.crec.nrels = nrels;
! 	xlrec.crec.nsubxacts = nchildren;
! 	xlrec.crec.nmsgs = ninvalmsgs;
! 
! 	rdata[0].data = (char *) (&xlrec);
! 	rdata[0].len = MinSizeOfXactCommitPrepared;
  	rdata[0].buffer = InvalidBuffer;
  	/* dump rels to delete */
  	if (nrels > 0)
  	{
  		rdata[0].next = &(rdata[1]);
  		rdata[1].data = (char *) rels;
  		rdata[1].len = nrels * sizeof(RelFileNode);
  		rdata[1].buffer = InvalidBuffer;
--- 1989,2006 ----
  	MyProc->inCommit = true;
  
  	/* Emit the XLOG commit record */
! 	xlrec->xid = xid;
! 	xlrec->crec.xact_time = GetCurrentTimestamp();
! 	xlrec->crec.xinfo = initfileinval ? XACT_COMPLETION_UPDATE_RELCACHE_FILE : 0;
! 	rdata[0].data = (char *) (xlrec);
! 	rdata[0].len = xlrec_len;
  	rdata[0].buffer = InvalidBuffer;
  	/* dump rels to delete */
  	if (nrels > 0)
  	{
  		rdata[0].next = &(rdata[1]);
+ 		xlrec->crec.xinfo |= XACT_COMPLETION_DROP_REL_NODES_PRESENT;
+ 		xlrec->crec.counts[0] = nrels;
  		rdata[1].data = (char *) rels;
  		rdata[1].len = nrels * sizeof(RelFileNode);
  		rdata[1].buffer = InvalidBuffer;
*************** RecordTransactionCommitPrepared(Transact
*** 1998,2012 ****
  	if (nchildren > 0)
  	{
  		rdata[lastrdata].next = &(rdata[2]);
  		rdata[2].data = (char *) children;
  		rdata[2].len = nchildren * sizeof(TransactionId);
  		rdata[2].buffer = InvalidBuffer;
  		lastrdata = 2;
  	}
! 	/* dump cache invalidation messages */
  	if (ninvalmsgs > 0)
  	{
  		rdata[lastrdata].next = &(rdata[3]);
  		rdata[3].data = (char *) invalmsgs;
  		rdata[3].len = ninvalmsgs * sizeof(SharedInvalidationMessage);
  		rdata[3].buffer = InvalidBuffer;
--- 2010,2028 ----
  	if (nchildren > 0)
  	{
  		rdata[lastrdata].next = &(rdata[2]);
+ 		xlrec->crec.xinfo |= XACT_COMPLETION_COMMITTED_SUB_PRESENT;
+ 		xlrec->crec.counts[lastrdata] = nchildren;
  		rdata[2].data = (char *) children;
  		rdata[2].len = nchildren * sizeof(TransactionId);
  		rdata[2].buffer = InvalidBuffer;
  		lastrdata = 2;
  	}
! 	/* dump shared cache invalidation messages */
  	if (ninvalmsgs > 0)
  	{
  		rdata[lastrdata].next = &(rdata[3]);
+ 		xlrec->crec.counts[lastrdata] = ninvalmsgs;
+ 		xlrec->crec.xinfo |= XACT_COMPLETION_SHARED_INV_PRESENT;
  		rdata[3].data = (char *) invalmsgs;
  		rdata[3].len = ninvalmsgs * sizeof(SharedInvalidationMessage);
  		rdata[3].buffer = InvalidBuffer;
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
new file mode 100644
index 2ca1c14..f758b41
*** a/src/backend/access/transam/xact.c
--- b/src/backend/access/transam/xact.c
*************** RecordTransactionCommit(void)
*** 964,970 ****
  		 */
  		XLogRecData rdata[4];
  		int			lastrdata = 0;
! 		xl_xact_commit xlrec;
  
  		/* Tell bufmgr and smgr to prepare for commit */
  		BufmgrCommit();
--- 964,985 ----
  		 */
  		XLogRecData rdata[4];
  		int			lastrdata = 0;
! 		xl_xact_commit *xlrec;
! 		int xlrec_len = MinSizeOfXactCommit;
! 		if (nrels > 0)
! 		{
! 			xlrec_len += sizeof(int);
! 		}
! 		if (nchildren > 0)
! 		{
! 			xlrec_len += sizeof(int);
! 		}
! 		if (nmsgs > 0)
! 		{
! 			xlrec_len += sizeof(int);
! 		}
! 
! 		xlrec = (xl_xact_commit *)palloc(xlrec_len);
  
  		/* Tell bufmgr and smgr to prepare for commit */
  		BufmgrCommit();
*************** RecordTransactionCommit(void)
*** 972,985 ****
  		/*
  		 * Set flags required for recovery processing of commits.
  		 */
! 		xlrec.xinfo = 0;
  		if (RelcacheInitFileInval)
! 			xlrec.xinfo |= XACT_COMPLETION_UPDATE_RELCACHE_FILE;
  		if (forceSyncCommit)
! 			xlrec.xinfo |= XACT_COMPLETION_FORCE_SYNC_COMMIT;
  
! 		xlrec.dbId = MyDatabaseId;
! 		xlrec.tsId = MyDatabaseTableSpace;
  
  		/*
  		 * Mark ourselves as within our "commit critical section".	This
--- 987,1000 ----
  		/*
  		 * Set flags required for recovery processing of commits.
  		 */
! 		xlrec->xinfo = 0;
  		if (RelcacheInitFileInval)
! 			xlrec->xinfo |= XACT_COMPLETION_UPDATE_RELCACHE_FILE;
  		if (forceSyncCommit)
! 			xlrec->xinfo |= XACT_COMPLETION_FORCE_SYNC_COMMIT;
  
! 		xlrec->dbId = MyDatabaseId;
! 		xlrec->tsId = MyDatabaseTableSpace;
  
  		/*
  		 * Mark ourselves as within our "commit critical section".	This
*************** RecordTransactionCommit(void)
*** 1002,1018 ****
  		MyProc->inCommit = true;
  
  		SetCurrentTransactionStopTimestamp();
! 		xlrec.xact_time = xactStopTimestamp;
! 		xlrec.nrels = nrels;
! 		xlrec.nsubxacts = nchildren;
! 		xlrec.nmsgs = nmsgs;
! 		rdata[0].data = (char *) (&xlrec);
! 		rdata[0].len = MinSizeOfXactCommit;
  		rdata[0].buffer = InvalidBuffer;
  		/* dump rels to delete */
  		if (nrels > 0)
  		{
  			rdata[0].next = &(rdata[1]);
  			rdata[1].data = (char *) rels;
  			rdata[1].len = nrels * sizeof(RelFileNode);
  			rdata[1].buffer = InvalidBuffer;
--- 1017,1032 ----
  		MyProc->inCommit = true;
  
  		SetCurrentTransactionStopTimestamp();
! 		xlrec->xact_time = xactStopTimestamp;
! 		rdata[0].data = (char *) (xlrec);
! 		rdata[0].len = xlrec_len;
  		rdata[0].buffer = InvalidBuffer;
  		/* dump rels to delete */
  		if (nrels > 0)
  		{
  			rdata[0].next = &(rdata[1]);
+ 			xlrec->xinfo |= XACT_COMPLETION_DROP_REL_NODES_PRESENT;
+ 			xlrec->counts[0] = nrels;
  			rdata[1].data = (char *) rels;
  			rdata[1].len = nrels * sizeof(RelFileNode);
  			rdata[1].buffer = InvalidBuffer;
*************** RecordTransactionCommit(void)
*** 1022,1027 ****
--- 1036,1043 ----
  		if (nchildren > 0)
  		{
  			rdata[lastrdata].next = &(rdata[2]);
+ 			xlrec->xinfo |= XACT_COMPLETION_COMMITTED_SUB_PRESENT;
+ 			xlrec->counts[lastrdata] = nchildren;
  			rdata[2].data = (char *) children;
  			rdata[2].len = nchildren * sizeof(TransactionId);
  			rdata[2].buffer = InvalidBuffer;
*************** RecordTransactionCommit(void)
*** 1031,1036 ****
--- 1047,1054 ----
  		if (nmsgs > 0)
  		{
  			rdata[lastrdata].next = &(rdata[3]);
+ 			xlrec->counts[lastrdata] = nmsgs;
+ 			xlrec->xinfo |= XACT_COMPLETION_SHARED_INV_PRESENT;
  			rdata[3].data = (char *) invalMessages;
  			rdata[3].len = nmsgs * sizeof(SharedInvalidationMessage);
  			rdata[3].buffer = InvalidBuffer;
*************** xactGetCommittedChildren(TransactionId *
*** 4437,4459 ****
   */
  
  /*
   * Before 9.0 this was a fairly short function, but now it performs many
   * actions for which the order of execution is critical.
   */
  static void
  xact_redo_commit(xl_xact_commit *xlrec, TransactionId xid, XLogRecPtr lsn)
  {
  	TransactionId *sub_xids;
  	SharedInvalidationMessage *inval_msgs;
  	TransactionId max_xid;
  	int			i;
  
! 	/* subxid array follows relfilenodes */
! 	sub_xids = (TransactionId *) &(xlrec->xnodes[xlrec->nrels]);
! 	/* invalidation messages array follows subxids */
! 	inval_msgs = (SharedInvalidationMessage *) &(sub_xids[xlrec->nsubxacts]);
! 
! 	max_xid = TransactionIdLatest(xid, xlrec->nsubxacts, sub_xids);
  
  	/*
  	 * Make sure nextXid is beyond any XID mentioned in the record.
--- 4455,4520 ----
   */
  
  /*
+  * Based on xlrec->xinfo, reads the optional arrays out of xlrec.
+  * The pointers returned by the function are valid only in case the
+  * corresponding int is not 0.
+  */
+ static void
+ xact_read_commit(xl_xact_commit *xlrec,
+ 					RelFileNode **drop_rels,
+ 					TransactionId **sub_xids,
+ 					SharedInvalidationMessage **inval_msgs,
+ 					int *nrels,
+ 					int *nsubxacts,
+ 					int *nmsgs)
+ {
+ 	int lastpos = 0;
+ 	*nrels = 0;
+ 	*nsubxacts = 0;
+ 	*nmsgs = 0;
+ 	if (XactCompletionDropRelPresent(xlrec))
+ 	{
+ 		*nrels = xlrec->counts[lastpos];
+ 		lastpos++;
+ 	}
+ 
+ 	if (XactCompletionCommittedSubPresent(xlrec))
+ 	{
+ 		*nsubxacts = xlrec->counts[lastpos];
+ 		lastpos++;
+ 	}
+ 
+ 	if (XactCompletionSharedInvPresent(xlrec))
+ 	{
+ 		*nmsgs = xlrec->counts[lastpos];
+ 		lastpos++;
+ 	}
+ 
+ 
+ 	*drop_rels = (RelFileNode *) &xlrec->counts[lastpos];
+ 	*sub_xids = (TransactionId *) &drop_rels[*nrels];
+ 	*inval_msgs = (SharedInvalidationMessage *) &sub_xids[*nsubxacts];
+ }
+ 
+ /*
   * Before 9.0 this was a fairly short function, but now it performs many
   * actions for which the order of execution is critical.
   */
  static void
  xact_redo_commit(xl_xact_commit *xlrec, TransactionId xid, XLogRecPtr lsn)
  {
+ 	RelFileNode		*drop_rels;
  	TransactionId *sub_xids;
  	SharedInvalidationMessage *inval_msgs;
  	TransactionId max_xid;
  	int			i;
+ 	int 		nrels = 0;
+ 	int 		nsubxacts = 0;
+ 	int 		nmsgs = 0;
  
! 	xact_read_commit(xlrec, &drop_rels, &sub_xids, &inval_msgs, &nrels,
! 						&nsubxacts, &nmsgs);
! 	max_xid = TransactionIdLatest(xid, nsubxacts, sub_xids);
  
  	/*
  	 * Make sure nextXid is beyond any XID mentioned in the record.
*************** xact_redo_commit(xl_xact_commit *xlrec, 
*** 4476,4482 ****
  		/*
  		 * Mark the transaction committed in pg_clog.
  		 */
! 		TransactionIdCommitTree(xid, xlrec->nsubxacts, sub_xids);
  	}
  	else
  	{
--- 4537,4543 ----
  		/*
  		 * Mark the transaction committed in pg_clog.
  		 */
! 		TransactionIdCommitTree(xid, nsubxacts, sub_xids);
  	}
  	else
  	{
*************** xact_redo_commit(xl_xact_commit *xlrec, 
*** 4500,4518 ****
  		 * bits set on changes made by transactions that haven't yet
  		 * recovered. It's unlikely but it's good to be safe.
  		 */
! 		TransactionIdAsyncCommitTree(xid, xlrec->nsubxacts, sub_xids, lsn);
  
  		/*
  		 * We must mark clog before we update the ProcArray.
  		 */
! 		ExpireTreeKnownAssignedTransactionIds(xid, xlrec->nsubxacts, sub_xids, max_xid);
  
  		/*
  		 * Send any cache invalidations attached to the commit. We must
  		 * maintain the same order of invalidation then release locks as
  		 * occurs in CommitTransaction().
  		 */
! 		ProcessCommittedInvalidationMessages(inval_msgs, xlrec->nmsgs,
  								  XactCompletionRelcacheInitFileInval(xlrec),
  											 xlrec->dbId, xlrec->tsId);
  
--- 4561,4579 ----
  		 * bits set on changes made by transactions that haven't yet
  		 * recovered. It's unlikely but it's good to be safe.
  		 */
! 		TransactionIdAsyncCommitTree(xid, nsubxacts, sub_xids, lsn);
  
  		/*
  		 * We must mark clog before we update the ProcArray.
  		 */
! 		ExpireTreeKnownAssignedTransactionIds(xid, nsubxacts, sub_xids, max_xid);
  
  		/*
  		 * Send any cache invalidations attached to the commit. We must
  		 * maintain the same order of invalidation then release locks as
  		 * occurs in CommitTransaction().
  		 */
! 		ProcessCommittedInvalidationMessages(inval_msgs, nmsgs,
  								  XactCompletionRelcacheInitFileInval(xlrec),
  											 xlrec->dbId, xlrec->tsId);
  
*************** xact_redo_commit(xl_xact_commit *xlrec, 
*** 4521,4540 ****
  		 * phase transactions. In effect we are ignoring the prepare phase and
  		 * just going straight to lock release.
  		 */
! 		StandbyReleaseLockTree(xid, xlrec->nsubxacts, sub_xids);
  	}
  
  	/* Make sure files supposed to be dropped are dropped */
! 	for (i = 0; i < xlrec->nrels; i++)
  	{
! 		SMgrRelation srel = smgropen(xlrec->xnodes[i], InvalidBackendId);
  		ForkNumber	fork;
  
  		for (fork = 0; fork <= MAX_FORKNUM; fork++)
  		{
  			if (smgrexists(srel, fork))
  			{
! 				XLogDropRelation(xlrec->xnodes[i], fork);
  				smgrdounlink(srel, fork, true);
  			}
  		}
--- 4582,4601 ----
  		 * phase transactions. In effect we are ignoring the prepare phase and
  		 * just going straight to lock release.
  		 */
! 		StandbyReleaseLockTree(xid, nsubxacts, sub_xids);
  	}
  
  	/* Make sure files supposed to be dropped are dropped */
! 	for (i = 0; i < nrels; i++)
  	{
! 		SMgrRelation srel = smgropen(drop_rels[i], InvalidBackendId);
  		ForkNumber	fork;
  
  		for (fork = 0; fork <= MAX_FORKNUM; fork++)
  		{
  			if (smgrexists(srel, fork))
  			{
! 				XLogDropRelation(drop_rels[i], fork);
  				smgrdounlink(srel, fork, true);
  			}
  		}
*************** xact_desc_commit(StringInfo buf, xl_xact
*** 4704,4745 ****
  {
  	int			i;
  	TransactionId *xacts;
  
! 	xacts = (TransactionId *) &xlrec->xnodes[xlrec->nrels];
  
  	appendStringInfoString(buf, timestamptz_to_str(xlrec->xact_time));
  
! 	if (xlrec->nrels > 0)
  	{
  		appendStringInfo(buf, "; rels:");
! 		for (i = 0; i < xlrec->nrels; i++)
  		{
! 			char	   *path = relpathperm(xlrec->xnodes[i], MAIN_FORKNUM);
  
  			appendStringInfo(buf, " %s", path);
  			pfree(path);
  		}
  	}
! 	if (xlrec->nsubxacts > 0)
  	{
  		appendStringInfo(buf, "; subxacts:");
! 		for (i = 0; i < xlrec->nsubxacts; i++)
  			appendStringInfo(buf, " %u", xacts[i]);
  	}
! 	if (xlrec->nmsgs > 0)
  	{
- 		SharedInvalidationMessage *msgs;
- 
- 		msgs = (SharedInvalidationMessage *) &xacts[xlrec->nsubxacts];
- 
  		if (XactCompletionRelcacheInitFileInval(xlrec))
  			appendStringInfo(buf, "; relcache init file inval dbid %u tsid %u",
  							 xlrec->dbId, xlrec->tsId);
  
  		appendStringInfo(buf, "; inval msgs:");
! 		for (i = 0; i < xlrec->nmsgs; i++)
  		{
! 			SharedInvalidationMessage *msg = &msgs[i];
  
  			if (msg->id >= 0)
  				appendStringInfo(buf, " catcache %d", msg->id);
--- 4765,4808 ----
  {
  	int			i;
  	TransactionId *xacts;
+ 	RelFileNode		*drop_rels;
+ 	SharedInvalidationMessage *inval_msgs;
+ 	int 		nrels = 0;
+ 	int 		nsubxacts = 0;
+ 	int 		nmsgs = 0;
  
! 	xact_read_commit(xlrec, &drop_rels, &xacts, &inval_msgs, &nrels,
! 						&nsubxacts, &nmsgs);
  
  	appendStringInfoString(buf, timestamptz_to_str(xlrec->xact_time));
  
! 	if (nrels > 0)
  	{
  		appendStringInfo(buf, "; rels:");
! 		for (i = 0; i < nrels; i++)
  		{
! 			char	   *path = relpathperm(drop_rels[i], MAIN_FORKNUM);
  
  			appendStringInfo(buf, " %s", path);
  			pfree(path);
  		}
  	}
! 	if (nsubxacts > 0)
  	{
  		appendStringInfo(buf, "; subxacts:");
! 		for (i = 0; i < nsubxacts; i++)
  			appendStringInfo(buf, " %u", xacts[i]);
  	}
! 	if (nmsgs > 0)
  	{
  		if (XactCompletionRelcacheInitFileInval(xlrec))
  			appendStringInfo(buf, "; relcache init file inval dbid %u tsid %u",
  							 xlrec->dbId, xlrec->tsId);
  
  		appendStringInfo(buf, "; inval msgs:");
! 		for (i = 0; i < nmsgs; i++)
  		{
! 			SharedInvalidationMessage *msg = &inval_msgs[i];
  
  			if (msg->id >= 0)
  				appendStringInfo(buf, " catcache %d", msg->id);
diff --git a/src/include/access/xact.h b/src/include/access/xact.h
new file mode 100644
index cb440d4..402437f
*** a/src/include/access/xact.h
--- b/src/include/access/xact.h
*************** typedef struct xl_xact_assignment
*** 116,137 ****
  
  #define MinSizeOfXactAssignment offsetof(xl_xact_assignment, xsub)
  
  typedef struct xl_xact_commit
  {
  	TimestampTz xact_time;		/* time of commit */
  	uint32		xinfo;			/* info flags */
- 	int			nrels;			/* number of RelFileNodes */
- 	int			nsubxacts;		/* number of subtransaction XIDs */
- 	int			nmsgs;			/* number of shared inval msgs */
  	Oid			dbId;			/* MyDatabaseId */
  	Oid			tsId;			/* MyDatabaseTableSpace */
! 	/* Array of RelFileNode(s) to drop at commit */
! 	RelFileNode xnodes[1];		/* VARIABLE LENGTH ARRAY */
! 	/* ARRAY OF COMMITTED SUBTRANSACTION XIDs FOLLOWS */
! 	/* ARRAY OF SHARED INVALIDATION MESSAGES FOLLOWS */
  } xl_xact_commit;
  
! #define MinSizeOfXactCommit offsetof(xl_xact_commit, xnodes)
  
  /*
   * These flags are set in the xinfo fields of WAL commit records,
--- 116,146 ----
  
  #define MinSizeOfXactAssignment offsetof(xl_xact_assignment, xsub)
  
+ /*
+  * As of 9.2, the record format changed to reduce space: instead of having
+  * ints that represent the length of the optional arrays at the end of
+  * xl_xact_commit, some bits in xinfo are used to flag which optional arrays
+  * are, in fact, present. For each present array, an int that represents the
+  * array length is placed at the end of the struct. The present arrays are
+  * stored after those ints.
+  */
  typedef struct xl_xact_commit
  {
  	TimestampTz xact_time;		/* time of commit */
  	uint32		xinfo;			/* info flags */
  	Oid			dbId;			/* MyDatabaseId */
  	Oid			tsId;			/* MyDatabaseTableSpace */
! 	int  		counts[1];  	/* variable-length array of counts, xinfo flags */
! 								/* define length of array and meaning of counts */
! 	/* OPTIONAL RELATIONS TO DROP LENGTH FOLLOW */
! 	/* OPTIONAL COMMITTED SUBTRANSACTION XIDs LENGTH FOLLOW */
! 	/* OPTIONAL SHARED INVALIDATION MESSAGES LENGTH FOLLOW */
! 	/* ARRAY OF OPTIONAL RELATIONS TO DROP FOLLOWS */
! 	/* ARRAY OF OPTIONAL COMMITTED SUBTRANSACTION XIDs FOLLOWS */
! 	/* ARRAY OF OPTIONAL SHARED INVALIDATION MESSAGES FOLLOWS */
  } xl_xact_commit;
  
! #define MinSizeOfXactCommit offsetof(xl_xact_commit, counts)
  
  /*
   * These flags are set in the xinfo fields of WAL commit records,
*************** typedef struct xl_xact_commit
*** 148,153 ****
--- 157,176 ----
  #define XactCompletionRelcacheInitFileInval(xlrec)	((xlrec)->xinfo & XACT_COMPLETION_UPDATE_RELCACHE_FILE)
  #define XactCompletionForceSyncCommit(xlrec)		((xlrec)->xinfo & XACT_COMPLETION_FORCE_SYNC_COMMIT)
  
+ /*
+  * These flags are set in the xinfo fields of WAL commit records,
+  * indicating the presence of optionals xl_xact_commit_opt structures
+  * (RelFileNodes to drop at commit, subtransaction XIDs, shared inval msgs)
+  */
+ #define XACT_COMPLETION_DROP_REL_NODES_PRESENT	0x04
+ #define XACT_COMPLETION_COMMITTED_SUB_PRESENT	0x08
+ #define XACT_COMPLETION_SHARED_INV_PRESENT		0x10
+ 
+ /* Access macros for above flags */
+ #define XactCompletionDropRelPresent(xlrec)	((xlrec)->xinfo & XACT_COMPLETION_DROP_REL_NODES_PRESENT)
+ #define XactCompletionCommittedSubPresent(xlrec)		((xlrec)->xinfo & XACT_COMPLETION_COMMITTED_SUB_PRESENT)
+ #define XactCompletionSharedInvPresent(xlrec)		((xlrec)->xinfo & XACT_COMPLETION_SHARED_INV_PRESENT)
+ 
  typedef struct xl_xact_abort
  {
  	TimestampTz xact_time;		/* time of abort */
*************** typedef struct xl_xact_commit_prepared
*** 176,182 ****
  	/* MORE DATA FOLLOWS AT END OF STRUCT */
  } xl_xact_commit_prepared;
  
! #define MinSizeOfXactCommitPrepared offsetof(xl_xact_commit_prepared, crec.xnodes)
  
  typedef struct xl_xact_abort_prepared
  {
--- 199,205 ----
  	/* MORE DATA FOLLOWS AT END OF STRUCT */
  } xl_xact_commit_prepared;
  
! #define MinSizeOfXactCommitPrepared offsetof(xl_xact_commit_prepared, crec.counts)
  
  typedef struct xl_xact_abort_prepared
  {
#8Leonardo Francalanci
m_lists@yahoo.it
In reply to: Leonardo Francalanci (#7)
1 attachment(s)
Re: use less space in xl_xact_commit patch

Attachments:

commitlog_lessbytes02.patchapplication/octet-stream; name=commitlog_lessbytes02.patchDownload
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
new file mode 100644
index 2812681..f2b88a5
*** a/src/backend/access/transam/twophase.c
--- b/src/backend/access/transam/twophase.c
*************** RecordTransactionCommitPrepared(Transact
*** 1964,1972 ****
  								bool initfileinval)
  {
  	XLogRecData rdata[4];
- 	int			lastrdata = 0;
- 	xl_xact_commit_prepared xlrec;
  	XLogRecPtr	recptr;
  
  	START_CRIT_SECTION();
  
--- 1964,1987 ----
  								bool initfileinval)
  {
  	XLogRecData rdata[4];
  	XLogRecPtr	recptr;
+ 	int			lastrdata = 0;
+ 	xl_xact_commit_prepared *xlrec;
+ 	int xlrec_len = MinSizeOfXactCommitPrepared;
+ 	if (nrels > 0)
+ 	{
+ 		xlrec_len += sizeof(int);
+ 	}
+ 	if (nchildren > 0)
+ 	{
+ 		xlrec_len += sizeof(int);
+ 	}
+ 	if (ninvalmsgs > 0)
+ 	{
+ 		xlrec_len += sizeof(int);
+ 	}
+ 
+ 	xlrec = (xl_xact_commit_prepared *)palloc(xlrec_len);
  
  	START_CRIT_SECTION();
  
*************** RecordTransactionCommitPrepared(Transact
*** 1974,1994 ****
  	MyProc->inCommit = true;
  
  	/* Emit the XLOG commit record */
! 	xlrec.xid = xid;
! 	xlrec.crec.xact_time = GetCurrentTimestamp();
! 	xlrec.crec.xinfo = initfileinval ? XACT_COMPLETION_UPDATE_RELCACHE_FILE : 0;
! 	xlrec.crec.nmsgs = 0;
! 	xlrec.crec.nrels = nrels;
! 	xlrec.crec.nsubxacts = nchildren;
! 	xlrec.crec.nmsgs = ninvalmsgs;
! 
! 	rdata[0].data = (char *) (&xlrec);
! 	rdata[0].len = MinSizeOfXactCommitPrepared;
  	rdata[0].buffer = InvalidBuffer;
  	/* dump rels to delete */
  	if (nrels > 0)
  	{
  		rdata[0].next = &(rdata[1]);
  		rdata[1].data = (char *) rels;
  		rdata[1].len = nrels * sizeof(RelFileNode);
  		rdata[1].buffer = InvalidBuffer;
--- 1989,2006 ----
  	MyProc->inCommit = true;
  
  	/* Emit the XLOG commit record */
! 	xlrec->xid = xid;
! 	xlrec->crec.xact_time = GetCurrentTimestamp();
! 	xlrec->crec.xinfo = initfileinval ? XACT_COMPLETION_UPDATE_RELCACHE_FILE : 0;
! 	rdata[0].data = (char *) (xlrec);
! 	rdata[0].len = xlrec_len;
  	rdata[0].buffer = InvalidBuffer;
  	/* dump rels to delete */
  	if (nrels > 0)
  	{
  		rdata[0].next = &(rdata[1]);
+ 		xlrec->crec.xinfo |= XACT_COMPLETION_DROP_REL_NODES_PRESENT;
+ 		xlrec->crec.counts[0] = nrels;
  		rdata[1].data = (char *) rels;
  		rdata[1].len = nrels * sizeof(RelFileNode);
  		rdata[1].buffer = InvalidBuffer;
*************** RecordTransactionCommitPrepared(Transact
*** 1998,2012 ****
  	if (nchildren > 0)
  	{
  		rdata[lastrdata].next = &(rdata[2]);
  		rdata[2].data = (char *) children;
  		rdata[2].len = nchildren * sizeof(TransactionId);
  		rdata[2].buffer = InvalidBuffer;
  		lastrdata = 2;
  	}
! 	/* dump cache invalidation messages */
  	if (ninvalmsgs > 0)
  	{
  		rdata[lastrdata].next = &(rdata[3]);
  		rdata[3].data = (char *) invalmsgs;
  		rdata[3].len = ninvalmsgs * sizeof(SharedInvalidationMessage);
  		rdata[3].buffer = InvalidBuffer;
--- 2010,2028 ----
  	if (nchildren > 0)
  	{
  		rdata[lastrdata].next = &(rdata[2]);
+ 		xlrec->crec.xinfo |= XACT_COMPLETION_COMMITTED_SUB_PRESENT;
+ 		xlrec->crec.counts[lastrdata] = nchildren;
  		rdata[2].data = (char *) children;
  		rdata[2].len = nchildren * sizeof(TransactionId);
  		rdata[2].buffer = InvalidBuffer;
  		lastrdata = 2;
  	}
! 	/* dump shared cache invalidation messages */
  	if (ninvalmsgs > 0)
  	{
  		rdata[lastrdata].next = &(rdata[3]);
+ 		xlrec->crec.counts[lastrdata] = ninvalmsgs;
+ 		xlrec->crec.xinfo |= XACT_COMPLETION_SHARED_INV_PRESENT;
  		rdata[3].data = (char *) invalmsgs;
  		rdata[3].len = ninvalmsgs * sizeof(SharedInvalidationMessage);
  		rdata[3].buffer = InvalidBuffer;
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
new file mode 100644
index 2ca1c14..f758b41
*** a/src/backend/access/transam/xact.c
--- b/src/backend/access/transam/xact.c
*************** RecordTransactionCommit(void)
*** 964,970 ****
  		 */
  		XLogRecData rdata[4];
  		int			lastrdata = 0;
! 		xl_xact_commit xlrec;
  
  		/* Tell bufmgr and smgr to prepare for commit */
  		BufmgrCommit();
--- 964,985 ----
  		 */
  		XLogRecData rdata[4];
  		int			lastrdata = 0;
! 		xl_xact_commit *xlrec;
! 		int xlrec_len = MinSizeOfXactCommit;
! 		if (nrels > 0)
! 		{
! 			xlrec_len += sizeof(int);
! 		}
! 		if (nchildren > 0)
! 		{
! 			xlrec_len += sizeof(int);
! 		}
! 		if (nmsgs > 0)
! 		{
! 			xlrec_len += sizeof(int);
! 		}
! 
! 		xlrec = (xl_xact_commit *)palloc(xlrec_len);
  
  		/* Tell bufmgr and smgr to prepare for commit */
  		BufmgrCommit();
*************** RecordTransactionCommit(void)
*** 972,985 ****
  		/*
  		 * Set flags required for recovery processing of commits.
  		 */
! 		xlrec.xinfo = 0;
  		if (RelcacheInitFileInval)
! 			xlrec.xinfo |= XACT_COMPLETION_UPDATE_RELCACHE_FILE;
  		if (forceSyncCommit)
! 			xlrec.xinfo |= XACT_COMPLETION_FORCE_SYNC_COMMIT;
  
! 		xlrec.dbId = MyDatabaseId;
! 		xlrec.tsId = MyDatabaseTableSpace;
  
  		/*
  		 * Mark ourselves as within our "commit critical section".	This
--- 987,1000 ----
  		/*
  		 * Set flags required for recovery processing of commits.
  		 */
! 		xlrec->xinfo = 0;
  		if (RelcacheInitFileInval)
! 			xlrec->xinfo |= XACT_COMPLETION_UPDATE_RELCACHE_FILE;
  		if (forceSyncCommit)
! 			xlrec->xinfo |= XACT_COMPLETION_FORCE_SYNC_COMMIT;
  
! 		xlrec->dbId = MyDatabaseId;
! 		xlrec->tsId = MyDatabaseTableSpace;
  
  		/*
  		 * Mark ourselves as within our "commit critical section".	This
*************** RecordTransactionCommit(void)
*** 1002,1018 ****
  		MyProc->inCommit = true;
  
  		SetCurrentTransactionStopTimestamp();
! 		xlrec.xact_time = xactStopTimestamp;
! 		xlrec.nrels = nrels;
! 		xlrec.nsubxacts = nchildren;
! 		xlrec.nmsgs = nmsgs;
! 		rdata[0].data = (char *) (&xlrec);
! 		rdata[0].len = MinSizeOfXactCommit;
  		rdata[0].buffer = InvalidBuffer;
  		/* dump rels to delete */
  		if (nrels > 0)
  		{
  			rdata[0].next = &(rdata[1]);
  			rdata[1].data = (char *) rels;
  			rdata[1].len = nrels * sizeof(RelFileNode);
  			rdata[1].buffer = InvalidBuffer;
--- 1017,1032 ----
  		MyProc->inCommit = true;
  
  		SetCurrentTransactionStopTimestamp();
! 		xlrec->xact_time = xactStopTimestamp;
! 		rdata[0].data = (char *) (xlrec);
! 		rdata[0].len = xlrec_len;
  		rdata[0].buffer = InvalidBuffer;
  		/* dump rels to delete */
  		if (nrels > 0)
  		{
  			rdata[0].next = &(rdata[1]);
+ 			xlrec->xinfo |= XACT_COMPLETION_DROP_REL_NODES_PRESENT;
+ 			xlrec->counts[0] = nrels;
  			rdata[1].data = (char *) rels;
  			rdata[1].len = nrels * sizeof(RelFileNode);
  			rdata[1].buffer = InvalidBuffer;
*************** RecordTransactionCommit(void)
*** 1022,1027 ****
--- 1036,1043 ----
  		if (nchildren > 0)
  		{
  			rdata[lastrdata].next = &(rdata[2]);
+ 			xlrec->xinfo |= XACT_COMPLETION_COMMITTED_SUB_PRESENT;
+ 			xlrec->counts[lastrdata] = nchildren;
  			rdata[2].data = (char *) children;
  			rdata[2].len = nchildren * sizeof(TransactionId);
  			rdata[2].buffer = InvalidBuffer;
*************** RecordTransactionCommit(void)
*** 1031,1036 ****
--- 1047,1054 ----
  		if (nmsgs > 0)
  		{
  			rdata[lastrdata].next = &(rdata[3]);
+ 			xlrec->counts[lastrdata] = nmsgs;
+ 			xlrec->xinfo |= XACT_COMPLETION_SHARED_INV_PRESENT;
  			rdata[3].data = (char *) invalMessages;
  			rdata[3].len = nmsgs * sizeof(SharedInvalidationMessage);
  			rdata[3].buffer = InvalidBuffer;
*************** xactGetCommittedChildren(TransactionId *
*** 4437,4459 ****
   */
  
  /*
   * Before 9.0 this was a fairly short function, but now it performs many
   * actions for which the order of execution is critical.
   */
  static void
  xact_redo_commit(xl_xact_commit *xlrec, TransactionId xid, XLogRecPtr lsn)
  {
  	TransactionId *sub_xids;
  	SharedInvalidationMessage *inval_msgs;
  	TransactionId max_xid;
  	int			i;
  
! 	/* subxid array follows relfilenodes */
! 	sub_xids = (TransactionId *) &(xlrec->xnodes[xlrec->nrels]);
! 	/* invalidation messages array follows subxids */
! 	inval_msgs = (SharedInvalidationMessage *) &(sub_xids[xlrec->nsubxacts]);
! 
! 	max_xid = TransactionIdLatest(xid, xlrec->nsubxacts, sub_xids);
  
  	/*
  	 * Make sure nextXid is beyond any XID mentioned in the record.
--- 4455,4520 ----
   */
  
  /*
+  * Based on xlrec->xinfo, reads the optional arrays out of xlrec.
+  * The pointers returned by the function are valid only in case the
+  * corresponding int is not 0.
+  */
+ static void
+ xact_read_commit(xl_xact_commit *xlrec,
+ 					RelFileNode **drop_rels,
+ 					TransactionId **sub_xids,
+ 					SharedInvalidationMessage **inval_msgs,
+ 					int *nrels,
+ 					int *nsubxacts,
+ 					int *nmsgs)
+ {
+ 	int lastpos = 0;
+ 	*nrels = 0;
+ 	*nsubxacts = 0;
+ 	*nmsgs = 0;
+ 	if (XactCompletionDropRelPresent(xlrec))
+ 	{
+ 		*nrels = xlrec->counts[lastpos];
+ 		lastpos++;
+ 	}
+ 
+ 	if (XactCompletionCommittedSubPresent(xlrec))
+ 	{
+ 		*nsubxacts = xlrec->counts[lastpos];
+ 		lastpos++;
+ 	}
+ 
+ 	if (XactCompletionSharedInvPresent(xlrec))
+ 	{
+ 		*nmsgs = xlrec->counts[lastpos];
+ 		lastpos++;
+ 	}
+ 
+ 
+ 	*drop_rels = (RelFileNode *) &xlrec->counts[lastpos];
+ 	*sub_xids = (TransactionId *) &drop_rels[*nrels];
+ 	*inval_msgs = (SharedInvalidationMessage *) &sub_xids[*nsubxacts];
+ }
+ 
+ /*
   * Before 9.0 this was a fairly short function, but now it performs many
   * actions for which the order of execution is critical.
   */
  static void
  xact_redo_commit(xl_xact_commit *xlrec, TransactionId xid, XLogRecPtr lsn)
  {
+ 	RelFileNode		*drop_rels;
  	TransactionId *sub_xids;
  	SharedInvalidationMessage *inval_msgs;
  	TransactionId max_xid;
  	int			i;
+ 	int 		nrels = 0;
+ 	int 		nsubxacts = 0;
+ 	int 		nmsgs = 0;
  
! 	xact_read_commit(xlrec, &drop_rels, &sub_xids, &inval_msgs, &nrels,
! 						&nsubxacts, &nmsgs);
! 	max_xid = TransactionIdLatest(xid, nsubxacts, sub_xids);
  
  	/*
  	 * Make sure nextXid is beyond any XID mentioned in the record.
*************** xact_redo_commit(xl_xact_commit *xlrec, 
*** 4476,4482 ****
  		/*
  		 * Mark the transaction committed in pg_clog.
  		 */
! 		TransactionIdCommitTree(xid, xlrec->nsubxacts, sub_xids);
  	}
  	else
  	{
--- 4537,4543 ----
  		/*
  		 * Mark the transaction committed in pg_clog.
  		 */
! 		TransactionIdCommitTree(xid, nsubxacts, sub_xids);
  	}
  	else
  	{
*************** xact_redo_commit(xl_xact_commit *xlrec, 
*** 4500,4518 ****
  		 * bits set on changes made by transactions that haven't yet
  		 * recovered. It's unlikely but it's good to be safe.
  		 */
! 		TransactionIdAsyncCommitTree(xid, xlrec->nsubxacts, sub_xids, lsn);
  
  		/*
  		 * We must mark clog before we update the ProcArray.
  		 */
! 		ExpireTreeKnownAssignedTransactionIds(xid, xlrec->nsubxacts, sub_xids, max_xid);
  
  		/*
  		 * Send any cache invalidations attached to the commit. We must
  		 * maintain the same order of invalidation then release locks as
  		 * occurs in CommitTransaction().
  		 */
! 		ProcessCommittedInvalidationMessages(inval_msgs, xlrec->nmsgs,
  								  XactCompletionRelcacheInitFileInval(xlrec),
  											 xlrec->dbId, xlrec->tsId);
  
--- 4561,4579 ----
  		 * bits set on changes made by transactions that haven't yet
  		 * recovered. It's unlikely but it's good to be safe.
  		 */
! 		TransactionIdAsyncCommitTree(xid, nsubxacts, sub_xids, lsn);
  
  		/*
  		 * We must mark clog before we update the ProcArray.
  		 */
! 		ExpireTreeKnownAssignedTransactionIds(xid, nsubxacts, sub_xids, max_xid);
  
  		/*
  		 * Send any cache invalidations attached to the commit. We must
  		 * maintain the same order of invalidation then release locks as
  		 * occurs in CommitTransaction().
  		 */
! 		ProcessCommittedInvalidationMessages(inval_msgs, nmsgs,
  								  XactCompletionRelcacheInitFileInval(xlrec),
  											 xlrec->dbId, xlrec->tsId);
  
*************** xact_redo_commit(xl_xact_commit *xlrec, 
*** 4521,4540 ****
  		 * phase transactions. In effect we are ignoring the prepare phase and
  		 * just going straight to lock release.
  		 */
! 		StandbyReleaseLockTree(xid, xlrec->nsubxacts, sub_xids);
  	}
  
  	/* Make sure files supposed to be dropped are dropped */
! 	for (i = 0; i < xlrec->nrels; i++)
  	{
! 		SMgrRelation srel = smgropen(xlrec->xnodes[i], InvalidBackendId);
  		ForkNumber	fork;
  
  		for (fork = 0; fork <= MAX_FORKNUM; fork++)
  		{
  			if (smgrexists(srel, fork))
  			{
! 				XLogDropRelation(xlrec->xnodes[i], fork);
  				smgrdounlink(srel, fork, true);
  			}
  		}
--- 4582,4601 ----
  		 * phase transactions. In effect we are ignoring the prepare phase and
  		 * just going straight to lock release.
  		 */
! 		StandbyReleaseLockTree(xid, nsubxacts, sub_xids);
  	}
  
  	/* Make sure files supposed to be dropped are dropped */
! 	for (i = 0; i < nrels; i++)
  	{
! 		SMgrRelation srel = smgropen(drop_rels[i], InvalidBackendId);
  		ForkNumber	fork;
  
  		for (fork = 0; fork <= MAX_FORKNUM; fork++)
  		{
  			if (smgrexists(srel, fork))
  			{
! 				XLogDropRelation(drop_rels[i], fork);
  				smgrdounlink(srel, fork, true);
  			}
  		}
*************** xact_desc_commit(StringInfo buf, xl_xact
*** 4704,4745 ****
  {
  	int			i;
  	TransactionId *xacts;
  
! 	xacts = (TransactionId *) &xlrec->xnodes[xlrec->nrels];
  
  	appendStringInfoString(buf, timestamptz_to_str(xlrec->xact_time));
  
! 	if (xlrec->nrels > 0)
  	{
  		appendStringInfo(buf, "; rels:");
! 		for (i = 0; i < xlrec->nrels; i++)
  		{
! 			char	   *path = relpathperm(xlrec->xnodes[i], MAIN_FORKNUM);
  
  			appendStringInfo(buf, " %s", path);
  			pfree(path);
  		}
  	}
! 	if (xlrec->nsubxacts > 0)
  	{
  		appendStringInfo(buf, "; subxacts:");
! 		for (i = 0; i < xlrec->nsubxacts; i++)
  			appendStringInfo(buf, " %u", xacts[i]);
  	}
! 	if (xlrec->nmsgs > 0)
  	{
- 		SharedInvalidationMessage *msgs;
- 
- 		msgs = (SharedInvalidationMessage *) &xacts[xlrec->nsubxacts];
- 
  		if (XactCompletionRelcacheInitFileInval(xlrec))
  			appendStringInfo(buf, "; relcache init file inval dbid %u tsid %u",
  							 xlrec->dbId, xlrec->tsId);
  
  		appendStringInfo(buf, "; inval msgs:");
! 		for (i = 0; i < xlrec->nmsgs; i++)
  		{
! 			SharedInvalidationMessage *msg = &msgs[i];
  
  			if (msg->id >= 0)
  				appendStringInfo(buf, " catcache %d", msg->id);
--- 4765,4808 ----
  {
  	int			i;
  	TransactionId *xacts;
+ 	RelFileNode		*drop_rels;
+ 	SharedInvalidationMessage *inval_msgs;
+ 	int 		nrels = 0;
+ 	int 		nsubxacts = 0;
+ 	int 		nmsgs = 0;
  
! 	xact_read_commit(xlrec, &drop_rels, &xacts, &inval_msgs, &nrels,
! 						&nsubxacts, &nmsgs);
  
  	appendStringInfoString(buf, timestamptz_to_str(xlrec->xact_time));
  
! 	if (nrels > 0)
  	{
  		appendStringInfo(buf, "; rels:");
! 		for (i = 0; i < nrels; i++)
  		{
! 			char	   *path = relpathperm(drop_rels[i], MAIN_FORKNUM);
  
  			appendStringInfo(buf, " %s", path);
  			pfree(path);
  		}
  	}
! 	if (nsubxacts > 0)
  	{
  		appendStringInfo(buf, "; subxacts:");
! 		for (i = 0; i < nsubxacts; i++)
  			appendStringInfo(buf, " %u", xacts[i]);
  	}
! 	if (nmsgs > 0)
  	{
  		if (XactCompletionRelcacheInitFileInval(xlrec))
  			appendStringInfo(buf, "; relcache init file inval dbid %u tsid %u",
  							 xlrec->dbId, xlrec->tsId);
  
  		appendStringInfo(buf, "; inval msgs:");
! 		for (i = 0; i < nmsgs; i++)
  		{
! 			SharedInvalidationMessage *msg = &inval_msgs[i];
  
  			if (msg->id >= 0)
  				appendStringInfo(buf, " catcache %d", msg->id);
diff --git a/src/include/access/xact.h b/src/include/access/xact.h
new file mode 100644
index cb440d4..402437f
*** a/src/include/access/xact.h
--- b/src/include/access/xact.h
*************** typedef struct xl_xact_assignment
*** 116,137 ****
  
  #define MinSizeOfXactAssignment offsetof(xl_xact_assignment, xsub)
  
  typedef struct xl_xact_commit
  {
  	TimestampTz xact_time;		/* time of commit */
  	uint32		xinfo;			/* info flags */
- 	int			nrels;			/* number of RelFileNodes */
- 	int			nsubxacts;		/* number of subtransaction XIDs */
- 	int			nmsgs;			/* number of shared inval msgs */
  	Oid			dbId;			/* MyDatabaseId */
  	Oid			tsId;			/* MyDatabaseTableSpace */
! 	/* Array of RelFileNode(s) to drop at commit */
! 	RelFileNode xnodes[1];		/* VARIABLE LENGTH ARRAY */
! 	/* ARRAY OF COMMITTED SUBTRANSACTION XIDs FOLLOWS */
! 	/* ARRAY OF SHARED INVALIDATION MESSAGES FOLLOWS */
  } xl_xact_commit;
  
! #define MinSizeOfXactCommit offsetof(xl_xact_commit, xnodes)
  
  /*
   * These flags are set in the xinfo fields of WAL commit records,
--- 116,146 ----
  
  #define MinSizeOfXactAssignment offsetof(xl_xact_assignment, xsub)
  
+ /*
+  * As of 9.2, the record format changed to reduce space: instead of having
+  * ints that represent the length of the optional arrays at the end of
+  * xl_xact_commit, some bits in xinfo are used to flag which optional arrays
+  * are, in fact, present. For each present array, an int that represents the
+  * array length is placed at the end of the struct. The present arrays are
+  * stored after those ints.
+  */
  typedef struct xl_xact_commit
  {
  	TimestampTz xact_time;		/* time of commit */
  	uint32		xinfo;			/* info flags */
  	Oid			dbId;			/* MyDatabaseId */
  	Oid			tsId;			/* MyDatabaseTableSpace */
! 	int  		counts[1];  	/* variable-length array of counts, xinfo flags */
! 								/* define length of array and meaning of counts */
! 	/* OPTIONAL RELATIONS TO DROP LENGTH FOLLOWS */
! 	/* OPTIONAL COMMITTED SUBTRANSACTION XIDs LENGTH FOLLOWS */
! 	/* OPTIONAL SHARED INVALIDATION MESSAGES LENGTH FOLLOWS */
! 	/* ARRAY OF OPTIONAL RELATIONS TO DROP FOLLOWS */
! 	/* ARRAY OF OPTIONAL COMMITTED SUBTRANSACTION XIDs FOLLOWS */
! 	/* ARRAY OF OPTIONAL SHARED INVALIDATION MESSAGES FOLLOWS */
  } xl_xact_commit;
  
! #define MinSizeOfXactCommit offsetof(xl_xact_commit, counts)
  
  /*
   * These flags are set in the xinfo fields of WAL commit records,
*************** typedef struct xl_xact_commit
*** 148,153 ****
--- 157,176 ----
  #define XactCompletionRelcacheInitFileInval(xlrec)	((xlrec)->xinfo & XACT_COMPLETION_UPDATE_RELCACHE_FILE)
  #define XactCompletionForceSyncCommit(xlrec)		((xlrec)->xinfo & XACT_COMPLETION_FORCE_SYNC_COMMIT)
  
+ /*
+  * These flags are set in the xinfo fields of WAL commit records,
+  * indicating the presence of optionals xl_xact_commit_opt structures
+  * (RelFileNodes to drop at commit, subtransaction XIDs, shared inval msgs)
+  */
+ #define XACT_COMPLETION_DROP_REL_NODES_PRESENT	0x04
+ #define XACT_COMPLETION_COMMITTED_SUB_PRESENT	0x08
+ #define XACT_COMPLETION_SHARED_INV_PRESENT		0x10
+ 
+ /* Access macros for above flags */
+ #define XactCompletionDropRelPresent(xlrec)	((xlrec)->xinfo & XACT_COMPLETION_DROP_REL_NODES_PRESENT)
+ #define XactCompletionCommittedSubPresent(xlrec)		((xlrec)->xinfo & XACT_COMPLETION_COMMITTED_SUB_PRESENT)
+ #define XactCompletionSharedInvPresent(xlrec)		((xlrec)->xinfo & XACT_COMPLETION_SHARED_INV_PRESENT)
+ 
  typedef struct xl_xact_abort
  {
  	TimestampTz xact_time;		/* time of abort */
*************** typedef struct xl_xact_commit_prepared
*** 176,182 ****
  	/* MORE DATA FOLLOWS AT END OF STRUCT */
  } xl_xact_commit_prepared;
  
! #define MinSizeOfXactCommitPrepared offsetof(xl_xact_commit_prepared, crec.xnodes)
  
  typedef struct xl_xact_abort_prepared
  {
--- 199,205 ----
  	/* MORE DATA FOLLOWS AT END OF STRUCT */
  } xl_xact_commit_prepared;
  
! #define MinSizeOfXactCommitPrepared offsetof(xl_xact_commit_prepared, crec.counts)
  
  typedef struct xl_xact_abort_prepared
  {
#9Leonardo Francalanci
m_lists@yahoo.it
In reply to: Leonardo Francalanci (#8)
1 attachment(s)
Re: use less space in xl_xact_commit patch

Sorry, email sent without body.

Fixed some English mistakes.

Attachments:

commitlog_lessbytes02.patchapplication/octet-stream; name=commitlog_lessbytes02.patchDownload
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
new file mode 100644
index 2812681..f2b88a5
*** a/src/backend/access/transam/twophase.c
--- b/src/backend/access/transam/twophase.c
*************** RecordTransactionCommitPrepared(Transact
*** 1964,1972 ****
  								bool initfileinval)
  {
  	XLogRecData rdata[4];
- 	int			lastrdata = 0;
- 	xl_xact_commit_prepared xlrec;
  	XLogRecPtr	recptr;
  
  	START_CRIT_SECTION();
  
--- 1964,1987 ----
  								bool initfileinval)
  {
  	XLogRecData rdata[4];
  	XLogRecPtr	recptr;
+ 	int			lastrdata = 0;
+ 	xl_xact_commit_prepared *xlrec;
+ 	int xlrec_len = MinSizeOfXactCommitPrepared;
+ 	if (nrels > 0)
+ 	{
+ 		xlrec_len += sizeof(int);
+ 	}
+ 	if (nchildren > 0)
+ 	{
+ 		xlrec_len += sizeof(int);
+ 	}
+ 	if (ninvalmsgs > 0)
+ 	{
+ 		xlrec_len += sizeof(int);
+ 	}
+ 
+ 	xlrec = (xl_xact_commit_prepared *)palloc(xlrec_len);
  
  	START_CRIT_SECTION();
  
*************** RecordTransactionCommitPrepared(Transact
*** 1974,1994 ****
  	MyProc->inCommit = true;
  
  	/* Emit the XLOG commit record */
! 	xlrec.xid = xid;
! 	xlrec.crec.xact_time = GetCurrentTimestamp();
! 	xlrec.crec.xinfo = initfileinval ? XACT_COMPLETION_UPDATE_RELCACHE_FILE : 0;
! 	xlrec.crec.nmsgs = 0;
! 	xlrec.crec.nrels = nrels;
! 	xlrec.crec.nsubxacts = nchildren;
! 	xlrec.crec.nmsgs = ninvalmsgs;
! 
! 	rdata[0].data = (char *) (&xlrec);
! 	rdata[0].len = MinSizeOfXactCommitPrepared;
  	rdata[0].buffer = InvalidBuffer;
  	/* dump rels to delete */
  	if (nrels > 0)
  	{
  		rdata[0].next = &(rdata[1]);
  		rdata[1].data = (char *) rels;
  		rdata[1].len = nrels * sizeof(RelFileNode);
  		rdata[1].buffer = InvalidBuffer;
--- 1989,2006 ----
  	MyProc->inCommit = true;
  
  	/* Emit the XLOG commit record */
! 	xlrec->xid = xid;
! 	xlrec->crec.xact_time = GetCurrentTimestamp();
! 	xlrec->crec.xinfo = initfileinval ? XACT_COMPLETION_UPDATE_RELCACHE_FILE : 0;
! 	rdata[0].data = (char *) (xlrec);
! 	rdata[0].len = xlrec_len;
  	rdata[0].buffer = InvalidBuffer;
  	/* dump rels to delete */
  	if (nrels > 0)
  	{
  		rdata[0].next = &(rdata[1]);
+ 		xlrec->crec.xinfo |= XACT_COMPLETION_DROP_REL_NODES_PRESENT;
+ 		xlrec->crec.counts[0] = nrels;
  		rdata[1].data = (char *) rels;
  		rdata[1].len = nrels * sizeof(RelFileNode);
  		rdata[1].buffer = InvalidBuffer;
*************** RecordTransactionCommitPrepared(Transact
*** 1998,2012 ****
  	if (nchildren > 0)
  	{
  		rdata[lastrdata].next = &(rdata[2]);
  		rdata[2].data = (char *) children;
  		rdata[2].len = nchildren * sizeof(TransactionId);
  		rdata[2].buffer = InvalidBuffer;
  		lastrdata = 2;
  	}
! 	/* dump cache invalidation messages */
  	if (ninvalmsgs > 0)
  	{
  		rdata[lastrdata].next = &(rdata[3]);
  		rdata[3].data = (char *) invalmsgs;
  		rdata[3].len = ninvalmsgs * sizeof(SharedInvalidationMessage);
  		rdata[3].buffer = InvalidBuffer;
--- 2010,2028 ----
  	if (nchildren > 0)
  	{
  		rdata[lastrdata].next = &(rdata[2]);
+ 		xlrec->crec.xinfo |= XACT_COMPLETION_COMMITTED_SUB_PRESENT;
+ 		xlrec->crec.counts[lastrdata] = nchildren;
  		rdata[2].data = (char *) children;
  		rdata[2].len = nchildren * sizeof(TransactionId);
  		rdata[2].buffer = InvalidBuffer;
  		lastrdata = 2;
  	}
! 	/* dump shared cache invalidation messages */
  	if (ninvalmsgs > 0)
  	{
  		rdata[lastrdata].next = &(rdata[3]);
+ 		xlrec->crec.counts[lastrdata] = ninvalmsgs;
+ 		xlrec->crec.xinfo |= XACT_COMPLETION_SHARED_INV_PRESENT;
  		rdata[3].data = (char *) invalmsgs;
  		rdata[3].len = ninvalmsgs * sizeof(SharedInvalidationMessage);
  		rdata[3].buffer = InvalidBuffer;
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
new file mode 100644
index 2ca1c14..f758b41
*** a/src/backend/access/transam/xact.c
--- b/src/backend/access/transam/xact.c
*************** RecordTransactionCommit(void)
*** 964,970 ****
  		 */
  		XLogRecData rdata[4];
  		int			lastrdata = 0;
! 		xl_xact_commit xlrec;
  
  		/* Tell bufmgr and smgr to prepare for commit */
  		BufmgrCommit();
--- 964,985 ----
  		 */
  		XLogRecData rdata[4];
  		int			lastrdata = 0;
! 		xl_xact_commit *xlrec;
! 		int xlrec_len = MinSizeOfXactCommit;
! 		if (nrels > 0)
! 		{
! 			xlrec_len += sizeof(int);
! 		}
! 		if (nchildren > 0)
! 		{
! 			xlrec_len += sizeof(int);
! 		}
! 		if (nmsgs > 0)
! 		{
! 			xlrec_len += sizeof(int);
! 		}
! 
! 		xlrec = (xl_xact_commit *)palloc(xlrec_len);
  
  		/* Tell bufmgr and smgr to prepare for commit */
  		BufmgrCommit();
*************** RecordTransactionCommit(void)
*** 972,985 ****
  		/*
  		 * Set flags required for recovery processing of commits.
  		 */
! 		xlrec.xinfo = 0;
  		if (RelcacheInitFileInval)
! 			xlrec.xinfo |= XACT_COMPLETION_UPDATE_RELCACHE_FILE;
  		if (forceSyncCommit)
! 			xlrec.xinfo |= XACT_COMPLETION_FORCE_SYNC_COMMIT;
  
! 		xlrec.dbId = MyDatabaseId;
! 		xlrec.tsId = MyDatabaseTableSpace;
  
  		/*
  		 * Mark ourselves as within our "commit critical section".	This
--- 987,1000 ----
  		/*
  		 * Set flags required for recovery processing of commits.
  		 */
! 		xlrec->xinfo = 0;
  		if (RelcacheInitFileInval)
! 			xlrec->xinfo |= XACT_COMPLETION_UPDATE_RELCACHE_FILE;
  		if (forceSyncCommit)
! 			xlrec->xinfo |= XACT_COMPLETION_FORCE_SYNC_COMMIT;
  
! 		xlrec->dbId = MyDatabaseId;
! 		xlrec->tsId = MyDatabaseTableSpace;
  
  		/*
  		 * Mark ourselves as within our "commit critical section".	This
*************** RecordTransactionCommit(void)
*** 1002,1018 ****
  		MyProc->inCommit = true;
  
  		SetCurrentTransactionStopTimestamp();
! 		xlrec.xact_time = xactStopTimestamp;
! 		xlrec.nrels = nrels;
! 		xlrec.nsubxacts = nchildren;
! 		xlrec.nmsgs = nmsgs;
! 		rdata[0].data = (char *) (&xlrec);
! 		rdata[0].len = MinSizeOfXactCommit;
  		rdata[0].buffer = InvalidBuffer;
  		/* dump rels to delete */
  		if (nrels > 0)
  		{
  			rdata[0].next = &(rdata[1]);
  			rdata[1].data = (char *) rels;
  			rdata[1].len = nrels * sizeof(RelFileNode);
  			rdata[1].buffer = InvalidBuffer;
--- 1017,1032 ----
  		MyProc->inCommit = true;
  
  		SetCurrentTransactionStopTimestamp();
! 		xlrec->xact_time = xactStopTimestamp;
! 		rdata[0].data = (char *) (xlrec);
! 		rdata[0].len = xlrec_len;
  		rdata[0].buffer = InvalidBuffer;
  		/* dump rels to delete */
  		if (nrels > 0)
  		{
  			rdata[0].next = &(rdata[1]);
+ 			xlrec->xinfo |= XACT_COMPLETION_DROP_REL_NODES_PRESENT;
+ 			xlrec->counts[0] = nrels;
  			rdata[1].data = (char *) rels;
  			rdata[1].len = nrels * sizeof(RelFileNode);
  			rdata[1].buffer = InvalidBuffer;
*************** RecordTransactionCommit(void)
*** 1022,1027 ****
--- 1036,1043 ----
  		if (nchildren > 0)
  		{
  			rdata[lastrdata].next = &(rdata[2]);
+ 			xlrec->xinfo |= XACT_COMPLETION_COMMITTED_SUB_PRESENT;
+ 			xlrec->counts[lastrdata] = nchildren;
  			rdata[2].data = (char *) children;
  			rdata[2].len = nchildren * sizeof(TransactionId);
  			rdata[2].buffer = InvalidBuffer;
*************** RecordTransactionCommit(void)
*** 1031,1036 ****
--- 1047,1054 ----
  		if (nmsgs > 0)
  		{
  			rdata[lastrdata].next = &(rdata[3]);
+ 			xlrec->counts[lastrdata] = nmsgs;
+ 			xlrec->xinfo |= XACT_COMPLETION_SHARED_INV_PRESENT;
  			rdata[3].data = (char *) invalMessages;
  			rdata[3].len = nmsgs * sizeof(SharedInvalidationMessage);
  			rdata[3].buffer = InvalidBuffer;
*************** xactGetCommittedChildren(TransactionId *
*** 4437,4459 ****
   */
  
  /*
   * Before 9.0 this was a fairly short function, but now it performs many
   * actions for which the order of execution is critical.
   */
  static void
  xact_redo_commit(xl_xact_commit *xlrec, TransactionId xid, XLogRecPtr lsn)
  {
  	TransactionId *sub_xids;
  	SharedInvalidationMessage *inval_msgs;
  	TransactionId max_xid;
  	int			i;
  
! 	/* subxid array follows relfilenodes */
! 	sub_xids = (TransactionId *) &(xlrec->xnodes[xlrec->nrels]);
! 	/* invalidation messages array follows subxids */
! 	inval_msgs = (SharedInvalidationMessage *) &(sub_xids[xlrec->nsubxacts]);
! 
! 	max_xid = TransactionIdLatest(xid, xlrec->nsubxacts, sub_xids);
  
  	/*
  	 * Make sure nextXid is beyond any XID mentioned in the record.
--- 4455,4520 ----
   */
  
  /*
+  * Based on xlrec->xinfo, reads the optional arrays out of xlrec.
+  * The pointers returned by the function are valid only in case the
+  * corresponding int is not 0.
+  */
+ static void
+ xact_read_commit(xl_xact_commit *xlrec,
+ 					RelFileNode **drop_rels,
+ 					TransactionId **sub_xids,
+ 					SharedInvalidationMessage **inval_msgs,
+ 					int *nrels,
+ 					int *nsubxacts,
+ 					int *nmsgs)
+ {
+ 	int lastpos = 0;
+ 	*nrels = 0;
+ 	*nsubxacts = 0;
+ 	*nmsgs = 0;
+ 	if (XactCompletionDropRelPresent(xlrec))
+ 	{
+ 		*nrels = xlrec->counts[lastpos];
+ 		lastpos++;
+ 	}
+ 
+ 	if (XactCompletionCommittedSubPresent(xlrec))
+ 	{
+ 		*nsubxacts = xlrec->counts[lastpos];
+ 		lastpos++;
+ 	}
+ 
+ 	if (XactCompletionSharedInvPresent(xlrec))
+ 	{
+ 		*nmsgs = xlrec->counts[lastpos];
+ 		lastpos++;
+ 	}
+ 
+ 
+ 	*drop_rels = (RelFileNode *) &xlrec->counts[lastpos];
+ 	*sub_xids = (TransactionId *) &drop_rels[*nrels];
+ 	*inval_msgs = (SharedInvalidationMessage *) &sub_xids[*nsubxacts];
+ }
+ 
+ /*
   * Before 9.0 this was a fairly short function, but now it performs many
   * actions for which the order of execution is critical.
   */
  static void
  xact_redo_commit(xl_xact_commit *xlrec, TransactionId xid, XLogRecPtr lsn)
  {
+ 	RelFileNode		*drop_rels;
  	TransactionId *sub_xids;
  	SharedInvalidationMessage *inval_msgs;
  	TransactionId max_xid;
  	int			i;
+ 	int 		nrels = 0;
+ 	int 		nsubxacts = 0;
+ 	int 		nmsgs = 0;
  
! 	xact_read_commit(xlrec, &drop_rels, &sub_xids, &inval_msgs, &nrels,
! 						&nsubxacts, &nmsgs);
! 	max_xid = TransactionIdLatest(xid, nsubxacts, sub_xids);
  
  	/*
  	 * Make sure nextXid is beyond any XID mentioned in the record.
*************** xact_redo_commit(xl_xact_commit *xlrec, 
*** 4476,4482 ****
  		/*
  		 * Mark the transaction committed in pg_clog.
  		 */
! 		TransactionIdCommitTree(xid, xlrec->nsubxacts, sub_xids);
  	}
  	else
  	{
--- 4537,4543 ----
  		/*
  		 * Mark the transaction committed in pg_clog.
  		 */
! 		TransactionIdCommitTree(xid, nsubxacts, sub_xids);
  	}
  	else
  	{
*************** xact_redo_commit(xl_xact_commit *xlrec, 
*** 4500,4518 ****
  		 * bits set on changes made by transactions that haven't yet
  		 * recovered. It's unlikely but it's good to be safe.
  		 */
! 		TransactionIdAsyncCommitTree(xid, xlrec->nsubxacts, sub_xids, lsn);
  
  		/*
  		 * We must mark clog before we update the ProcArray.
  		 */
! 		ExpireTreeKnownAssignedTransactionIds(xid, xlrec->nsubxacts, sub_xids, max_xid);
  
  		/*
  		 * Send any cache invalidations attached to the commit. We must
  		 * maintain the same order of invalidation then release locks as
  		 * occurs in CommitTransaction().
  		 */
! 		ProcessCommittedInvalidationMessages(inval_msgs, xlrec->nmsgs,
  								  XactCompletionRelcacheInitFileInval(xlrec),
  											 xlrec->dbId, xlrec->tsId);
  
--- 4561,4579 ----
  		 * bits set on changes made by transactions that haven't yet
  		 * recovered. It's unlikely but it's good to be safe.
  		 */
! 		TransactionIdAsyncCommitTree(xid, nsubxacts, sub_xids, lsn);
  
  		/*
  		 * We must mark clog before we update the ProcArray.
  		 */
! 		ExpireTreeKnownAssignedTransactionIds(xid, nsubxacts, sub_xids, max_xid);
  
  		/*
  		 * Send any cache invalidations attached to the commit. We must
  		 * maintain the same order of invalidation then release locks as
  		 * occurs in CommitTransaction().
  		 */
! 		ProcessCommittedInvalidationMessages(inval_msgs, nmsgs,
  								  XactCompletionRelcacheInitFileInval(xlrec),
  											 xlrec->dbId, xlrec->tsId);
  
*************** xact_redo_commit(xl_xact_commit *xlrec, 
*** 4521,4540 ****
  		 * phase transactions. In effect we are ignoring the prepare phase and
  		 * just going straight to lock release.
  		 */
! 		StandbyReleaseLockTree(xid, xlrec->nsubxacts, sub_xids);
  	}
  
  	/* Make sure files supposed to be dropped are dropped */
! 	for (i = 0; i < xlrec->nrels; i++)
  	{
! 		SMgrRelation srel = smgropen(xlrec->xnodes[i], InvalidBackendId);
  		ForkNumber	fork;
  
  		for (fork = 0; fork <= MAX_FORKNUM; fork++)
  		{
  			if (smgrexists(srel, fork))
  			{
! 				XLogDropRelation(xlrec->xnodes[i], fork);
  				smgrdounlink(srel, fork, true);
  			}
  		}
--- 4582,4601 ----
  		 * phase transactions. In effect we are ignoring the prepare phase and
  		 * just going straight to lock release.
  		 */
! 		StandbyReleaseLockTree(xid, nsubxacts, sub_xids);
  	}
  
  	/* Make sure files supposed to be dropped are dropped */
! 	for (i = 0; i < nrels; i++)
  	{
! 		SMgrRelation srel = smgropen(drop_rels[i], InvalidBackendId);
  		ForkNumber	fork;
  
  		for (fork = 0; fork <= MAX_FORKNUM; fork++)
  		{
  			if (smgrexists(srel, fork))
  			{
! 				XLogDropRelation(drop_rels[i], fork);
  				smgrdounlink(srel, fork, true);
  			}
  		}
*************** xact_desc_commit(StringInfo buf, xl_xact
*** 4704,4745 ****
  {
  	int			i;
  	TransactionId *xacts;
  
! 	xacts = (TransactionId *) &xlrec->xnodes[xlrec->nrels];
  
  	appendStringInfoString(buf, timestamptz_to_str(xlrec->xact_time));
  
! 	if (xlrec->nrels > 0)
  	{
  		appendStringInfo(buf, "; rels:");
! 		for (i = 0; i < xlrec->nrels; i++)
  		{
! 			char	   *path = relpathperm(xlrec->xnodes[i], MAIN_FORKNUM);
  
  			appendStringInfo(buf, " %s", path);
  			pfree(path);
  		}
  	}
! 	if (xlrec->nsubxacts > 0)
  	{
  		appendStringInfo(buf, "; subxacts:");
! 		for (i = 0; i < xlrec->nsubxacts; i++)
  			appendStringInfo(buf, " %u", xacts[i]);
  	}
! 	if (xlrec->nmsgs > 0)
  	{
- 		SharedInvalidationMessage *msgs;
- 
- 		msgs = (SharedInvalidationMessage *) &xacts[xlrec->nsubxacts];
- 
  		if (XactCompletionRelcacheInitFileInval(xlrec))
  			appendStringInfo(buf, "; relcache init file inval dbid %u tsid %u",
  							 xlrec->dbId, xlrec->tsId);
  
  		appendStringInfo(buf, "; inval msgs:");
! 		for (i = 0; i < xlrec->nmsgs; i++)
  		{
! 			SharedInvalidationMessage *msg = &msgs[i];
  
  			if (msg->id >= 0)
  				appendStringInfo(buf, " catcache %d", msg->id);
--- 4765,4808 ----
  {
  	int			i;
  	TransactionId *xacts;
+ 	RelFileNode		*drop_rels;
+ 	SharedInvalidationMessage *inval_msgs;
+ 	int 		nrels = 0;
+ 	int 		nsubxacts = 0;
+ 	int 		nmsgs = 0;
  
! 	xact_read_commit(xlrec, &drop_rels, &xacts, &inval_msgs, &nrels,
! 						&nsubxacts, &nmsgs);
  
  	appendStringInfoString(buf, timestamptz_to_str(xlrec->xact_time));
  
! 	if (nrels > 0)
  	{
  		appendStringInfo(buf, "; rels:");
! 		for (i = 0; i < nrels; i++)
  		{
! 			char	   *path = relpathperm(drop_rels[i], MAIN_FORKNUM);
  
  			appendStringInfo(buf, " %s", path);
  			pfree(path);
  		}
  	}
! 	if (nsubxacts > 0)
  	{
  		appendStringInfo(buf, "; subxacts:");
! 		for (i = 0; i < nsubxacts; i++)
  			appendStringInfo(buf, " %u", xacts[i]);
  	}
! 	if (nmsgs > 0)
  	{
  		if (XactCompletionRelcacheInitFileInval(xlrec))
  			appendStringInfo(buf, "; relcache init file inval dbid %u tsid %u",
  							 xlrec->dbId, xlrec->tsId);
  
  		appendStringInfo(buf, "; inval msgs:");
! 		for (i = 0; i < nmsgs; i++)
  		{
! 			SharedInvalidationMessage *msg = &inval_msgs[i];
  
  			if (msg->id >= 0)
  				appendStringInfo(buf, " catcache %d", msg->id);
diff --git a/src/include/access/xact.h b/src/include/access/xact.h
new file mode 100644
index cb440d4..402437f
*** a/src/include/access/xact.h
--- b/src/include/access/xact.h
*************** typedef struct xl_xact_assignment
*** 116,137 ****
  
  #define MinSizeOfXactAssignment offsetof(xl_xact_assignment, xsub)
  
  typedef struct xl_xact_commit
  {
  	TimestampTz xact_time;		/* time of commit */
  	uint32		xinfo;			/* info flags */
- 	int			nrels;			/* number of RelFileNodes */
- 	int			nsubxacts;		/* number of subtransaction XIDs */
- 	int			nmsgs;			/* number of shared inval msgs */
  	Oid			dbId;			/* MyDatabaseId */
  	Oid			tsId;			/* MyDatabaseTableSpace */
! 	/* Array of RelFileNode(s) to drop at commit */
! 	RelFileNode xnodes[1];		/* VARIABLE LENGTH ARRAY */
! 	/* ARRAY OF COMMITTED SUBTRANSACTION XIDs FOLLOWS */
! 	/* ARRAY OF SHARED INVALIDATION MESSAGES FOLLOWS */
  } xl_xact_commit;
  
! #define MinSizeOfXactCommit offsetof(xl_xact_commit, xnodes)
  
  /*
   * These flags are set in the xinfo fields of WAL commit records,
--- 116,146 ----
  
  #define MinSizeOfXactAssignment offsetof(xl_xact_assignment, xsub)
  
+ /*
+  * As of 9.2, the record format changed to reduce space: instead of having
+  * ints that represent the length of the optional arrays at the end of
+  * xl_xact_commit, some bits in xinfo are used to flag which optional arrays
+  * are, in fact, present. For each present array, an int that represents the
+  * array length is placed at the end of the struct. The present arrays are
+  * stored after those ints.
+  */
  typedef struct xl_xact_commit
  {
  	TimestampTz xact_time;		/* time of commit */
  	uint32		xinfo;			/* info flags */
  	Oid			dbId;			/* MyDatabaseId */
  	Oid			tsId;			/* MyDatabaseTableSpace */
! 	int  		counts[1];  	/* variable-length array of counts, xinfo flags */
! 								/* define length of array and meaning of counts */
! 	/* OPTIONAL RELATIONS TO DROP LENGTH FOLLOWS */
! 	/* OPTIONAL COMMITTED SUBTRANSACTION XIDs LENGTH FOLLOWS */
! 	/* OPTIONAL SHARED INVALIDATION MESSAGES LENGTH FOLLOWS */
! 	/* ARRAY OF OPTIONAL RELATIONS TO DROP FOLLOWS */
! 	/* ARRAY OF OPTIONAL COMMITTED SUBTRANSACTION XIDs FOLLOWS */
! 	/* ARRAY OF OPTIONAL SHARED INVALIDATION MESSAGES FOLLOWS */
  } xl_xact_commit;
  
! #define MinSizeOfXactCommit offsetof(xl_xact_commit, counts)
  
  /*
   * These flags are set in the xinfo fields of WAL commit records,
*************** typedef struct xl_xact_commit
*** 148,153 ****
--- 157,176 ----
  #define XactCompletionRelcacheInitFileInval(xlrec)	((xlrec)->xinfo & XACT_COMPLETION_UPDATE_RELCACHE_FILE)
  #define XactCompletionForceSyncCommit(xlrec)		((xlrec)->xinfo & XACT_COMPLETION_FORCE_SYNC_COMMIT)
  
+ /*
+  * These flags are set in the xinfo fields of WAL commit records,
+  * indicating the presence of optionals xl_xact_commit_opt structures
+  * (RelFileNodes to drop at commit, subtransaction XIDs, shared inval msgs)
+  */
+ #define XACT_COMPLETION_DROP_REL_NODES_PRESENT	0x04
+ #define XACT_COMPLETION_COMMITTED_SUB_PRESENT	0x08
+ #define XACT_COMPLETION_SHARED_INV_PRESENT		0x10
+ 
+ /* Access macros for above flags */
+ #define XactCompletionDropRelPresent(xlrec)	((xlrec)->xinfo & XACT_COMPLETION_DROP_REL_NODES_PRESENT)
+ #define XactCompletionCommittedSubPresent(xlrec)		((xlrec)->xinfo & XACT_COMPLETION_COMMITTED_SUB_PRESENT)
+ #define XactCompletionSharedInvPresent(xlrec)		((xlrec)->xinfo & XACT_COMPLETION_SHARED_INV_PRESENT)
+ 
  typedef struct xl_xact_abort
  {
  	TimestampTz xact_time;		/* time of abort */
*************** typedef struct xl_xact_commit_prepared
*** 176,182 ****
  	/* MORE DATA FOLLOWS AT END OF STRUCT */
  } xl_xact_commit_prepared;
  
! #define MinSizeOfXactCommitPrepared offsetof(xl_xact_commit_prepared, crec.xnodes)
  
  typedef struct xl_xact_abort_prepared
  {
--- 199,205 ----
  	/* MORE DATA FOLLOWS AT END OF STRUCT */
  } xl_xact_commit_prepared;
  
! #define MinSizeOfXactCommitPrepared offsetof(xl_xact_commit_prepared, crec.counts)
  
  typedef struct xl_xact_abort_prepared
  {
#10Simon Riggs
simon@2ndQuadrant.com
In reply to: Leonardo Francalanci (#7)
Re: use less space in xl_xact_commit patch

On Wed, May 25, 2011 at 2:43 PM, Leonardo Francalanci <m_lists@yahoo.it> wrote:

Da: Simon Riggs <simon@2ndQuadrant.com>
I can't find a clear  discussion of what you are trying to do, and how,
just a URL back to a  complex discussion on another topic.

While trying to write a patch to allow changing an unlogged table into
a logged one, I had to add another int field to xl_xact_commit.
Robert Haas said:

 "I have to admit I don't like this approach very much.  I can't see
adding 4 bytes to every commit record for this feature."

which is a correct remark.

xl_xact_commit can contain some arrays (relation to drops,
committed sub-trans, shared invalidation msgs). The length of
these arrays is specified using 3 ints in the struct.

So, to avoid adding more ints to the struct, I've been suggested to
remove all the ints, and use   xl_xact_commit.xinfo to flag which
arrays are, in fact, present.

So the whole idea is:

- remove nrels, nsubxacts and nmsgs from xl_xact_commit
- use bits in xinfo to signal which arrays are present at the end
of   xl_xact_commit
- for each present array, add the length of the array (as int) at
the end of    xl_xact_commit
- add each present array after all the lengths

OK, thats clear. Thanks.

That formatting sounds quite complex.

I would propose we split this into 2 WAL records: xl_xact_commit and
xl_xact_commit_with_info

xl_xact_commit doesn't have any flags, counts or arrays.

xl_xact_commit_with_info always has all 3 counts, even if zero.
Arrays follow the main record

I think it might also be possible to removed dbId and tsId from
xl_act_commit if we use that definition.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

#11Simon Riggs
simon@2ndQuadrant.com
In reply to: Simon Riggs (#10)
Re: use less space in xl_xact_commit patch

On Wed, May 25, 2011 at 3:41 PM, Simon Riggs <simon@2ndquadrant.com> wrote:

On Wed, May 25, 2011 at 2:43 PM, Leonardo Francalanci <m_lists@yahoo.it> wrote:

Da: Simon Riggs <simon@2ndQuadrant.com>
I can't find a clear  discussion of what you are trying to do, and how,
just a URL back to a  complex discussion on another topic.

While trying to write a patch to allow changing an unlogged table into
a logged one, I had to add another int field to xl_xact_commit.
Robert Haas said:

 "I have to admit I don't like this approach very much.  I can't see
adding 4 bytes to every commit record for this feature."

which is a correct remark.

xl_xact_commit can contain some arrays (relation to drops,
committed sub-trans, shared invalidation msgs). The length of
these arrays is specified using 3 ints in the struct.

So, to avoid adding more ints to the struct, I've been suggested to
remove all the ints, and use   xl_xact_commit.xinfo to flag which
arrays are, in fact, present.

So the whole idea is:

- remove nrels, nsubxacts and nmsgs from xl_xact_commit
- use bits in xinfo to signal which arrays are present at the end
of   xl_xact_commit
- for each present array, add the length of the array (as int) at
the end of    xl_xact_commit
- add each present array after all the lengths

OK, thats clear. Thanks.

That formatting sounds quite complex.

I would propose we split this into 2 WAL records: xl_xact_commit and
xl_xact_commit_with_info

xl_xact_commit doesn't have any flags, counts or arrays.

xl_xact_commit_with_info always has all 3 counts, even if zero.
Arrays follow the main record

I think it might also be possible to removed dbId and tsId from
xl_act_commit if we use that definition.

Yes, that's correct. We can remove them from the normal commit record
when nmsgs == 0.

I think we can get away with these 2 definitions, but pls check. Using
static definitions works better for me because we can see what they
contain, rather than having the info flags imply that the record can
contain any permutation of settings when that's not really possible.

{
TimestampTz xact_time; /* time of commit */
int nsubxacts; /* number of subtransaction XIDs */
/* ARRAY OF COMMITTED SUBTRANSACTION XIDs FOLLOWS */
} xl_xact_commit;

{
TimestampTz xact_time; /* time of commit */
uint32 xinfo; /* info flags */
int nrels; /* number of RelFileNodes */
int nsubxacts; /* number of subtransaction XIDs */
int nmsgs; /* number of shared inval msgs */
Oid dbId; /* MyDatabaseId */
Oid tsId; /* MyDatabaseTableSpace */
/* Array of RelFileNode(s) to drop at commit */
RelFileNode xnodes[1]; /* VARIABLE LENGTH ARRAY */
/* ARRAY OF COMMITTED SUBTRANSACTION XIDs FOLLOWS */
/* ARRAY OF SHARED INVALIDATION MESSAGES FOLLOWS */
} xl_xact_commit_with_info;

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

#12Robert Haas
robertmhaas@gmail.com
In reply to: Simon Riggs (#10)
Re: use less space in xl_xact_commit patch

On Wed, May 25, 2011 at 10:41 AM, Simon Riggs <simon@2ndquadrant.com> wrote:

OK, thats clear. Thanks.

That formatting sounds quite complex.

I would propose we split this into 2 WAL records: xl_xact_commit and
xl_xact_commit_with_info

xl_xact_commit doesn't have any flags, counts or arrays.

xl_xact_commit_with_info always has all 3 counts, even if zero.
Arrays follow the main record

I think it might also be possible to removed dbId and tsId from
xl_act_commit if we use that definition.

That's an interesting idea. I hadn't noticed that if nmsgs=0 then
those fields aren't needed either. Also, both nrels>0 and nmsgs>0
will probably only happen if some DDL has been done, which most
transactions probably don't, so it would be logical to have one record
type that excludes nrels, nmsgs, dbId, tsId, and another record type
that includes all of those. In fact, we could probably throw in xinfo
as well: if there's no DDL involved, we shouldn't need to set
XACT_COMPLETION_UPDATE_RELCACHE_FILE or
XACT_COMPLETION_FORCE_SYNC_COMMIT either.

But I'm not so sure about nsubxacts. It seems to me that we might
lose a lot of the benefit of having two record types if we have to use
the larger one whenever nsubxacts>0. I'm not exactly sure how common
subtransactions are, but if I had to guess I'd speculate that they are
far more frequent than DDL operations. Maybe we should think about
making the xl_xact_commit record contain just xact_time, nsubxacts,
and a subxact array; and then have the xl_xact_commit_with_info (or
maybe xl_xact_commit_full, or whatever we decide to call it) contain
everything else. That would shave off 20 bytes from the commit
records of any non-DDL operation, which would be pretty nice.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#13Robert Haas
robertmhaas@gmail.com
In reply to: Simon Riggs (#11)
Re: use less space in xl_xact_commit patch

On Wed, May 25, 2011 at 3:05 PM, Simon Riggs <simon@2ndquadrant.com> wrote:

On Wed, May 25, 2011 at 3:41 PM, Simon Riggs <simon@2ndquadrant.com> wrote:

On Wed, May 25, 2011 at 2:43 PM, Leonardo Francalanci <m_lists@yahoo.it> wrote:

Da: Simon Riggs <simon@2ndQuadrant.com>
I can't find a clear  discussion of what you are trying to do, and how,
just a URL back to a  complex discussion on another topic.

While trying to write a patch to allow changing an unlogged table into
a logged one, I had to add another int field to xl_xact_commit.
Robert Haas said:

 "I have to admit I don't like this approach very much.  I can't see
adding 4 bytes to every commit record for this feature."

which is a correct remark.

xl_xact_commit can contain some arrays (relation to drops,
committed sub-trans, shared invalidation msgs). The length of
these arrays is specified using 3 ints in the struct.

So, to avoid adding more ints to the struct, I've been suggested to
remove all the ints, and use   xl_xact_commit.xinfo to flag which
arrays are, in fact, present.

So the whole idea is:

- remove nrels, nsubxacts and nmsgs from xl_xact_commit
- use bits in xinfo to signal which arrays are present at the end
of   xl_xact_commit
- for each present array, add the length of the array (as int) at
the end of    xl_xact_commit
- add each present array after all the lengths

OK, thats clear. Thanks.

That formatting sounds quite complex.

I would propose we split this into 2 WAL records: xl_xact_commit and
xl_xact_commit_with_info

xl_xact_commit doesn't have any flags, counts or arrays.

xl_xact_commit_with_info always has all 3 counts, even if zero.
Arrays follow the main record

I think it might also be possible to removed dbId and tsId from
xl_act_commit if we use that definition.

Yes, that's correct. We can remove them from the normal commit record
when nmsgs == 0.

I think we can get away with these 2 definitions, but pls check. Using
static definitions works better for me because we can see what they
contain, rather than having the info flags imply that the record can
contain any permutation of settings when that's not really possible.

 {
       TimestampTz xact_time;          /* time of commit */
       int                     nsubxacts;              /* number of subtransaction XIDs */
       /* ARRAY OF COMMITTED SUBTRANSACTION XIDs FOLLOWS */
 } xl_xact_commit;

 {
       TimestampTz xact_time;          /* time of commit */
       uint32          xinfo;                  /* info flags */
       int                     nrels;                  /* number of RelFileNodes */
       int                     nsubxacts;              /* number of subtransaction XIDs */
       int                     nmsgs;          /* number of shared inval msgs */
       Oid                     dbId;                   /* MyDatabaseId */
       Oid                     tsId;                   /* MyDatabaseTableSpace */
       /* Array of RelFileNode(s) to drop at commit */
       RelFileNode xnodes[1];          /* VARIABLE LENGTH ARRAY */
       /* ARRAY OF COMMITTED SUBTRANSACTION XIDs FOLLOWS */
       /* ARRAY OF SHARED INVALIDATION MESSAGES FOLLOWS */
 } xl_xact_commit_with_info;

Wow, that is identical to the conclusion that I came to about 60 seconds ago.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#14Robert Haas
robertmhaas@gmail.com
In reply to: Simon Riggs (#11)
Re: use less space in xl_xact_commit patch

On Wed, May 25, 2011 at 3:05 PM, Simon Riggs <simon@2ndquadrant.com> wrote:

Yes, that's correct. We can remove them from the normal commit record
when nmsgs == 0.

Leonardo, can you submit an updated version of this patch today that
incorporates Simon's suggestion? The CommitFest starts tomorrow. If
not, please feel free to resubmit to the next CommitFest. Simon or I
may find the time to review it sooner rather than later even if it
misses the deadline for this CommitFest, because I think it's an
important optimization and I know you have other work that depends on
it. But for now we should mark it Returned with Feedback if you don't
have an updated version ready to go.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#15Simon Riggs
simon@2ndQuadrant.com
In reply to: Robert Haas (#14)
Re: use less space in xl_xact_commit patch

On Tue, Jun 14, 2011 at 4:21 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On Wed, May 25, 2011 at 3:05 PM, Simon Riggs <simon@2ndquadrant.com> wrote:

Yes, that's correct. We can remove them from the normal commit record
when nmsgs == 0.

Leonardo, can you submit an updated version of this patch today that
incorporates Simon's suggestion?  The CommitFest starts tomorrow.  If
not, please feel free to resubmit to the next CommitFest.  Simon or I
may find the time to review it sooner rather than later even if it
misses the deadline for this CommitFest, because I think it's an
important optimization and I know you have other work that depends on
it.  But for now we should mark it Returned with Feedback if you don't
have an updated version ready to go.

We don't need to be in a hurry here. As the reviewer I'm happy to give
Leonardo some time, obviously no more than the end of the commit fest.

If he doesn't respond at all, I'll do it, but I'd like to give him the
chance and the experience if possible.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

#16Robert Haas
robertmhaas@gmail.com
In reply to: Simon Riggs (#15)
Re: use less space in xl_xact_commit patch

On Tue, Jun 14, 2011 at 2:31 PM, Simon Riggs <simon@2ndquadrant.com> wrote:

We don't need to be in a hurry here. As the reviewer I'm happy to give
Leonardo some time, obviously no more than the end of the commit fest.

Well, we certainly have the option to review and commit the patch any
time up until feature freeze. However, I don't want the CommitFest
application to be full of entries for patches that are not actually
being worked on, because it makes it hard for reviewers to figure out
which patches in a state where they can be usefully looked at. AIUI,
this one is currently not, because it was reviewed three weeks ago and
hasn't been updated.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#17Leonardo Francalanci
m_lists@yahoo.it
In reply to: Simon Riggs (#15)
Re: use less space in xl_xact_commit patch

We don't need to be in a hurry here. As the reviewer I'm happy to give
Leonardo some time, obviously no more than the end of the commit fest.

If he doesn't respond at all, I'll do it, but I'd like to give him the
chance and the experience if possible.

Sorry I couldn't update the patch (in fact, it's more of a total-rewrite than
an update).

How much time do I have?

Leonardo

#18Leonardo Francalanci
m_lists@yahoo.it
In reply to: Robert Haas (#16)
Re: use less space in xl_xact_commit patch

On Tue, Jun 14, 2011 at 2:31 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
Well, we certainly have the option to review and commit the patch any
time up until feature freeze. However, I don't want the CommitFest
application to be full of entries for patches that are not actually
being worked on, because it makes it hard for reviewers to figure out
which patches in a state where they can be usefully looked at. AIUI,
this one is currently not, because it was reviewed three weeks ago and
hasn't been updated.

Yes it's true: I thought I could find the time to work on it, but I didn't.
Let me know the deadline for it, and I'll see if I can make it (or I'll make
it in the next commit fest).

Leonardo

#19Leonardo Francalanci
m_lists@yahoo.it
In reply to: Simon Riggs (#15)
1 attachment(s)
Re: use less space in xl_xact_commit patch

On Wed, May 25, 2011 at 3:05 PM, Simon Riggs <simon@2ndquadrant.com>

wrote:

Leonardo, can you submit an updated version of this patch today that
incorporates Simon's suggestion?

Mmmh, maybe it was simpler than I thought; I must be
missing something... patch attached

How can I test it with "weird" stuff as subtransactions, shared
cache invalidation messages...?

Leonardo

Attachments:

commitlog_lessbytes_v2.patchapplication/octet-stream; name=commitlog_lessbytes_v2.patchDownload
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
new file mode 100644
index 2ca1c14..727b86b
*** a/src/backend/access/transam/xact.c
--- b/src/backend/access/transam/xact.c
*************** RecordTransactionCommit(void)
*** 962,987 ****
  		/*
  		 * Begin commit critical section and insert the commit XLOG record.
  		 */
- 		XLogRecData rdata[4];
- 		int			lastrdata = 0;
- 		xl_xact_commit xlrec;
- 
  		/* Tell bufmgr and smgr to prepare for commit */
  		BufmgrCommit();
  
  		/*
- 		 * Set flags required for recovery processing of commits.
- 		 */
- 		xlrec.xinfo = 0;
- 		if (RelcacheInitFileInval)
- 			xlrec.xinfo |= XACT_COMPLETION_UPDATE_RELCACHE_FILE;
- 		if (forceSyncCommit)
- 			xlrec.xinfo |= XACT_COMPLETION_FORCE_SYNC_COMMIT;
- 
- 		xlrec.dbId = MyDatabaseId;
- 		xlrec.tsId = MyDatabaseTableSpace;
- 
- 		/*
  		 * Mark ourselves as within our "commit critical section".	This
  		 * forces any concurrent checkpoint to wait until we've updated
  		 * pg_clog.  Without this, it is possible for the checkpoint to set
--- 962,971 ----
*************** RecordTransactionCommit(void)
*** 1002,1044 ****
  		MyProc->inCommit = true;
  
  		SetCurrentTransactionStopTimestamp();
! 		xlrec.xact_time = xactStopTimestamp;
! 		xlrec.nrels = nrels;
! 		xlrec.nsubxacts = nchildren;
! 		xlrec.nmsgs = nmsgs;
! 		rdata[0].data = (char *) (&xlrec);
! 		rdata[0].len = MinSizeOfXactCommit;
! 		rdata[0].buffer = InvalidBuffer;
! 		/* dump rels to delete */
! 		if (nrels > 0)
! 		{
! 			rdata[0].next = &(rdata[1]);
! 			rdata[1].data = (char *) rels;
! 			rdata[1].len = nrels * sizeof(RelFileNode);
! 			rdata[1].buffer = InvalidBuffer;
! 			lastrdata = 1;
! 		}
! 		/* dump committed child Xids */
! 		if (nchildren > 0)
  		{
! 			rdata[lastrdata].next = &(rdata[2]);
! 			rdata[2].data = (char *) children;
! 			rdata[2].len = nchildren * sizeof(TransactionId);
! 			rdata[2].buffer = InvalidBuffer;
! 			lastrdata = 2;
  		}
! 		/* dump shared cache invalidation messages */
! 		if (nmsgs > 0)
  		{
! 			rdata[lastrdata].next = &(rdata[3]);
! 			rdata[3].data = (char *) invalMessages;
! 			rdata[3].len = nmsgs * sizeof(SharedInvalidationMessage);
! 			rdata[3].buffer = InvalidBuffer;
! 			lastrdata = 3;
! 		}
! 		rdata[lastrdata].next = NULL;
  
! 		(void) XLogInsert(RM_XACT_ID, XLOG_XACT_COMMIT, rdata);
  	}
  
  	/*
--- 986,1071 ----
  		MyProc->inCommit = true;
  
  		SetCurrentTransactionStopTimestamp();
! 
! 		if (nrels == 0 && nmsgs == 0)
  		{
! 			XLogRecData rdata[2];
! 			int			lastrdata = 0;
! 			xl_xact_commit xlrec;
! 			xlrec.xact_time = xactStopTimestamp;
! 			xlrec.nsubxacts = nchildren;
! 			rdata[0].data = (char *) (&xlrec);
! 			rdata[0].len = MinSizeOfXactCommit;
! 			rdata[0].buffer = InvalidBuffer;
! 			/* dump committed child Xids */
! 			if (nchildren > 0)
! 			{
! 				rdata[0].next = &(rdata[1]);
! 				rdata[1].data = (char *) children;
! 				rdata[1].len = nchildren * sizeof(TransactionId);
! 				rdata[1].buffer = InvalidBuffer;
! 				lastrdata = 1;
! 			}
! 			rdata[lastrdata].next = NULL;
! 
! 			(void) XLogInsert(RM_XACT_ID, XLOG_XACT_COMMIT, rdata);
! 
  		}
! 		else
  		{
! 			XLogRecData rdata[4];
! 			int			lastrdata = 0;
! 			xl_xact_commit_with_info xlrec;
! 			/*
! 			 * Set flags required for recovery processing of commits.
! 			 */
! 			xlrec.xinfo = 0;
! 			if (RelcacheInitFileInval)
! 				xlrec.xinfo |= XACT_COMPLETION_UPDATE_RELCACHE_FILE;
! 			if (forceSyncCommit)
! 				xlrec.xinfo |= XACT_COMPLETION_FORCE_SYNC_COMMIT;
  
! 			xlrec.dbId = MyDatabaseId;
! 			xlrec.tsId = MyDatabaseTableSpace;
! 
! 			xlrec.xact_time = xactStopTimestamp;
! 			xlrec.nrels = nrels;
! 			xlrec.nsubxacts = nchildren;
! 			xlrec.nmsgs = nmsgs;
! 			rdata[0].data = (char *) (&xlrec);
! 			rdata[0].len = MinSizeOfXactCommit;
! 			rdata[0].buffer = InvalidBuffer;
! 			/* dump rels to delete */
! 			if (nrels > 0)
! 			{
! 				rdata[0].next = &(rdata[1]);
! 				rdata[1].data = (char *) rels;
! 				rdata[1].len = nrels * sizeof(RelFileNode);
! 				rdata[1].buffer = InvalidBuffer;
! 				lastrdata = 1;
! 			}
! 			/* dump committed child Xids */
! 			if (nchildren > 0)
! 			{
! 				rdata[lastrdata].next = &(rdata[2]);
! 				rdata[2].data = (char *) children;
! 				rdata[2].len = nchildren * sizeof(TransactionId);
! 				rdata[2].buffer = InvalidBuffer;
! 				lastrdata = 2;
! 			}
! 			/* dump shared cache invalidation messages */
! 			if (nmsgs > 0)
! 			{
! 				rdata[lastrdata].next = &(rdata[3]);
! 				rdata[3].data = (char *) invalMessages;
! 				rdata[3].len = nmsgs * sizeof(SharedInvalidationMessage);
! 				rdata[3].buffer = InvalidBuffer;
! 				lastrdata = 3;
! 			}
! 			rdata[lastrdata].next = NULL;
! 
! 			(void) XLogInsert(RM_XACT_ID, XLOG_XACT_COMMIT_WITH_INFO, rdata);
! 		}
  	}
  
  	/*
*************** xactGetCommittedChildren(TransactionId *
*** 4441,4459 ****
   * actions for which the order of execution is critical.
   */
  static void
! xact_redo_commit(xl_xact_commit *xlrec, TransactionId xid, XLogRecPtr lsn)
  {
- 	TransactionId *sub_xids;
- 	SharedInvalidationMessage *inval_msgs;
  	TransactionId max_xid;
  	int			i;
  
! 	/* subxid array follows relfilenodes */
! 	sub_xids = (TransactionId *) &(xlrec->xnodes[xlrec->nrels]);
! 	/* invalidation messages array follows subxids */
! 	inval_msgs = (SharedInvalidationMessage *) &(sub_xids[xlrec->nsubxacts]);
! 
! 	max_xid = TransactionIdLatest(xid, xlrec->nsubxacts, sub_xids);
  
  	/*
  	 * Make sure nextXid is beyond any XID mentioned in the record.
--- 4468,4483 ----
   * actions for which the order of execution is critical.
   */
  static void
! xact_redo_commit(RelFileNode *xnodes, int nrels,
! 					TransactionId *sub_xids, int nsubxacts,
! 					SharedInvalidationMessage *inval_msgs, int nmsgs,
! 					Oid dbId, Oid tsId, uint32 xinfo,
! 					TransactionId xid, XLogRecPtr lsn)
  {
  	TransactionId max_xid;
  	int			i;
  
! 	max_xid = TransactionIdLatest(xid, nsubxacts, sub_xids);
  
  	/*
  	 * Make sure nextXid is beyond any XID mentioned in the record.
*************** xact_redo_commit(xl_xact_commit *xlrec, 
*** 4476,4482 ****
  		/*
  		 * Mark the transaction committed in pg_clog.
  		 */
! 		TransactionIdCommitTree(xid, xlrec->nsubxacts, sub_xids);
  	}
  	else
  	{
--- 4500,4506 ----
  		/*
  		 * Mark the transaction committed in pg_clog.
  		 */
! 		TransactionIdCommitTree(xid, nsubxacts, sub_xids);
  	}
  	else
  	{
*************** xact_redo_commit(xl_xact_commit *xlrec, 
*** 4500,4540 ****
  		 * bits set on changes made by transactions that haven't yet
  		 * recovered. It's unlikely but it's good to be safe.
  		 */
! 		TransactionIdAsyncCommitTree(xid, xlrec->nsubxacts, sub_xids, lsn);
  
  		/*
  		 * We must mark clog before we update the ProcArray.
  		 */
! 		ExpireTreeKnownAssignedTransactionIds(xid, xlrec->nsubxacts, sub_xids, max_xid);
  
  		/*
  		 * Send any cache invalidations attached to the commit. We must
  		 * maintain the same order of invalidation then release locks as
  		 * occurs in CommitTransaction().
  		 */
! 		ProcessCommittedInvalidationMessages(inval_msgs, xlrec->nmsgs,
! 								  XactCompletionRelcacheInitFileInval(xlrec),
! 											 xlrec->dbId, xlrec->tsId);
  
  		/*
  		 * Release locks, if any. We do this for both two phase and normal one
  		 * phase transactions. In effect we are ignoring the prepare phase and
  		 * just going straight to lock release.
  		 */
! 		StandbyReleaseLockTree(xid, xlrec->nsubxacts, sub_xids);
  	}
  
  	/* Make sure files supposed to be dropped are dropped */
! 	for (i = 0; i < xlrec->nrels; i++)
  	{
! 		SMgrRelation srel = smgropen(xlrec->xnodes[i], InvalidBackendId);
  		ForkNumber	fork;
  
  		for (fork = 0; fork <= MAX_FORKNUM; fork++)
  		{
  			if (smgrexists(srel, fork))
  			{
! 				XLogDropRelation(xlrec->xnodes[i], fork);
  				smgrdounlink(srel, fork, true);
  			}
  		}
--- 4524,4564 ----
  		 * bits set on changes made by transactions that haven't yet
  		 * recovered. It's unlikely but it's good to be safe.
  		 */
! 		TransactionIdAsyncCommitTree(xid, nsubxacts, sub_xids, lsn);
  
  		/*
  		 * We must mark clog before we update the ProcArray.
  		 */
! 		ExpireTreeKnownAssignedTransactionIds(xid, nsubxacts, sub_xids, max_xid);
  
  		/*
  		 * Send any cache invalidations attached to the commit. We must
  		 * maintain the same order of invalidation then release locks as
  		 * occurs in CommitTransaction().
  		 */
! 		ProcessCommittedInvalidationMessages(inval_msgs, nmsgs,
! 								  XactCompletionRelcacheInitFileInval(xinfo),
! 											 dbId, tsId);
  
  		/*
  		 * Release locks, if any. We do this for both two phase and normal one
  		 * phase transactions. In effect we are ignoring the prepare phase and
  		 * just going straight to lock release.
  		 */
! 		StandbyReleaseLockTree(xid, nsubxacts, sub_xids);
  	}
  
  	/* Make sure files supposed to be dropped are dropped */
! 	for (i = 0; i < nrels; i++)
  	{
! 		SMgrRelation srel = smgropen(xnodes[i], InvalidBackendId);
  		ForkNumber	fork;
  
  		for (fork = 0; fork <= MAX_FORKNUM; fork++)
  		{
  			if (smgrexists(srel, fork))
  			{
! 				XLogDropRelation(xnodes[i], fork);
  				smgrdounlink(srel, fork, true);
  			}
  		}
*************** xact_redo_commit(xl_xact_commit *xlrec, 
*** 4553,4560 ****
  	 * to reduce that problem window, for any user that requested
  	 * ForceSyncCommit().
  	 */
! 	if (XactCompletionForceSyncCommit(xlrec))
  		XLogFlush(lsn);
  }
  
  /*
--- 4577,4616 ----
  	 * to reduce that problem window, for any user that requested
  	 * ForceSyncCommit().
  	 */
! 	if (XactCompletionForceSyncCommit(xinfo))
  		XLogFlush(lsn);
+ 
+ }
+ /*
+  * utility function to call xact_redo_commit with parameters found in xlrec
+  */
+ static void
+ xact_redo_commit_with_info(xl_xact_commit_with_info *xlrec,
+ 							TransactionId xid, XLogRecPtr lsn)
+ {
+ 	TransactionId *sub_xids;
+ 	SharedInvalidationMessage *inval_msgs;
+ 
+ 	/* subxid array follows relfilenodes */
+ 	sub_xids = (TransactionId *) &(xlrec->xnodes[xlrec->nrels]);
+ 	/* invalidation messages array follows subxids */
+ 	inval_msgs = (SharedInvalidationMessage *) &(sub_xids[xlrec->nsubxacts]);
+ 
+ 	xact_redo_commit(xlrec->xnodes, xlrec->nrels, sub_xids, xlrec->nsubxacts,
+ 						inval_msgs, xlrec->nmsgs, xlrec->dbId, xlrec->tsId,
+ 						xlrec->xinfo, xid, lsn);
+ }
+ 
+ /*
+  * utility function to call xact_redo_commit with proper parameters:
+  * parameters not available in xl_xact_commit are defaulted to 0.
+  */
+ static void
+ xact_redo_commit_without_info(xl_xact_commit *xlrec,
+ 							TransactionId xid, XLogRecPtr lsn)
+ {
+ 	xact_redo_commit(NULL, 0, xlrec->children, xlrec->nsubxacts,
+ 						NULL, 0, InvalidOid, InvalidOid, 0, xid, lsn);
  }
  
  /*
*************** xact_redo(XLogRecPtr lsn, XLogRecord *re
*** 4659,4665 ****
  	{
  		xl_xact_commit *xlrec = (xl_xact_commit *) XLogRecGetData(record);
  
! 		xact_redo_commit(xlrec, record->xl_xid, lsn);
  	}
  	else if (info == XLOG_XACT_ABORT)
  	{
--- 4715,4727 ----
  	{
  		xl_xact_commit *xlrec = (xl_xact_commit *) XLogRecGetData(record);
  
! 		xact_redo_commit_without_info(xlrec, record->xl_xid, lsn);
! 	}
! 	else if (info == XLOG_XACT_COMMIT_WITH_INFO)
! 	{
! 		xl_xact_commit_with_info *xlrec = (xl_xact_commit_with_info *) XLogRecGetData(record);
! 
! 		xact_redo_commit_with_info(xlrec, record->xl_xid, lsn);
  	}
  	else if (info == XLOG_XACT_ABORT)
  	{
*************** xact_redo(XLogRecPtr lsn, XLogRecord *re
*** 4677,4683 ****
  	{
  		xl_xact_commit_prepared *xlrec = (xl_xact_commit_prepared *) XLogRecGetData(record);
  
! 		xact_redo_commit(&xlrec->crec, xlrec->xid, lsn);
  		RemoveTwoPhaseFile(xlrec->xid, false);
  	}
  	else if (info == XLOG_XACT_ABORT_PREPARED)
--- 4739,4745 ----
  	{
  		xl_xact_commit_prepared *xlrec = (xl_xact_commit_prepared *) XLogRecGetData(record);
  
! 		xact_redo_commit_with_info(&xlrec->crec, xlrec->xid, lsn);
  		RemoveTwoPhaseFile(xlrec->xid, false);
  	}
  	else if (info == XLOG_XACT_ABORT_PREPARED)
*************** xact_redo(XLogRecPtr lsn, XLogRecord *re
*** 4700,4706 ****
  }
  
  static void
! xact_desc_commit(StringInfo buf, xl_xact_commit *xlrec)
  {
  	int			i;
  	TransactionId *xacts;
--- 4762,4768 ----
  }
  
  static void
! xact_desc_commit_with_info(StringInfo buf, xl_xact_commit_with_info *xlrec)
  {
  	int			i;
  	TransactionId *xacts;
*************** xact_desc_commit(StringInfo buf, xl_xact
*** 4732,4738 ****
  
  		msgs = (SharedInvalidationMessage *) &xacts[xlrec->nsubxacts];
  
! 		if (XactCompletionRelcacheInitFileInval(xlrec))
  			appendStringInfo(buf, "; relcache init file inval dbid %u tsid %u",
  							 xlrec->dbId, xlrec->tsId);
  
--- 4794,4800 ----
  
  		msgs = (SharedInvalidationMessage *) &xacts[xlrec->nsubxacts];
  
! 		if (XactCompletionRelcacheInitFileInval(xlrec->xinfo))
  			appendStringInfo(buf, "; relcache init file inval dbid %u tsid %u",
  							 xlrec->dbId, xlrec->tsId);
  
*************** xact_desc_commit(StringInfo buf, xl_xact
*** 4759,4764 ****
--- 4821,4841 ----
  }
  
  static void
+ xact_desc_commit(StringInfo buf, xl_xact_commit *xlrec)
+ {
+ 	int			i;
+ 
+ 	appendStringInfoString(buf, timestamptz_to_str(xlrec->xact_time));
+ 
+ 	if (xlrec->nsubxacts > 0)
+ 	{
+ 		appendStringInfo(buf, "; subxacts:");
+ 		for (i = 0; i < xlrec->nsubxacts; i++)
+ 			appendStringInfo(buf, " %u", xlrec->children[i]);
+ 	}
+ }
+ 
+ static void
  xact_desc_abort(StringInfo buf, xl_xact_abort *xlrec)
  {
  	int			i;
*************** xact_desc(StringInfo buf, uint8 xl_info,
*** 4809,4814 ****
--- 4886,4898 ----
  		appendStringInfo(buf, "commit: ");
  		xact_desc_commit(buf, xlrec);
  	}
+ 	else if (info == XLOG_XACT_COMMIT_WITH_INFO)
+ 	{
+ 		xl_xact_commit_with_info *xlrec = (xl_xact_commit_with_info *) rec;
+ 
+ 		appendStringInfo(buf, "commit: ");
+ 		xact_desc_commit_with_info(buf, xlrec);
+ 	}
  	else if (info == XLOG_XACT_ABORT)
  	{
  		xl_xact_abort *xlrec = (xl_xact_abort *) rec;
*************** xact_desc(StringInfo buf, uint8 xl_info,
*** 4825,4831 ****
  		xl_xact_commit_prepared *xlrec = (xl_xact_commit_prepared *) rec;
  
  		appendStringInfo(buf, "commit prepared %u: ", xlrec->xid);
! 		xact_desc_commit(buf, &xlrec->crec);
  	}
  	else if (info == XLOG_XACT_ABORT_PREPARED)
  	{
--- 4909,4915 ----
  		xl_xact_commit_prepared *xlrec = (xl_xact_commit_prepared *) rec;
  
  		appendStringInfo(buf, "commit prepared %u: ", xlrec->xid);
! 		xact_desc_commit_with_info(buf, &xlrec->crec);
  	}
  	else if (info == XLOG_XACT_ABORT_PREPARED)
  	{
diff --git a/src/include/access/xact.h b/src/include/access/xact.h
new file mode 100644
index cb440d4..a75be3f
*** a/src/include/access/xact.h
--- b/src/include/access/xact.h
*************** typedef void (*SubXactCallback) (SubXact
*** 106,111 ****
--- 106,112 ----
  #define XLOG_XACT_COMMIT_PREPARED	0x30
  #define XLOG_XACT_ABORT_PREPARED	0x40
  #define XLOG_XACT_ASSIGNMENT		0x50
+ #define XLOG_XACT_COMMIT_WITH_INFO  0x60
  
  typedef struct xl_xact_assignment
  {
*************** typedef struct xl_xact_assignment
*** 119,124 ****
--- 120,136 ----
  typedef struct xl_xact_commit
  {
  	TimestampTz xact_time;		/* time of commit */
+ 	int			nsubxacts;		/* number of subtransaction XIDs */
+ 	/* ARRAY OF COMMITTED SUBTRANSACTION XIDs FOLLOWS */
+ 	TransactionId children[1];		/* VARIABLE LENGTH ARRAY */
+ } xl_xact_commit;
+ 
+ #define MinSizeOfXactCommit offsetof(xl_xact_commit, children)
+ 
+ 
+ typedef struct xl_xact_commit_with_info
+ {
+ 	TimestampTz xact_time;		/* time of commit */
  	uint32		xinfo;			/* info flags */
  	int			nrels;			/* number of RelFileNodes */
  	int			nsubxacts;		/* number of subtransaction XIDs */
*************** typedef struct xl_xact_commit
*** 129,137 ****
  	RelFileNode xnodes[1];		/* VARIABLE LENGTH ARRAY */
  	/* ARRAY OF COMMITTED SUBTRANSACTION XIDs FOLLOWS */
  	/* ARRAY OF SHARED INVALIDATION MESSAGES FOLLOWS */
! } xl_xact_commit;
  
! #define MinSizeOfXactCommit offsetof(xl_xact_commit, xnodes)
  
  /*
   * These flags are set in the xinfo fields of WAL commit records,
--- 141,149 ----
  	RelFileNode xnodes[1];		/* VARIABLE LENGTH ARRAY */
  	/* ARRAY OF COMMITTED SUBTRANSACTION XIDs FOLLOWS */
  	/* ARRAY OF SHARED INVALIDATION MESSAGES FOLLOWS */
! } xl_xact_commit_with_info;
  
! #define MinSizeOfXactCommitWithInfo offsetof(xl_xact_commit_with_info, xnodes)
  
  /*
   * These flags are set in the xinfo fields of WAL commit records,
*************** typedef struct xl_xact_commit
*** 145,152 ****
  #define XACT_COMPLETION_FORCE_SYNC_COMMIT		0x02
  
  /* Access macros for above flags */
! #define XactCompletionRelcacheInitFileInval(xlrec)	((xlrec)->xinfo & XACT_COMPLETION_UPDATE_RELCACHE_FILE)
! #define XactCompletionForceSyncCommit(xlrec)		((xlrec)->xinfo & XACT_COMPLETION_FORCE_SYNC_COMMIT)
  
  typedef struct xl_xact_abort
  {
--- 157,164 ----
  #define XACT_COMPLETION_FORCE_SYNC_COMMIT		0x02
  
  /* Access macros for above flags */
! #define XactCompletionRelcacheInitFileInval(xinfo)	(xinfo & XACT_COMPLETION_UPDATE_RELCACHE_FILE)
! #define XactCompletionForceSyncCommit(xinfo)		(xinfo & XACT_COMPLETION_FORCE_SYNC_COMMIT)
  
  typedef struct xl_xact_abort
  {
*************** typedef struct xl_xact_abort
*** 172,178 ****
  typedef struct xl_xact_commit_prepared
  {
  	TransactionId xid;			/* XID of prepared xact */
! 	xl_xact_commit crec;		/* COMMIT record */
  	/* MORE DATA FOLLOWS AT END OF STRUCT */
  } xl_xact_commit_prepared;
  
--- 184,190 ----
  typedef struct xl_xact_commit_prepared
  {
  	TransactionId xid;			/* XID of prepared xact */
! 	xl_xact_commit_with_info crec;		/* COMMIT record */
  	/* MORE DATA FOLLOWS AT END OF STRUCT */
  } xl_xact_commit_prepared;
  
#20Simon Riggs
simon@2ndQuadrant.com
In reply to: Leonardo Francalanci (#19)
Re: use less space in xl_xact_commit patch

On Wed, Jun 15, 2011 at 10:55 AM, Leonardo Francalanci <m_lists@yahoo.it> wrote:

 On Wed, May 25, 2011 at 3:05 PM, Simon Riggs <simon@2ndquadrant.com>

wrote:

Leonardo, can you  submit an updated version of this patch today that
incorporates Simon's  suggestion?

Mmmh, maybe it was simpler than I thought; I must be
missing something... patch attached

No, I think it is that simple.

Patch looks OK on initial eyeball. More detailed review to follow.

With regards to the naming, I think it would be better if we kept
XLOG_XACT_COMMIT record exactly as it is now, and make the second
record an entirely new record called XLOG_XACT_COMMIT_FASTPATH. That
way we retain backwards compatibility.

If you'd like to rework like that please, otherwise I can take it from
here if you'd like.

How can I test it with "weird" stuff as subtransactions, shared
cache invalidation messages...?

make installcheck should cover those.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

#21Leonardo Francalanci
m_lists@yahoo.it
In reply to: Simon Riggs (#20)
1 attachment(s)
Re: use less space in xl_xact_commit patch

With regards to the naming, I think it would be better if we kept
XLOG_XACT_COMMIT record exactly as it is now, and make the second
record an entirely new record called XLOG_XACT_COMMIT_FASTPATH. That
way we retain backwards compatibility.

If you'd like to rework like that please, otherwise I can take it from
here if you'd like.

I think I did it; while doing it, I think I've found a bug: I didn't update
"recoveryStopsHere". Please double check that, as I really don't
know what I'm doing there...
Should I also change the struct name from xl_xact_commit to
xl_xact_commit_fast_path?

How can I test it with "weird" stuff as subtransactions, shared
cache invalidation messages...?

make installcheck should cover those.

Ok, all tests passed.

Attachments:

commitlog_lessbytes_v3.patchapplication/octet-stream; name=commitlog_lessbytes_v3.patchDownload
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
new file mode 100644
index 2ca1c14..393a517
*** a/src/backend/access/transam/xact.c
--- b/src/backend/access/transam/xact.c
*************** RecordTransactionCommit(void)
*** 962,987 ****
  		/*
  		 * Begin commit critical section and insert the commit XLOG record.
  		 */
- 		XLogRecData rdata[4];
- 		int			lastrdata = 0;
- 		xl_xact_commit xlrec;
- 
  		/* Tell bufmgr and smgr to prepare for commit */
  		BufmgrCommit();
  
  		/*
- 		 * Set flags required for recovery processing of commits.
- 		 */
- 		xlrec.xinfo = 0;
- 		if (RelcacheInitFileInval)
- 			xlrec.xinfo |= XACT_COMPLETION_UPDATE_RELCACHE_FILE;
- 		if (forceSyncCommit)
- 			xlrec.xinfo |= XACT_COMPLETION_FORCE_SYNC_COMMIT;
- 
- 		xlrec.dbId = MyDatabaseId;
- 		xlrec.tsId = MyDatabaseTableSpace;
- 
- 		/*
  		 * Mark ourselves as within our "commit critical section".	This
  		 * forces any concurrent checkpoint to wait until we've updated
  		 * pg_clog.  Without this, it is possible for the checkpoint to set
--- 962,971 ----
*************** RecordTransactionCommit(void)
*** 1002,1044 ****
  		MyProc->inCommit = true;
  
  		SetCurrentTransactionStopTimestamp();
! 		xlrec.xact_time = xactStopTimestamp;
! 		xlrec.nrels = nrels;
! 		xlrec.nsubxacts = nchildren;
! 		xlrec.nmsgs = nmsgs;
! 		rdata[0].data = (char *) (&xlrec);
! 		rdata[0].len = MinSizeOfXactCommit;
! 		rdata[0].buffer = InvalidBuffer;
! 		/* dump rels to delete */
! 		if (nrels > 0)
! 		{
! 			rdata[0].next = &(rdata[1]);
! 			rdata[1].data = (char *) rels;
! 			rdata[1].len = nrels * sizeof(RelFileNode);
! 			rdata[1].buffer = InvalidBuffer;
! 			lastrdata = 1;
! 		}
! 		/* dump committed child Xids */
! 		if (nchildren > 0)
  		{
! 			rdata[lastrdata].next = &(rdata[2]);
! 			rdata[2].data = (char *) children;
! 			rdata[2].len = nchildren * sizeof(TransactionId);
! 			rdata[2].buffer = InvalidBuffer;
! 			lastrdata = 2;
  		}
! 		/* dump shared cache invalidation messages */
! 		if (nmsgs > 0)
  		{
! 			rdata[lastrdata].next = &(rdata[3]);
! 			rdata[3].data = (char *) invalMessages;
! 			rdata[3].len = nmsgs * sizeof(SharedInvalidationMessage);
! 			rdata[3].buffer = InvalidBuffer;
! 			lastrdata = 3;
! 		}
! 		rdata[lastrdata].next = NULL;
  
! 		(void) XLogInsert(RM_XACT_ID, XLOG_XACT_COMMIT, rdata);
  	}
  
  	/*
--- 986,1071 ----
  		MyProc->inCommit = true;
  
  		SetCurrentTransactionStopTimestamp();
! 
! 		if (nrels == 0 && nmsgs == 0)
  		{
! 			XLogRecData rdata[2];
! 			int			lastrdata = 0;
! 			xl_xact_commit xlrec;
! 			xlrec.xact_time = xactStopTimestamp;
! 			xlrec.nsubxacts = nchildren;
! 			rdata[0].data = (char *) (&xlrec);
! 			rdata[0].len = MinSizeOfXactCommit;
! 			rdata[0].buffer = InvalidBuffer;
! 			/* dump committed child Xids */
! 			if (nchildren > 0)
! 			{
! 				rdata[0].next = &(rdata[1]);
! 				rdata[1].data = (char *) children;
! 				rdata[1].len = nchildren * sizeof(TransactionId);
! 				rdata[1].buffer = InvalidBuffer;
! 				lastrdata = 1;
! 			}
! 			rdata[lastrdata].next = NULL;
! 
! 			(void) XLogInsert(RM_XACT_ID, XLOG_XACT_COMMIT_FAST_PATH, rdata);
! 
  		}
! 		else
  		{
! 			XLogRecData rdata[4];
! 			int			lastrdata = 0;
! 			xl_xact_commit_with_info xlrec;
! 			/*
! 			 * Set flags required for recovery processing of commits.
! 			 */
! 			xlrec.xinfo = 0;
! 			if (RelcacheInitFileInval)
! 				xlrec.xinfo |= XACT_COMPLETION_UPDATE_RELCACHE_FILE;
! 			if (forceSyncCommit)
! 				xlrec.xinfo |= XACT_COMPLETION_FORCE_SYNC_COMMIT;
  
! 			xlrec.dbId = MyDatabaseId;
! 			xlrec.tsId = MyDatabaseTableSpace;
! 
! 			xlrec.xact_time = xactStopTimestamp;
! 			xlrec.nrels = nrels;
! 			xlrec.nsubxacts = nchildren;
! 			xlrec.nmsgs = nmsgs;
! 			rdata[0].data = (char *) (&xlrec);
! 			rdata[0].len = MinSizeOfXactCommit;
! 			rdata[0].buffer = InvalidBuffer;
! 			/* dump rels to delete */
! 			if (nrels > 0)
! 			{
! 				rdata[0].next = &(rdata[1]);
! 				rdata[1].data = (char *) rels;
! 				rdata[1].len = nrels * sizeof(RelFileNode);
! 				rdata[1].buffer = InvalidBuffer;
! 				lastrdata = 1;
! 			}
! 			/* dump committed child Xids */
! 			if (nchildren > 0)
! 			{
! 				rdata[lastrdata].next = &(rdata[2]);
! 				rdata[2].data = (char *) children;
! 				rdata[2].len = nchildren * sizeof(TransactionId);
! 				rdata[2].buffer = InvalidBuffer;
! 				lastrdata = 2;
! 			}
! 			/* dump shared cache invalidation messages */
! 			if (nmsgs > 0)
! 			{
! 				rdata[lastrdata].next = &(rdata[3]);
! 				rdata[3].data = (char *) invalMessages;
! 				rdata[3].len = nmsgs * sizeof(SharedInvalidationMessage);
! 				rdata[3].buffer = InvalidBuffer;
! 				lastrdata = 3;
! 			}
! 			rdata[lastrdata].next = NULL;
! 
! 			(void) XLogInsert(RM_XACT_ID, XLOG_XACT_COMMIT, rdata);
! 		}
  	}
  
  	/*
*************** xactGetCommittedChildren(TransactionId *
*** 4441,4459 ****
   * actions for which the order of execution is critical.
   */
  static void
! xact_redo_commit(xl_xact_commit *xlrec, TransactionId xid, XLogRecPtr lsn)
  {
- 	TransactionId *sub_xids;
- 	SharedInvalidationMessage *inval_msgs;
  	TransactionId max_xid;
  	int			i;
  
! 	/* subxid array follows relfilenodes */
! 	sub_xids = (TransactionId *) &(xlrec->xnodes[xlrec->nrels]);
! 	/* invalidation messages array follows subxids */
! 	inval_msgs = (SharedInvalidationMessage *) &(sub_xids[xlrec->nsubxacts]);
! 
! 	max_xid = TransactionIdLatest(xid, xlrec->nsubxacts, sub_xids);
  
  	/*
  	 * Make sure nextXid is beyond any XID mentioned in the record.
--- 4468,4483 ----
   * actions for which the order of execution is critical.
   */
  static void
! xact_redo_commit(RelFileNode *xnodes, int nrels,
! 					TransactionId *sub_xids, int nsubxacts,
! 					SharedInvalidationMessage *inval_msgs, int nmsgs,
! 					Oid dbId, Oid tsId, uint32 xinfo,
! 					TransactionId xid, XLogRecPtr lsn)
  {
  	TransactionId max_xid;
  	int			i;
  
! 	max_xid = TransactionIdLatest(xid, nsubxacts, sub_xids);
  
  	/*
  	 * Make sure nextXid is beyond any XID mentioned in the record.
*************** xact_redo_commit(xl_xact_commit *xlrec, 
*** 4476,4482 ****
  		/*
  		 * Mark the transaction committed in pg_clog.
  		 */
! 		TransactionIdCommitTree(xid, xlrec->nsubxacts, sub_xids);
  	}
  	else
  	{
--- 4500,4506 ----
  		/*
  		 * Mark the transaction committed in pg_clog.
  		 */
! 		TransactionIdCommitTree(xid, nsubxacts, sub_xids);
  	}
  	else
  	{
*************** xact_redo_commit(xl_xact_commit *xlrec, 
*** 4500,4540 ****
  		 * bits set on changes made by transactions that haven't yet
  		 * recovered. It's unlikely but it's good to be safe.
  		 */
! 		TransactionIdAsyncCommitTree(xid, xlrec->nsubxacts, sub_xids, lsn);
  
  		/*
  		 * We must mark clog before we update the ProcArray.
  		 */
! 		ExpireTreeKnownAssignedTransactionIds(xid, xlrec->nsubxacts, sub_xids, max_xid);
  
  		/*
  		 * Send any cache invalidations attached to the commit. We must
  		 * maintain the same order of invalidation then release locks as
  		 * occurs in CommitTransaction().
  		 */
! 		ProcessCommittedInvalidationMessages(inval_msgs, xlrec->nmsgs,
! 								  XactCompletionRelcacheInitFileInval(xlrec),
! 											 xlrec->dbId, xlrec->tsId);
  
  		/*
  		 * Release locks, if any. We do this for both two phase and normal one
  		 * phase transactions. In effect we are ignoring the prepare phase and
  		 * just going straight to lock release.
  		 */
! 		StandbyReleaseLockTree(xid, xlrec->nsubxacts, sub_xids);
  	}
  
  	/* Make sure files supposed to be dropped are dropped */
! 	for (i = 0; i < xlrec->nrels; i++)
  	{
! 		SMgrRelation srel = smgropen(xlrec->xnodes[i], InvalidBackendId);
  		ForkNumber	fork;
  
  		for (fork = 0; fork <= MAX_FORKNUM; fork++)
  		{
  			if (smgrexists(srel, fork))
  			{
! 				XLogDropRelation(xlrec->xnodes[i], fork);
  				smgrdounlink(srel, fork, true);
  			}
  		}
--- 4524,4564 ----
  		 * bits set on changes made by transactions that haven't yet
  		 * recovered. It's unlikely but it's good to be safe.
  		 */
! 		TransactionIdAsyncCommitTree(xid, nsubxacts, sub_xids, lsn);
  
  		/*
  		 * We must mark clog before we update the ProcArray.
  		 */
! 		ExpireTreeKnownAssignedTransactionIds(xid, nsubxacts, sub_xids, max_xid);
  
  		/*
  		 * Send any cache invalidations attached to the commit. We must
  		 * maintain the same order of invalidation then release locks as
  		 * occurs in CommitTransaction().
  		 */
! 		ProcessCommittedInvalidationMessages(inval_msgs, nmsgs,
! 								  XactCompletionRelcacheInitFileInval(xinfo),
! 											 dbId, tsId);
  
  		/*
  		 * Release locks, if any. We do this for both two phase and normal one
  		 * phase transactions. In effect we are ignoring the prepare phase and
  		 * just going straight to lock release.
  		 */
! 		StandbyReleaseLockTree(xid, nsubxacts, sub_xids);
  	}
  
  	/* Make sure files supposed to be dropped are dropped */
! 	for (i = 0; i < nrels; i++)
  	{
! 		SMgrRelation srel = smgropen(xnodes[i], InvalidBackendId);
  		ForkNumber	fork;
  
  		for (fork = 0; fork <= MAX_FORKNUM; fork++)
  		{
  			if (smgrexists(srel, fork))
  			{
! 				XLogDropRelation(xnodes[i], fork);
  				smgrdounlink(srel, fork, true);
  			}
  		}
*************** xact_redo_commit(xl_xact_commit *xlrec, 
*** 4553,4560 ****
  	 * to reduce that problem window, for any user that requested
  	 * ForceSyncCommit().
  	 */
! 	if (XactCompletionForceSyncCommit(xlrec))
  		XLogFlush(lsn);
  }
  
  /*
--- 4577,4616 ----
  	 * to reduce that problem window, for any user that requested
  	 * ForceSyncCommit().
  	 */
! 	if (XactCompletionForceSyncCommit(xinfo))
  		XLogFlush(lsn);
+ 
+ }
+ /*
+  * utility function to call xact_redo_commit with parameters found in xlrec
+  */
+ static void
+ xact_redo_commit_with_info(xl_xact_commit_with_info *xlrec,
+ 							TransactionId xid, XLogRecPtr lsn)
+ {
+ 	TransactionId *sub_xids;
+ 	SharedInvalidationMessage *inval_msgs;
+ 
+ 	/* subxid array follows relfilenodes */
+ 	sub_xids = (TransactionId *) &(xlrec->xnodes[xlrec->nrels]);
+ 	/* invalidation messages array follows subxids */
+ 	inval_msgs = (SharedInvalidationMessage *) &(sub_xids[xlrec->nsubxacts]);
+ 
+ 	xact_redo_commit(xlrec->xnodes, xlrec->nrels, sub_xids, xlrec->nsubxacts,
+ 						inval_msgs, xlrec->nmsgs, xlrec->dbId, xlrec->tsId,
+ 						xlrec->xinfo, xid, lsn);
+ }
+ 
+ /*
+  * utility function to call xact_redo_commit with proper parameters:
+  * parameters not available in xl_xact_commit are defaulted to 0.
+  */
+ static void
+ xact_redo_commit_without_info(xl_xact_commit *xlrec,
+ 							TransactionId xid, XLogRecPtr lsn)
+ {
+ 	xact_redo_commit(NULL, 0, xlrec->children, xlrec->nsubxacts,
+ 						NULL, 0, InvalidOid, InvalidOid, 0, xid, lsn);
  }
  
  /*
*************** xact_redo(XLogRecPtr lsn, XLogRecord *re
*** 4655,4665 ****
  	/* Backup blocks are not used in xact records */
  	Assert(!(record->xl_info & XLR_BKP_BLOCK_MASK));
  
! 	if (info == XLOG_XACT_COMMIT)
  	{
  		xl_xact_commit *xlrec = (xl_xact_commit *) XLogRecGetData(record);
  
! 		xact_redo_commit(xlrec, record->xl_xid, lsn);
  	}
  	else if (info == XLOG_XACT_ABORT)
  	{
--- 4711,4727 ----
  	/* Backup blocks are not used in xact records */
  	Assert(!(record->xl_info & XLR_BKP_BLOCK_MASK));
  
! 	if (info == XLOG_XACT_COMMIT_FAST_PATH)
  	{
  		xl_xact_commit *xlrec = (xl_xact_commit *) XLogRecGetData(record);
  
! 		xact_redo_commit_without_info(xlrec, record->xl_xid, lsn);
! 	}
! 	else if (info == XLOG_XACT_COMMIT)
! 	{
! 		xl_xact_commit_with_info *xlrec = (xl_xact_commit_with_info *) XLogRecGetData(record);
! 
! 		xact_redo_commit_with_info(xlrec, record->xl_xid, lsn);
  	}
  	else if (info == XLOG_XACT_ABORT)
  	{
*************** xact_redo(XLogRecPtr lsn, XLogRecord *re
*** 4677,4683 ****
  	{
  		xl_xact_commit_prepared *xlrec = (xl_xact_commit_prepared *) XLogRecGetData(record);
  
! 		xact_redo_commit(&xlrec->crec, xlrec->xid, lsn);
  		RemoveTwoPhaseFile(xlrec->xid, false);
  	}
  	else if (info == XLOG_XACT_ABORT_PREPARED)
--- 4739,4745 ----
  	{
  		xl_xact_commit_prepared *xlrec = (xl_xact_commit_prepared *) XLogRecGetData(record);
  
! 		xact_redo_commit_with_info(&xlrec->crec, xlrec->xid, lsn);
  		RemoveTwoPhaseFile(xlrec->xid, false);
  	}
  	else if (info == XLOG_XACT_ABORT_PREPARED)
*************** xact_redo(XLogRecPtr lsn, XLogRecord *re
*** 4700,4706 ****
  }
  
  static void
! xact_desc_commit(StringInfo buf, xl_xact_commit *xlrec)
  {
  	int			i;
  	TransactionId *xacts;
--- 4762,4768 ----
  }
  
  static void
! xact_desc_commit_with_info(StringInfo buf, xl_xact_commit_with_info *xlrec)
  {
  	int			i;
  	TransactionId *xacts;
*************** xact_desc_commit(StringInfo buf, xl_xact
*** 4732,4738 ****
  
  		msgs = (SharedInvalidationMessage *) &xacts[xlrec->nsubxacts];
  
! 		if (XactCompletionRelcacheInitFileInval(xlrec))
  			appendStringInfo(buf, "; relcache init file inval dbid %u tsid %u",
  							 xlrec->dbId, xlrec->tsId);
  
--- 4794,4800 ----
  
  		msgs = (SharedInvalidationMessage *) &xacts[xlrec->nsubxacts];
  
! 		if (XactCompletionRelcacheInitFileInval(xlrec->xinfo))
  			appendStringInfo(buf, "; relcache init file inval dbid %u tsid %u",
  							 xlrec->dbId, xlrec->tsId);
  
*************** xact_desc_commit(StringInfo buf, xl_xact
*** 4759,4764 ****
--- 4821,4841 ----
  }
  
  static void
+ xact_desc_commit(StringInfo buf, xl_xact_commit *xlrec)
+ {
+ 	int			i;
+ 
+ 	appendStringInfoString(buf, timestamptz_to_str(xlrec->xact_time));
+ 
+ 	if (xlrec->nsubxacts > 0)
+ 	{
+ 		appendStringInfo(buf, "; subxacts:");
+ 		for (i = 0; i < xlrec->nsubxacts; i++)
+ 			appendStringInfo(buf, " %u", xlrec->children[i]);
+ 	}
+ }
+ 
+ static void
  xact_desc_abort(StringInfo buf, xl_xact_abort *xlrec)
  {
  	int			i;
*************** xact_desc(StringInfo buf, uint8 xl_info,
*** 4802,4814 ****
  {
  	uint8		info = xl_info & ~XLR_INFO_MASK;
  
! 	if (info == XLOG_XACT_COMMIT)
  	{
  		xl_xact_commit *xlrec = (xl_xact_commit *) rec;
  
  		appendStringInfo(buf, "commit: ");
  		xact_desc_commit(buf, xlrec);
  	}
  	else if (info == XLOG_XACT_ABORT)
  	{
  		xl_xact_abort *xlrec = (xl_xact_abort *) rec;
--- 4879,4898 ----
  {
  	uint8		info = xl_info & ~XLR_INFO_MASK;
  
! 	if (info == XLOG_XACT_COMMIT_FAST_PATH)
  	{
  		xl_xact_commit *xlrec = (xl_xact_commit *) rec;
  
  		appendStringInfo(buf, "commit: ");
  		xact_desc_commit(buf, xlrec);
  	}
+ 	else if (info == XLOG_XACT_COMMIT)
+ 	{
+ 		xl_xact_commit_with_info *xlrec = (xl_xact_commit_with_info *) rec;
+ 
+ 		appendStringInfo(buf, "commit: ");
+ 		xact_desc_commit_with_info(buf, xlrec);
+ 	}
  	else if (info == XLOG_XACT_ABORT)
  	{
  		xl_xact_abort *xlrec = (xl_xact_abort *) rec;
*************** xact_desc(StringInfo buf, uint8 xl_info,
*** 4825,4831 ****
  		xl_xact_commit_prepared *xlrec = (xl_xact_commit_prepared *) rec;
  
  		appendStringInfo(buf, "commit prepared %u: ", xlrec->xid);
! 		xact_desc_commit(buf, &xlrec->crec);
  	}
  	else if (info == XLOG_XACT_ABORT_PREPARED)
  	{
--- 4909,4915 ----
  		xl_xact_commit_prepared *xlrec = (xl_xact_commit_prepared *) rec;
  
  		appendStringInfo(buf, "commit prepared %u: ", xlrec->xid);
! 		xact_desc_commit_with_info(buf, &xlrec->crec);
  	}
  	else if (info == XLOG_XACT_ABORT_PREPARED)
  	{
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
new file mode 100644
index aa0b029..c2813c0
*** a/src/backend/access/transam/xlog.c
--- b/src/backend/access/transam/xlog.c
*************** recoveryStopsHere(XLogRecord *record, bo
*** 5594,5606 ****
  	if (record->xl_rmid != RM_XACT_ID && record->xl_rmid != RM_XLOG_ID)
  		return false;
  	record_info = record->xl_info & ~XLR_INFO_MASK;
! 	if (record->xl_rmid == RM_XACT_ID && record_info == XLOG_XACT_COMMIT)
  	{
  		xl_xact_commit *recordXactCommitData;
  
  		recordXactCommitData = (xl_xact_commit *) XLogRecGetData(record);
  		recordXtime = recordXactCommitData->xact_time;
  	}
  	else if (record->xl_rmid == RM_XACT_ID && record_info == XLOG_XACT_ABORT)
  	{
  		xl_xact_abort *recordXactAbortData;
--- 5594,5613 ----
  	if (record->xl_rmid != RM_XACT_ID && record->xl_rmid != RM_XLOG_ID)
  		return false;
  	record_info = record->xl_info & ~XLR_INFO_MASK;
! 	if (record->xl_rmid == RM_XACT_ID && record_info == XLOG_XACT_COMMIT_FAST_PATH)
  	{
  		xl_xact_commit *recordXactCommitData;
  
  		recordXactCommitData = (xl_xact_commit *) XLogRecGetData(record);
  		recordXtime = recordXactCommitData->xact_time;
  	}
+ 	else if (record->xl_rmid == RM_XACT_ID && record_info == XLOG_XACT_COMMIT)
+ 	{
+ 		xl_xact_commit_with_info *recordXactCommitData;
+ 
+ 		recordXactCommitData = (xl_xact_commit_with_info *) XLogRecGetData(record);
+ 		recordXtime = recordXactCommitData->xact_time;
+ 	}
  	else if (record->xl_rmid == RM_XACT_ID && record_info == XLOG_XACT_ABORT)
  	{
  		xl_xact_abort *recordXactAbortData;
*************** recoveryStopsHere(XLogRecord *record, bo
*** 5681,5687 ****
  		recoveryStopTime = recordXtime;
  		recoveryStopAfter = *includeThis;
  
! 		if (record_info == XLOG_XACT_COMMIT)
  		{
  			if (recoveryStopAfter)
  				ereport(LOG,
--- 5688,5694 ----
  		recoveryStopTime = recordXtime;
  		recoveryStopAfter = *includeThis;
  
! 		if (record_info == XLOG_XACT_COMMIT || record_info == XLOG_XACT_COMMIT_FAST_PATH)
  		{
  			if (recoveryStopAfter)
  				ereport(LOG,
diff --git a/src/include/access/xact.h b/src/include/access/xact.h
new file mode 100644
index cb440d4..f03463a
*** a/src/include/access/xact.h
--- b/src/include/access/xact.h
*************** typedef void (*SubXactCallback) (SubXact
*** 106,111 ****
--- 106,112 ----
  #define XLOG_XACT_COMMIT_PREPARED	0x30
  #define XLOG_XACT_ABORT_PREPARED	0x40
  #define XLOG_XACT_ASSIGNMENT		0x50
+ #define XLOG_XACT_COMMIT_FAST_PATH  0x60
  
  typedef struct xl_xact_assignment
  {
*************** typedef struct xl_xact_assignment
*** 119,124 ****
--- 120,136 ----
  typedef struct xl_xact_commit
  {
  	TimestampTz xact_time;		/* time of commit */
+ 	int			nsubxacts;		/* number of subtransaction XIDs */
+ 	/* ARRAY OF COMMITTED SUBTRANSACTION XIDs FOLLOWS */
+ 	TransactionId children[1];		/* VARIABLE LENGTH ARRAY */
+ } xl_xact_commit;
+ 
+ #define MinSizeOfXactCommit offsetof(xl_xact_commit, children)
+ 
+ 
+ typedef struct xl_xact_commit_with_info
+ {
+ 	TimestampTz xact_time;		/* time of commit */
  	uint32		xinfo;			/* info flags */
  	int			nrels;			/* number of RelFileNodes */
  	int			nsubxacts;		/* number of subtransaction XIDs */
*************** typedef struct xl_xact_commit
*** 129,137 ****
  	RelFileNode xnodes[1];		/* VARIABLE LENGTH ARRAY */
  	/* ARRAY OF COMMITTED SUBTRANSACTION XIDs FOLLOWS */
  	/* ARRAY OF SHARED INVALIDATION MESSAGES FOLLOWS */
! } xl_xact_commit;
  
! #define MinSizeOfXactCommit offsetof(xl_xact_commit, xnodes)
  
  /*
   * These flags are set in the xinfo fields of WAL commit records,
--- 141,149 ----
  	RelFileNode xnodes[1];		/* VARIABLE LENGTH ARRAY */
  	/* ARRAY OF COMMITTED SUBTRANSACTION XIDs FOLLOWS */
  	/* ARRAY OF SHARED INVALIDATION MESSAGES FOLLOWS */
! } xl_xact_commit_with_info;
  
! #define MinSizeOfXactCommitWithInfo offsetof(xl_xact_commit_with_info, xnodes)
  
  /*
   * These flags are set in the xinfo fields of WAL commit records,
*************** typedef struct xl_xact_commit
*** 145,152 ****
  #define XACT_COMPLETION_FORCE_SYNC_COMMIT		0x02
  
  /* Access macros for above flags */
! #define XactCompletionRelcacheInitFileInval(xlrec)	((xlrec)->xinfo & XACT_COMPLETION_UPDATE_RELCACHE_FILE)
! #define XactCompletionForceSyncCommit(xlrec)		((xlrec)->xinfo & XACT_COMPLETION_FORCE_SYNC_COMMIT)
  
  typedef struct xl_xact_abort
  {
--- 157,164 ----
  #define XACT_COMPLETION_FORCE_SYNC_COMMIT		0x02
  
  /* Access macros for above flags */
! #define XactCompletionRelcacheInitFileInval(xinfo)	(xinfo & XACT_COMPLETION_UPDATE_RELCACHE_FILE)
! #define XactCompletionForceSyncCommit(xinfo)		(xinfo & XACT_COMPLETION_FORCE_SYNC_COMMIT)
  
  typedef struct xl_xact_abort
  {
*************** typedef struct xl_xact_abort
*** 172,178 ****
  typedef struct xl_xact_commit_prepared
  {
  	TransactionId xid;			/* XID of prepared xact */
! 	xl_xact_commit crec;		/* COMMIT record */
  	/* MORE DATA FOLLOWS AT END OF STRUCT */
  } xl_xact_commit_prepared;
  
--- 184,190 ----
  typedef struct xl_xact_commit_prepared
  {
  	TransactionId xid;			/* XID of prepared xact */
! 	xl_xact_commit_with_info crec;		/* COMMIT record */
  	/* MORE DATA FOLLOWS AT END OF STRUCT */
  } xl_xact_commit_prepared;
  
#22Simon Riggs
simon@2ndQuadrant.com
In reply to: Leonardo Francalanci (#21)
Re: use less space in xl_xact_commit patch

On Thu, Jun 16, 2011 at 2:00 PM, Leonardo Francalanci <m_lists@yahoo.it> wrote:

With regards to the naming, I think it would be better if we  kept
XLOG_XACT_COMMIT record exactly as it is now, and make the  second
record an entirely new record called XLOG_XACT_COMMIT_FASTPATH.  That
way we retain backwards compatibility.

If you'd like to rework  like that please, otherwise I can take it from
here if you'd  like.

I think I did it; while doing it, I think I've found a bug: I didn't update
"recoveryStopsHere". Please double check that, as I really don't
know what I'm doing there...
Should I also change the struct name from xl_xact_commit to
xl_xact_commit_fast_path?

Yes please.

How can I test it with "weird" stuff as subtransactions,  shared
cache invalidation messages...?

make installcheck should  cover those.

Ok, all tests passed.

Even better.

Will review, thanks.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

#23Robert Haas
robertmhaas@gmail.com
In reply to: Simon Riggs (#20)
Re: use less space in xl_xact_commit patch

On Thu, Jun 16, 2011 at 7:25 AM, Simon Riggs <simon@2ndquadrant.com> wrote:

With regards to the naming, I think it would be better if we kept
XLOG_XACT_COMMIT record exactly as it is now, and make the second
record an entirely new record called XLOG_XACT_COMMIT_FASTPATH. That
way we retain backwards compatibility.

I liked your previous suggestion of commit and commit-with-info
better. There's nothing particularly fast about this; it's just less
info. So to speak.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#24Robert Haas
robertmhaas@gmail.com
In reply to: Robert Haas (#23)
Re: use less space in xl_xact_commit patch

On Thu, Jun 16, 2011 at 9:22 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Thu, Jun 16, 2011 at 7:25 AM, Simon Riggs <simon@2ndquadrant.com> wrote:

With regards to the naming, I think it would be better if we kept
XLOG_XACT_COMMIT record exactly as it is now, and make the second
record an entirely new record called XLOG_XACT_COMMIT_FASTPATH. That
way we retain backwards compatibility.

I liked your previous suggestion of commit and commit-with-info
better.  There's nothing particularly fast about this; it's just less
info.  So to speak.

At any rate we shouldn't name the stuct one way (xl_xact_commit and
xl_xact_commit_with_info) and the constants the other way
(XLOG_XACT_COMMIT and XLOG_XACT_COMMIT_FASTPATH). They should match.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#25Simon Riggs
simon@2ndQuadrant.com
In reply to: Robert Haas (#23)
Re: use less space in xl_xact_commit patch

On Thu, Jun 16, 2011 at 2:22 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On Thu, Jun 16, 2011 at 7:25 AM, Simon Riggs <simon@2ndquadrant.com> wrote:

With regards to the naming, I think it would be better if we kept
XLOG_XACT_COMMIT record exactly as it is now, and make the second
record an entirely new record called XLOG_XACT_COMMIT_FASTPATH. That
way we retain backwards compatibility.

I liked your previous suggestion of commit and commit-with-info
better.  There's nothing particularly fast about this; it's just less
info.  So to speak.

The important thing is that we retain backwards compatibility with
current XLOG_XACT_COMMIT. I'm not worried what we call the other one.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

#26Leonardo Francalanci
m_lists@yahoo.it
In reply to: Simon Riggs (#25)
Re: use less space in xl_xact_commit patch

The important thing is that we retain backwards compatibility with
current XLOG_XACT_COMMIT. I'm not worried what we call the other one.

Ok, let me see if I got it right:

#define XLOG_XACT_COMMIT 0x00

should become:

#define XLOG_XACT_COMMIT_WITH_INFO 0x00

and I'll add a

#define XLOG_XACT_COMMIT 0x60

Than I'll leave 2 structs, "xl_xact_commit_with_info" and
"xl_xact_commit".

Leonardo

#27Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#23)
Re: use less space in xl_xact_commit patch

Robert Haas <robertmhaas@gmail.com> writes:

On Thu, Jun 16, 2011 at 7:25 AM, Simon Riggs <simon@2ndquadrant.com> wrote:

With regards to the naming, I think it would be better if we kept
XLOG_XACT_COMMIT record exactly as it is now, and make the second
record an entirely new record called XLOG_XACT_COMMIT_FASTPATH. That
way we retain backwards compatibility.

I liked your previous suggestion of commit and commit-with-info
better. There's nothing particularly fast about this; it's just less
info. So to speak.

Yes. There is no need to preserve backwards compatibility here, so
let's just design the records in a way that makes sense on its own.

regards, tom lane

#28Simon Riggs
simon@2ndQuadrant.com
In reply to: Tom Lane (#27)
Re: use less space in xl_xact_commit patch

On Thu, Jun 16, 2011 at 3:14 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Thu, Jun 16, 2011 at 7:25 AM, Simon Riggs <simon@2ndquadrant.com>
wrote:

With regards to the naming, I think it would be better if we kept
XLOG_XACT_COMMIT record exactly as it is now, and make the second
record an entirely new record called XLOG_XACT_COMMIT_FASTPATH. That
way we retain backwards compatibility.

I liked your previous suggestion of commit and commit-with-info
better.  There's nothing particularly fast about this; it's just less
info.  So to speak.

Yes.  There is no need to preserve backwards compatibility here, so
let's just design the records in a way that makes sense on its own.

The only difference I'm proposing is the naming. It was foolish of me
to propose that the data structure that is exactly the same should
have a different name, yet the new structure should have the same name
as the previous version. That will lead to confusion to no benefit. My
second suggestion makes sense on its own, for no other reason.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

#29Alvaro Herrera
alvherre@commandprompt.com
In reply to: Leonardo Francalanci (#21)
Re: use less space in xl_xact_commit patch

Excerpts from Leonardo Francalanci's message of jue jun 16 09:00:15 -0400 2011:

Should I also change the struct name from xl_xact_commit to
xl_xact_commit_fast_path?

Yes, please.

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

#30Robert Haas
robertmhaas@gmail.com
In reply to: Simon Riggs (#28)
Re: use less space in xl_xact_commit patch

On Thu, Jun 16, 2011 at 12:12 PM, Simon Riggs <simon@2ndquadrant.com> wrote:

On Thu, Jun 16, 2011 at 3:14 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Thu, Jun 16, 2011 at 7:25 AM, Simon Riggs <simon@2ndquadrant.com>
wrote:

With regards to the naming, I think it would be better if we kept
XLOG_XACT_COMMIT record exactly as it is now, and make the second
record an entirely new record called XLOG_XACT_COMMIT_FASTPATH. That
way we retain backwards compatibility.

I liked your previous suggestion of commit and commit-with-info
better.  There's nothing particularly fast about this; it's just less
info.  So to speak.

Yes.  There is no need to preserve backwards compatibility here, so
let's just design the records in a way that makes sense on its own.

The only difference I'm proposing is the naming. It was foolish of me
to propose that the data structure that is exactly the same should
have a different name, yet the new structure should have the same name
as the previous version. That will lead to confusion to no benefit. My
second suggestion makes sense on its own, for no other reason.

That's a reasonable point, but I still don't really like the name
"fastpath", because it's not faster, and it's not a path. It's just
smaller. How about xl_xact_commit_simple or xl_xact_commit_compact or
something like that?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#31Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#30)
Re: use less space in xl_xact_commit patch

Robert Haas <robertmhaas@gmail.com> writes:

That's a reasonable point, but I still don't really like the name
"fastpath", because it's not faster, and it's not a path. It's just
smaller. How about xl_xact_commit_simple or xl_xact_commit_compact or
something like that?

<bikeshed>
xl_xact_commit_short ?
</bikeshed>

"_simple" would probably be my next choice.

regards, tom lane

#32Simon Riggs
simon@2ndQuadrant.com
In reply to: Robert Haas (#30)
Re: use less space in xl_xact_commit patch

On Thu, Jun 16, 2011 at 5:34 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On Thu, Jun 16, 2011 at 12:12 PM, Simon Riggs <simon@2ndquadrant.com> wrote:

On Thu, Jun 16, 2011 at 3:14 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Thu, Jun 16, 2011 at 7:25 AM, Simon Riggs <simon@2ndquadrant.com>
wrote:

With regards to the naming, I think it would be better if we kept
XLOG_XACT_COMMIT record exactly as it is now, and make the second
record an entirely new record called XLOG_XACT_COMMIT_FASTPATH. That
way we retain backwards compatibility.

I liked your previous suggestion of commit and commit-with-info
better.  There's nothing particularly fast about this; it's just less
info.  So to speak.

Yes.  There is no need to preserve backwards compatibility here, so
let's just design the records in a way that makes sense on its own.

The only difference I'm proposing is the naming. It was foolish of me
to propose that the data structure that is exactly the same should
have a different name, yet the new structure should have the same name
as the previous version. That will lead to confusion to no benefit. My
second suggestion makes sense on its own, for no other reason.

That's a reasonable point, but I still don't really like the name
"fastpath", because it's not faster, and it's not a path.  It's just
smaller.  How about xl_xact_commit_simple or xl_xact_commit_compact or
something like that?

Sure, anything like that is fine for me.

Rather annoyed at my email client because I lost my first email on
this topic, then had to retype it all from memory. The second copy
omitted things like "or similar name, that's not important" which
would have avoided the last couple of mails. :-(

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services