|From:||Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>|
|Cc:||alvherre(at)2ndquadrant(dot)com, hailong(dot)li(at)qunar(dot)com, pgsql-hackers(at)postgresql(dot)org|
|Subject:||Re: when set track_commit_timestamp on, database system abort startup|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
At Sat, 15 Sep 2018 19:26:39 +0900, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote in <CAD21AoAxSNorp3TjvJhrOAk+8q5yshSnW-n8buwz4bdU7qOtPA(at)mail(dot)gmail(dot)com>
> >> To fix that maybe we can disable commitTs if
> >> controlFile->track_commit_timestamp == false and the
> >> track_commit_timestamp == true even in crash recovery.
> > Hmm, so keep it off while crash recovery runs, and once it's out of that
> > then enable it automatically?
> Yes. The attached patch changes it to check
> controlFile->track_commit_timestamp even the crash recover case. If
> track_commit_timestamp is set to true in the config file, it's enabled
> at end of the recovery.
> > That might work -- by definition we don't
> > care about the commit TSs of the transaction replayed during crash
> > recovery, since they were executed in the primary that didn't have
> > commitTS enable anyway.
> > It seems like the first thing we need is TAP cases that reproduce these
> > two crash scenarios.
> I attached TAP test that reproduces this issue. We can reproduce it
> even with single server; making postgres replay a commit WAL in the
> crash recovery after consumed transactions and enabled
The fix looks good to me. The TAP test works fine.
In the TAP test:
The test script lacks a general description about its objective.
+ "track_commit_timestamp = off");
+# When we start firstly from the initdb the PARAMETER_CHANGES
+# is emitted at end of the recovery, which disables the
+# track_commit_timestamp if the crash recovery replay that
+# WAL. Therefore we restart the server so that we can recovery
+# from the point where doesn't contain that WAL.
The first startup actually doesn't emit a PARAMETER_CHAGES. If
track_commit_timestamp were set to on, we get one. However, I
agree that it is reasonable to eliminate the possiblity of being
affected by the record. How about something like this?
+# We don't want to replay PARAMETER_CHANGES record while the
+# crash recovery test below. It is not expected to be emitted
+# thus far, but we restart the server to get rid of it just in
+# During the crash recovery we replay the commit WAL that sets
+# the commit timestamp to a new page.
The comment is mentioning the unexpected symptom. Shouldn't it be
the desired behavior?
+# During crash recovery server will replay COMMIT records
+# emitted while commit timestamp was off. The server can start
+# if we correctly avoid processing commit timestamp for the
The following error message is seen when the issue happens.
> DETAIL: Could not read from file "pg_commit_ts/0000" at offset 8192: Success.
This seems what 811b6e36a9 tried to fix. This should be like the follows.
> DETAIL: Could not read from file "pg_commit_ts/0000" at offset 8192: read 0 of 8192"
I, as one of reviewers of it, didn't remember how it was
overlooked butthis also needs a fix in this patch or as a
NTT Open Source Software Center
|Next Message||Michael Paquier||2018-09-19 03:46:21||Re: when set track_commit_timestamp on, database system abort startup|
|Previous Message||Michael Paquier||2018-09-19 03:06:47||Re: heap_sync seems rather oblivious to partitioned tables (wal_level=minimal)|