pgsql: Keep track of transaction commit timestamps

On 12/03/2014 03:54 PM, Alvaro Herrera wrote:

Keep track of transaction commit timestamps

Transactions can now set their commit timestamp directly as they commit,
or an external transaction commit timestamp can be fed from an outside
system using the new function TransactionTreeSetCommitTsData(). This
data is crash-safe, and truncated at Xid freeze point, same as pg_clog.

This module is disabled by default because it causes a performance hit,
but can be enabled in postgresql.conf requiring only a server restart.

A new test in src/test/modules is included.

Catalog version bumped due to the new subdirectory within PGDATA and a
couple of new SQL functions.

Authors: Álvaro Herrera and Petr Jelínek

Reviewed to varying degrees by Michael Paquier, Andres Freund, Robert
Haas, Amit Kapila, Fujii Masao, Jaime Casanova, Simon Riggs, Steven
Singer, Peter Eisentraut

this broke the docs build:

http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=guaibasaurus&dt=2014-12-03%2016%3A17%3A01

Stefan

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

Stefan Kaltenbrunner wrote:

On 12/03/2014 03:54 PM, Alvaro Herrera wrote:

Keep track of transaction commit timestamps

this broke the docs build:

http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=guaibasaurus&dt=2014-12-03%2016%3A17%3A01

Woops, thanks. Fixed.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

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

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

On 04/12/14 10:42, Heikki Linnakangas wrote:

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.

That's API meant to be used by extensions, maybe it would be preferable
if there was SQL API exposed also?

(It might also make more sense once replication identifiers are submitted.)

* 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.

It's updated on CommitTransaction (well RecordTransactionCommit but
that's only called from CommitTransaction) time so the definition of
latest commit is last CommitTransaction call.

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

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

On 12/04/2014 01:16 PM, Petr Jelinek wrote:

On 04/12/14 10:42, Heikki Linnakangas wrote:

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.

That's API meant to be used by extensions, maybe it would be preferable
if there was SQL API exposed also?

(It might also make more sense once replication identifiers are submitted.)

Maybe. Hard to tell without any documentation or comments on how it's
supposed to work. In unacceptable in its current state.

I would prefer to remove it now, and add it back later together with the
code that actually uses it. I don't like having unused internal
functions and APIs sitting the source tree in the hope that someone will
find them useful in the future. It's more likely that it will bitrot, or
not actually work as intended, when someone later tries to use it.

What would the SQL API look like, and what would it be used for?

* 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.

It's updated on CommitTransaction (well RecordTransactionCommit but
that's only called from CommitTransaction) time so the definition of
latest commit is last CommitTransaction call.

Right. It was a rhetorical question, actually. What I meant is that that
needs to be documented, at least. Or changed so that it matches one of
the other definitions of commit order, and then 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

On 04/12/14 12:26, Heikki Linnakangas wrote:

On 12/04/2014 01:16 PM, Petr Jelinek wrote:

On 04/12/14 10:42, Heikki Linnakangas wrote:

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.

That's API meant to be used by extensions, maybe it would be preferable
if there was SQL API exposed also?

(It might also make more sense once replication identifiers are
submitted.)

Maybe. Hard to tell without any documentation or comments on how it's
supposed to work. In unacceptable in its current state.

I would prefer to remove it now, and add it back later together with the
code that actually uses it. I don't like having unused internal
functions and APIs sitting the source tree in the hope that someone will
find them useful in the future. It's more likely that it will bitrot, or
not actually work as intended, when someone later tries to use it.

The thing is I already have extension for 9.4 where I would use this API
for conflict detection if it was available so it's not about hoping for
somebody to find it useful. There was discussion about this during the
review process because the objection was raised already then.

What would the SQL API look like, and what would it be used for?

It would probably mirror the C one, from my POV it's not needed but
maybe some replication solution would prefer to use this without having
to write C components...

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

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

On 12/04/2014 01:47 PM, Petr Jelinek wrote:

On 04/12/14 12:26, Heikki Linnakangas wrote:

On 12/04/2014 01:16 PM, Petr Jelinek wrote:

On 04/12/14 10:42, Heikki Linnakangas wrote:

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.

That's API meant to be used by extensions, maybe it would be preferable
if there was SQL API exposed also?

(It might also make more sense once replication identifiers are
submitted.)

Maybe. Hard to tell without any documentation or comments on how it's
supposed to work. In unacceptable in its current state.

I would prefer to remove it now, and add it back later together with the
code that actually uses it. I don't like having unused internal
functions and APIs sitting the source tree in the hope that someone will
find them useful in the future. It's more likely that it will bitrot, or
not actually work as intended, when someone later tries to use it.

The thing is I already have extension for 9.4 where I would use this API
for conflict detection if it was available so it's not about hoping for
somebody to find it useful. There was discussion about this during the
review process because the objection was raised already then.

Yeah, it was raised. I don't think it was really addressed. There was
lengthy discussion on whether to include LSN, node id, and/or some other
information, but there was no discussion on:

* What is a node ID?
* How is it used?
* Who assigns it?

etc.

None of those things are explained in the documentation nor code comments.

The node ID sounds suspiciously like the replication identifiers that
have been proposed a couple of times. I don't remember if I like
replication identifiers or not, but I'd rather discuss that issue
explicitly and separately. I don't want the concept of a
replication/node ID to fly under the radar like this.

What would the SQL API look like, and what would it be used for?

It would probably mirror the C one, from my POV it's not needed but
maybe some replication solution would prefer to use this without having
to write C components...

I can't comment on that, because without any documentation, I don't even
know what the C API is.

BTW, why is it OK that the node-ID of a commit is logged as a separate
WAL record, after the commit record? That means that it's possible that
a transaction commits, but you crash just before writing the SETTS
record, so that after replay the transaction appears committed but with
the default node ID.

(Maybe that's OK given the intended use case, but it's hard to tell
without any documentation. See a pattern here? ;-) )

- Heikki

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

Heikki Linnakangas <hlinnakangas@vmware.com> writes:

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:

Also this commit breaks initdb of `make check' for me:

creating template1 database in /home/ash/build/postgresql/HEAD/src/test/regress/./tmp_check/data/base/1 ... TRAP: FailedAssertion("!(((xmax) >= ((TransactionId) 3)))", File: "/home/ash/src/postgresql/src/backend/storage/ipc/procarray.c", Line: 1414)
Aborted (core dumped)
child process exited with exit code 134

--
Alex

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

Alex Shulgin wrote:

Also this commit breaks initdb of `make check' for me:

creating template1 database in /home/ash/build/postgresql/HEAD/src/test/regress/./tmp_check/data/base/1 ... TRAP: FailedAssertion("!(((xmax) >= ((TransactionId) 3)))", File: "/home/ash/src/postgresql/src/backend/storage/ipc/procarray.c", Line: 1414)
Aborted (core dumped)
child process exited with exit code 134

Uh, that's odd. Can you please get a stack trace? Do you have unusual
settings or a patched build?

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Alex Shulgin wrote:

Also this commit breaks initdb of `make check' for me:

creating template1 database in
/home/ash/build/postgresql/HEAD/src/test/regress/./tmp_check/data/base/1
... TRAP: FailedAssertion("!(((xmax) >= ((TransactionId) 3)))",
File:
"/home/ash/src/postgresql/src/backend/storage/ipc/procarray.c",
Line: 1414)
Aborted (core dumped)
child process exited with exit code 134

Uh, that's odd. Can you please get a stack trace? Do you have unusual
settings or a patched build?

Not really, and I would mention that. Will get a trace.

--
Alex

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

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Uh, that's odd. Can you please get a stack trace? Do you have unusual
settings or a patched build?

Is there a way to pause the bootstrap process so I can attach gdb to it?

--
Alex

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

On 12/04/2014 10:50 PM, Alex Shulgin wrote:

Is there a way to pause the bootstrap process so I can attach gdb to it?

With a newer gdb, you can instead tell gdb to follow all forks. I wrote
some notes on it recently.

http://blog.2ndquadrant.com/processes-breakpoints-watchpoints-postgresql/

I've found it really handy when debugging crashes in regression tests.

However, it's often simpler to just:

ulimit -c unlimited
make check

(or whatever your crashing test is) then look at the core files that are
produced.

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

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

Craig Ringer <craig@2ndquadrant.com> writes:

On 12/04/2014 10:50 PM, Alex Shulgin wrote:

Is there a way to pause the bootstrap process so I can attach gdb to it?

With a newer gdb, you can instead tell gdb to follow all forks. I wrote
some notes on it recently.

http://blog.2ndquadrant.com/processes-breakpoints-watchpoints-postgresql/

I've found it really handy when debugging crashes in regression tests.

However, it's often simpler to just:

ulimit -c unlimited
make check

(or whatever your crashing test is) then look at the core files that are
produced.

Good one, it didn't occur to me that assert makes core files.

Figured it out with a pg_usleep in bootstrap.c anyway. Does this look sane?

DEBUG: inserting column 6 value "0"
DEBUG: inserted -> 0
DEBUG: inserting column 7 value "varchar_transform"
TRAP: FailedAssertion("!(((xmax) >= ((TransactionId) 3)))", File: "/home/ash/src/postgresql/src/backend/storage/ipc/procarray.c", Line: 1414)

Program received signal SIGABRT, Aborted.
0x00007f2757128d27 in __GI_raise (sig=sig@entry=6)
at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
56 ../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0 0x00007f2757128d27 in __GI_raise (sig=sig@entry=6)
at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1 0x00007f275712a418 in __GI_abort () at abort.c:89
#2 0x00000000009088b2 in ExceptionalCondition (
conditionName=0xac0710 "!(((xmax) >= ((TransactionId) 3)))",
errorType=0xac01d8 "FailedAssertion",
fileName=0xac0178 "/home/ash/src/postgresql/src/backend/storage/ipc/procarray.c", lineNumber=1414)
at /home/ash/src/postgresql/src/backend/utils/error/assert.c:54
#3 0x000000000079e125 in GetSnapshotData (
snapshot=0xdb2d60 <CatalogSnapshotData>)
at /home/ash/src/postgresql/src/backend/storage/ipc/procarray.c:1414
#4 0x000000000094e02d in GetNonHistoricCatalogSnapshot (relid=1255)
at /home/ash/src/postgresql/src/backend/utils/time/snapmgr.c:298
#5 0x000000000094dfdd in GetCatalogSnapshot (relid=1255)
at /home/ash/src/postgresql/src/backend/utils/time/snapmgr.c:272
#6 0x00000000004c8f0d in systable_beginscan (heapRelation=0x1d0e5c0,
indexId=2691, indexOK=1 '\001', snapshot=0x0, nkeys=1, key=0x7fff201bbc40)
at /home/ash/src/postgresql/src/backend/access/index/genam.c:275
#7 0x0000000000885070 in regprocin (fcinfo=0x7fff201bbce0)
at /home/ash/src/postgresql/src/backend/utils/adt/regproc.c:106
#8 0x0000000000914fe7 in InputFunctionCall (flinfo=0x7fff201bc0c0,
str=0x1d349b8 "varchar_transform", typioparam=24, typmod=-1)
---Type <return> to continue, or q <return> to quit---
at /home/ash/src/postgresql/src/backend/utils/fmgr/fmgr.c:1914
#9 0x000000000091533e in OidInputFunctionCall (functionId=44,
str=0x1d349b8 "varchar_transform", typioparam=24, typmod=-1)
at /home/ash/src/postgresql/src/backend/utils/fmgr/fmgr.c:2045
#10 0x000000000052af91 in InsertOneValue (value=0x1d349b8 "varchar_transform",
i=7) at /home/ash/src/postgresql/src/backend/bootstrap/bootstrap.c:840
#11 0x0000000000527409 in boot_yyparse ()
at /home/ash/src/postgresql/src/backend/bootstrap/bootparse.y:455
#12 0x000000000052a26b in BootstrapModeMain ()
at /home/ash/src/postgresql/src/backend/bootstrap/bootstrap.c:494
#13 0x000000000052a177 in AuxiliaryProcessMain (argc=6, argv=0x1cc8378)
at /home/ash/src/postgresql/src/backend/bootstrap/bootstrap.c:414
#14 0x00000000006a327c in main (argc=7, argv=0x1cc8370)
at /home/ash/src/postgresql/src/backend/main/main.c:212
(gdb)

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

Alex Shulgin <ash@commandprompt.com> writes:

Figured it out with a pg_usleep in bootstrap.c anyway. Does this look sane?

DEBUG: inserting column 6 value "0"
DEBUG: inserted -> 0
DEBUG: inserting column 7 value "varchar_transform"
TRAP: FailedAssertion("!(((xmax) >= ((TransactionId) 3)))", File: "/home/ash/src/postgresql/src/backend/storage/ipc/procarray.c", Line: 1414)

I've tried to debug this and I feel really dumbfound...

DEBUG: inserting column 7 value "varchar_transform"

Breakpoint 1, GetSnapshotData (snapshot=0xdb2d60 <CatalogSnapshotData>)
at /home/ash/src/postgresql/src/backend/storage/ipc/procarray.c:1413
1413 xmax = ShmemVariableCache->latestCompletedXid;

(gdb) p ShmemVariableCache->latestCompletedXid
$1 = 4294967295

(gdb) p *ShmemVariableCache
$2 = {nextOid = 10000, oidCount = 0, nextXid = 3, oldestXid = 3,
xidVacLimit = 200000003, xidWarnLimit = 2136483650,
xidStopLimit = 2146483650, xidWrapLimit = 2147483650, oldestXidDB = 1,
oldestCommitTs = 1, newestCommitTs = 0, latestCompletedXid = 4294967295}

(gdb) p xmax
$3 = 0

(gdb) n
1414 Assert(TransactionIdIsNormal(xmax));

(gdb) p xmax
$4 = 1

(gdb) p *ShmemVariableCache
$5 = {nextOid = 10000, oidCount = 0, nextXid = 3, oldestXid = 3,
xidVacLimit = 200000003, xidWarnLimit = 2136483650,
xidStopLimit = 2146483650, xidWrapLimit = 2147483650, oldestXidDB = 1,
oldestCommitTs = 1, newestCommitTs = 0, latestCompletedXid = 4294967295}

(gdb) p ShmemVariableCache->latestCompletedXid
$6 = 4294967295

(gdb)

How? Is there an another concurrent process with the old view of
VariableCacheData struct where latestCompletedXid still points to
oldestCommitTs?

This only happens with the CommitTs commit in effect.

--
Alex

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

Alex Shulgin wrote:

DEBUG: inserting column 7 value "varchar_transform"

Breakpoint 1, GetSnapshotData (snapshot=0xdb2d60 <CatalogSnapshotData>)
at /home/ash/src/postgresql/src/backend/storage/ipc/procarray.c:1413
1413 xmax = ShmemVariableCache->latestCompletedXid;

(gdb) p ShmemVariableCache->latestCompletedXid
$1 = 4294967295

I think you might need to "make distclean", then recompile. If you're
not passing --enable-depend to configure, I suggest you do so; that
greatly reduces such problems.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Alex Shulgin wrote:

DEBUG: inserting column 7 value "varchar_transform"

Breakpoint 1, GetSnapshotData (snapshot=0xdb2d60 <CatalogSnapshotData>)
at /home/ash/src/postgresql/src/backend/storage/ipc/procarray.c:1413
1413 xmax = ShmemVariableCache->latestCompletedXid;

(gdb) p ShmemVariableCache->latestCompletedXid
$1 = 4294967295

I think you might need to "make distclean", then recompile. If you're
not passing --enable-depend to configure, I suggest you do so; that
greatly reduces such problems.

Well I tried that before posting the complaint, let me try again though.

--
Alex

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

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Alex Shulgin wrote:

DEBUG: inserting column 7 value "varchar_transform"

Breakpoint 1, GetSnapshotData (snapshot=0xdb2d60 <CatalogSnapshotData>)
at /home/ash/src/postgresql/src/backend/storage/ipc/procarray.c:1413
1413 xmax = ShmemVariableCache->latestCompletedXid;

(gdb) p ShmemVariableCache->latestCompletedXid
$1 = 4294967295

I think you might need to "make distclean", then recompile. If you're
not passing --enable-depend to configure, I suggest you do so; that
greatly reduces such problems.

Yeah, that did it. Sorry for the noise, I must have messed it up with
the recompilations (note to self to always use --enable-depend).

--
Alex

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

On Thu, Dec 4, 2014 at 9:26 PM, Heikki Linnakangas
<hlinnakangas@vmware.com> wrote:

On 12/04/2014 01:47 PM, Petr Jelinek wrote:

On 04/12/14 12:26, Heikki Linnakangas wrote:

On 12/04/2014 01:16 PM, Petr Jelinek wrote:

On 04/12/14 10:42, Heikki Linnakangas wrote:

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.

That's API meant to be used by extensions, maybe it would be preferable
if there was SQL API exposed also?

(It might also make more sense once replication identifiers are
submitted.)

Maybe. Hard to tell without any documentation or comments on how it's
supposed to work. In unacceptable in its current state.

I would prefer to remove it now, and add it back later together with the
code that actually uses it. I don't like having unused internal
functions and APIs sitting the source tree in the hope that someone will
find them useful in the future. It's more likely that it will bitrot, or
not actually work as intended, when someone later tries to use it.

The thing is I already have extension for 9.4 where I would use this API
for conflict detection if it was available so it's not about hoping for
somebody to find it useful. There was discussion about this during the
review process because the objection was raised already then.

Yeah, it was raised. I don't think it was really addressed. There was
lengthy discussion on whether to include LSN, node id, and/or some other
information, but there was no discussion on:

* What is a node ID?
* How is it used?
* Who assigns it?

etc.

None of those things are explained in the documentation nor code comments.

The node ID sounds suspiciously like the replication identifiers that have
been proposed a couple of times. I don't remember if I like replication
identifiers or not, but I'd rather discuss that issue explicitly and
separately. I don't want the concept of a replication/node ID to fly under
the radar like this.

What would the SQL API look like, and what would it be used for?

It would probably mirror the C one, from my POV it's not needed but
maybe some replication solution would prefer to use this without having
to write C components...

I can't comment on that, because without any documentation, I don't even
know what the C API is.

BTW, why is it OK that the node-ID of a commit is logged as a separate WAL
record, after the commit record? That means that it's possible that a
transaction commits, but you crash just before writing the SETTS record, so
that after replay the transaction appears committed but with the default
node ID.

(Maybe that's OK given the intended use case, but it's hard to tell without
any documentation. See a pattern here? ;-) )

Could it be possible to get a refresh on that before it bitrots too
much or we'll simply forget about it? In the current state, there is
no documentation able to explain what is the point of the nodeid
stuff, how to use it and what use cases it is aimed at. The API in
committs.c should as well be part of it. If that's not done, I think
that it would be fair to remove this portion and simply WAL-log the
commit timestamp its GUC is activated.
Thanks,
--
Michael

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

On 10/12/14 04:26, Michael Paquier wrote:

On Thu, Dec 4, 2014 at 9:26 PM, Heikki Linnakangas
<hlinnakangas@vmware.com> wrote:

Yeah, it was raised. I don't think it was really addressed. There was
lengthy discussion on whether to include LSN, node id, and/or some other
information, but there was no discussion on:

* What is a node ID?
* How is it used?
* Who assigns it?

etc.

None of those things are explained in the documentation nor code comments.

The node ID sounds suspiciously like the replication identifiers that have
been proposed a couple of times. I don't remember if I like replication
identifiers or not, but I'd rather discuss that issue explicitly and
separately. I don't want the concept of a replication/node ID to fly under
the radar like this.

Replication identifiers are bigger thing but yes there is overlap as the
nodeid itself makes it possible to implement replication identifiers
outside of core if needed.

What would the SQL API look like, and what would it be used for?

It would probably mirror the C one, from my POV it's not needed but
maybe some replication solution would prefer to use this without having
to write C components...

I can't comment on that, because without any documentation, I don't even
know what the C API is.

BTW, why is it OK that the node-ID of a commit is logged as a separate WAL
record, after the commit record? That means that it's possible that a
transaction commits, but you crash just before writing the SETTS record, so
that after replay the transaction appears committed but with the default
node ID.

(Maybe that's OK given the intended use case, but it's hard to tell without
any documentation. See a pattern here? ;-) )

This is actually good point, it's not OK to have just commit record but
no nodeid record if nodeid was set.

Could it be possible to get a refresh on that before it bitrots too
much or we'll simply forget about it? In the current state, there is
no documentation able to explain what is the point of the nodeid
stuff, how to use it and what use cases it is aimed at. The API in
committs.c should as well be part of it. If that's not done, I think
that it would be fair to remove this portion and simply WAL-log the
commit timestamp its GUC is activated.

I have this on my list so it's not being forgotten and I don't see it
bitrotting unless we are changing XLog api again. I have bit busy
schedule right now though, can it wait till next week? - I will post
some code documentation patches then.

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

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