Re: POC and rebased patch for CSN based snapshots

On 2020/06/15 16:48, Fujii Masao wrote:
> 
> 
> On 2020/06/12 18:41, movead.li@highgo.ca wrote:
>> Hello hackers,
>>
>> Currently, I do some changes based on the last version:
>> 1. Catch up to the current  commit (c2bd1fec32ab54).
>> 2. Add regression and document.
>> 3. Add support to switch from xid-base snapshot to csn-base snapshot,
>> and the same with standby side.

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?

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.

Regards,

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