On 2021-Mar-18, Fujii Masao wrote:
> On 2021/03/18 13:36, PG Bug reporting form wrote:
> > Hi,
> > recently i am reading commit ts code, and i have a problem about it.
> > i found commit_ts_redo call TransactionTreeSetCommitTsData with last param
> > `write_xlog` true,
> > but RecordTransactionCommitPrepared and RecordTransactionCommit call
> > TransactionTreeSetCommitTsData
> > with `write_xlog` false, which cause there are never xlog with commit_ts -
> > COMMIT_TS_SETTS type.
Hmm, sharp eyes.
> I guess that TransactionTreeSetCommitTsData(write_xlog=true) is basically
> used by some extensions using commit_ts.
I am not aware of any extensions that do that.
> IIUC commit_ts_redo() is called during recovery. So it's strange that
> commit_ts_redo() calls TransactionTreeSetCommitTsData() with write_xlog=true
> because no new WAL can be generated during recovery. Probably this is a bug,
> and it should be called with write_xlog=false, instead. Patch attached.
Yeah, it seems like a bug. With your patch, the write_xlog=true path
becomes unused, and thus the whole COMMIT_TS_SETTS record, so we could
remove those things in branch master. The timestamp is acquired from
the COMMIT record.
> Also one problem is that there is no test for WAL replay of COMMIT_TS_SETTS
> for now. Maybe this is why we could not find that bug.
Yeah, coverage for xlog in commit_ts.c is lacking.
--
Álvaro Herrera 39°49'30"S 73°17'W
"En las profundidades de nuestro inconsciente hay una obsesiva necesidad
de un universo lógico y coherente. Pero el universo real se halla siempre
un paso más allá de la lógica" (Irulan)
