On 12/03/2014 04:54 PM, Alvaro Herrera wrote:
ir commit timestamp directly as they commit,
or an external transaction c

Sorry, I'm late to the party, but here's some random comments on this after a quick review:

* The whole concept of a node ID seems to be undocumented, and unused. No-one calls CommitTsSetDefaultNodeId(). The whole SET_TS record type is dead code too, when a non-default node_id is not set. Please remove, or explain how all that's supposed to work.

* COMMIT_TS_SETTS. "SETTS" sounds like a typo (like Robert complained about "committs" earlier). Rename to "COMMIT_TS_SET_TIMESTAMP", or just "COMMIT_TS_SET".

* committsdesc.c -> commit_ts_desc.c, to be consistent with "commit_ts.c"

* In commit_ts_desc:

+       nsubxids = ((XLogRecGetDataLen(record) - SizeOfCommitTsSet) /
+                   sizeof(TransactionId));
+       if (nsubxids > 0)
+       {
+           int     i;
+           TransactionId *subxids;
+
+           subxids = palloc(sizeof(TransactionId) * nsubxids);
+           memcpy(subxids,
+                  XLogRecGetData(record) + SizeOfCommitTsSet,
+                  sizeof(TransactionId) * nsubxids);
+           for (i = 0; i < nsubxids; i++)
+               appendStringInfo(buf, ", %u", subxids[i]);
+           pfree(subxids);
+       }

There's no need to memcpy() here. The subxid array in the WAL record is aligned.

* This seems to be a completely unrelated change.

--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -1438,7 +1438,7 @@ parse_hba_auth_opt(char *name, char *val, HbaLine 
*hbaline, int line_num)
                ereport(LOG,
                        (errcode(ERRCODE_CONFIG_FILE_ERROR),
                         errmsg("client certificates can only be checked if a root 
certificate store is available"),
-                        errhint("Make sure the configuration parameter 
\"ssl_ca_file\" is set."),
+                        errhint("Make sure the configuration parameter \"%s\" is set.", 
"ssl_ca_file"),
                         errcontext("line %d of configuration file \"%s\"",
                                    line_num, HbaFileName)));
                return false;


* What is the definition of "latest commit", in pg_last_committed_xact()? It doesn't necessarily line up with the order of commit WAL records, nor with the order the proc array is updated (i.e. the order that the effects become visible to other backends). It seems confusing to add yet another notion of commit order. Perhaps that's the best we can do, but it needs to be documented.

- Heikki



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to