On 2014-11-05 19:31:52 -0500, Steve Singer wrote:
> On 11/05/2014 05:43 PM, Andres Freund wrote:
> >On 2014-11-05 17:17:05 -0500, Steve Singer wrote:
> >Imo that's essentially a different feature. What you essentially would
> >need here is a 'commit sequence number' - but no timestamps. And
> >probably to be useful that number has to be 8 bytes in itself.
>
> I think this gets to the heart of some of the differing views people have
> expressed on this patch
I think it's actually besides the heart...
> Is this patch supposed to:
>
> A) Add commit timestamp tracking but nothing more
>
> B) Add infrastructure to store commit timestamps and provide a facility for
> storing additional bits of data extensions might want to be associated with
> the commit
>
> C). Add commit timestamps and node identifiers to commits
> If the answer is (A) then I can see why some people are objecting to adding
> extradata. If the answer is (B) then it's fair to ask how well does this
> patch handle various types of things people might want to attach to the
> commit record (such as the LSN).
I think there's a mistake exactly here. The LSN of the commit isn't just
some extra information about the commit. You can't say 'here, also
attach this piece of information'. Instead you need special case code in
xact.c to add it. Thus prohibiting that space to be used for something
else.
> I think the problem is that once you
> start providing a facility extensions can use to store data along with the
> commit record then being restricted to 4 or 8 bytes is very limiting.
Well, you can easily use those 4/8 bytes to start adding more data to
the transaction. By referring to some table with transaction metadata
for example.
> It also doesn't allow you to load two extensions at once on a system.
> You wouldn't be able to have both the 'steve_commit_order' extension
> and BDR installed at the same time. I don't think this patch does a
> very good job at (B) but It wasn't intended to.
Well, I don't agree that steve_commit_order makes much sense in this
context. But there actually is a real problem here, namely that there's
no namespacing in those bytes. I'd be ok with saying that we split the
extradata in for bytes for the namespace and four for the actual
data. That's roughly how it works for advisory locks already.
Greetings,
Andres Freund
-- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services
