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
