Hi,
On 28/10/14 13:25, Simon Riggs wrote:
> On 13 October 2014 10:05, Petr Jelinek <petr@2ndquadrant.com> wrote:
>
>>> I worked bit on this patch to make it closer to committable state.
>
>> Here is updated version that works with current HEAD for the October
>> committfest.
>
> I've reviewed this and it looks good to me. Clean, follows existing
> code neatly, documented and tested.
>
Thanks for looking at this.
> Please could you document a few things
>
> * ExtendCommitTS() works only because commit_ts_enabled can only be
> set at server start.
> We need that documented so somebody doesn't make it more easily
> enabled and break something.
> (Could we make it enabled at next checkpoint or similar?)
Maybe we could, but that means some kind of two step enabling facility
and I don't want to write that as part of the initial patch since that
will need separate discussion (i.e. do we want to have new GUC flag for
this, or hack solution just for committs?). So maybe later in a
follow-up patch.
> * The SLRU tracks timestamps of both xids and subxids. We need to
> document that it does this because Subtrans SLRU is not persistent. If
> we made Subtrans persistent we might need to store only the top level
> xid's commitTS, but that's very useful for typical use cases and
> wouldn't save much time at commit.
Attached version with the above comments near the relevant code.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
