Re: POC and rebased patch for CSN based snapshots

On 2020/06/19 12:12, movead(dot)li(at)highgo(dot)ca wrote:
>
> Thanks for reply.
>
> >Probably it's not time to do the code review yet, but when I glanced the patch,
>>I came up with one question.
>>0002 patch changes GenerateCSN() so that it generates CSN-related WAL records
>>(and inserts it into WAL buffers). Which means that new WAL record is generated
>>whenever CSN is assigned, e.g., in GetSnapshotData(). Is this WAL generation
>>really necessary for CSN?
> This is designed for crash recovery, here we record our most new lsn in wal so it
> will not use a history lsn after a restart. It will not write into wal every time, but with
> a gap which you can see CSNAddByNanosec() function.

You mean that the last generated CSN needs to be WAL-logged because any smaller
CSN than the last one should not be reused after crash recovery. Right?

If right, that WAL-logging seems not necessary because CSN mechanism assumes
CSN is increased monotonically. IOW, even without that WAL-logging, CSN afer
crash recovery must be larger than that before. No?

>>BTW, GenerateCSN() is called while holding ProcArrayLock. Also it inserts new
>>WAL record in WriteXidCsnXlogRec() while holding spinlock. Firstly this is not
>>acceptable because spinlocks are intended for *very* short-term locks.
>>Secondly, I don't think that WAL generation during ProcArrayLock is good
>>design because ProcArrayLock is likely to be bottleneck and its term should
>>be short for performance gain.
> Thanks for point out which may help me deeply, I will reconsider that.

Thanks for working on this!

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION