Re: issue with track_commit_timestamp and server restart

On 24/10/2016 06:58, Craig Ringer wrote:
> On 22 October 2016 at 19:51, Julien Rouhaud <julien(dot)rouhaud(at)dalibo(dot)com> wrote:
>> I just noticed that if track_commit_timestamp is enabled, the
>> oldestCommitTsXid and newestCommitTsXid don't persist after a server
>> restart, so you can't ask for the commit ts of a transaction that
>> committed before the last server start, although the information is
>> still available (unless of course if a freeze occured). AFAICT it
>> always behave this way.
>
> I initially could'n't see this when tested on 8f1fb7d with a
> src/test/recovery/t test script. But it turns out it's OK on immediate
> shutdown and crash recovery, but not on clean shutdown and restart.
>
> The attached patch adds a TAP test to src/test/recovery to show this.
> If you run the TAP test before recompiling with the fix it'll fail.
> "make" to apply the fix, then re-run and it'll succeed. Or just
> temporarily roll back the fix with:
>
> git checkout HEAD^1 -- src/backend/access/transam/commit_ts.c
> git reset src/backend/access/transam/commit_ts.c
>
> and rebuild to see it fail.
>
> To save time running the recovery suite, just
>
> rm src/test/recovery/00[0-8]*.pl
>
> (It'd be nice to have a prove_check target to run just one test file).
>
> This would explain a few issues I've seen reported with BDR from the
> community which I've so far been unable to reproduce, so thanks very
> much for the report.
>
> Álvaro, would you mind checking this and committing to HEAD and 9.6?
> The commits.c change only should also be committed to 9.5, but not the
> TAP test.
>

Thanks a lot for the review, and adding the tests!

--
Julien Rouhaud
http://dalibo.com - http://dalibo.org