On 7/28/23 14:44, Ashutosh Bapat wrote:
> On Wed, Jul 26, 2023 at 8:48 PM Tomas Vondra
> <tomas.vondra@enterprisedb.com> wrote:
>>
>> Anyway, I was thinking about this a bit more, and it seems it's not as
>> difficult to use the page LSN to ensure sequences don't go backwards.
>> The 0005 change does that, by:
>>
>> 1) adding pg_sequence_state, that returns both the sequence state and
>> the page LSN
>>
>> 2) copy_sequence returns the page LSN
>>
>> 3) tablesync then sets this LSN as origin_startpos (which for tables is
>> just the LSN of the replication slot)
>>
>> AFAICS this makes it work - we start decoding at the page LSN, so that
>> we skip the increments that could lead to the sequence going backwards.
>>
>
> I like this design very much. It makes things simpler than complex.
> Thanks for doing this.
>
I agree it seems simpler. It'd be good to try testing / reviewing it a
bit more, so that it doesn't misbehave in some way.
> I am wondering whether we could reuse pg_sequence_last_value() instead
> of adding a new function. But the name of the function doesn't leave
> much space for expanding its functionality. So we are good with a new
> one. Probably some code deduplication.
>
I don't think we should do that, the pg_sequence_last_value() function
is meant to do something different. I don't think it'd be any simpler to
also make it do what pg_sequence_state() does would make it any simpler.
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company