Re: logical decoding and replication of sequences, take 2
От | Tomas Vondra |
---|---|
Тема | Re: logical decoding and replication of sequences, take 2 |
Дата | |
Msg-id | d9ad466b-e31f-3d8a-dd74-bef7eb3f4b7d@enterprisedb.com обсуждение исходный текст |
Ответ на | Re: logical decoding and replication of sequences, take 2 (Amit Kapila <amit.kapila16@gmail.com>) |
Ответы |
Re: logical decoding and replication of sequences, take 2
(Amit Kapila <amit.kapila16@gmail.com>)
|
Список | pgsql-hackers |
On 7/28/23 11:42, Amit Kapila wrote: > On Wed, Jul 26, 2023 at 8:48 PM Tomas Vondra > <tomas.vondra@enterprisedb.com> wrote: >> >> On 7/26/23 09:27, Amit Kapila wrote: >>> On Wed, Jul 26, 2023 at 9:37 AM Amit Kapila <amit.kapila16@gmail.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. >> > > While studying the changes for this proposal and related areas, I have > a few comments: > 1. I think you need to advance the origin if it is changed due to > copy_sequence(), otherwise, if the sync worker restarts after > SUBREL_STATE_FINISHEDCOPY, then it will restart from the slot's LSN > value. > True, we want to restart at the new origin_startpos. > 2. Between the time of SYNCDONE and READY state, the patch can skip > applying non-transactional sequence changes even if it should apply > it. The reason is that during that state change > should_apply_changes_for_rel() decides whether to apply change based > on the value of remote_final_lsn which won't be set for > non-transactional change. I think we need to send the start LSN of a > non-transactional record and then use that as remote_final_lsn for > such a change. Good catch. remote_final_lsn is set in apply_handle_begin, but that won't happen for sequences. We're already sending the LSN, but logicalrep_read_sequence ignores it - it should be enough to add it to LogicalRepSequence and then set it in apply_handle_sequence(). > > 3. For non-transactional sequence change apply, we don't set > replorigin_session_origin_lsn/replorigin_session_origin_timestamp as > we are doing in apply_handle_commit_internal() before calling > CommitTransactionCommand(). So, that can lead to the origin moving > backwards after restart which will lead to requesting and applying the > same changes again and for that period of time sequence can go > backwards. This needs some more thought as to what is the correct > behaviour/solution for this. > I think saying "origin moves backwards" is a bit misleading. AFAICS the origin position is not actually moving backwards, it's more that we don't (and can't) move it forwards for each non-transactional change. So yeah, we may re-apply those, and IMHO that's expected - the sequence is allowed to be "ahead" on the subscriber. I don't see a way to improve this, except maybe having a separate LSN for non-transactional changes (for each origin). > 4. BTW, while checking this behaviour, I noticed that the initial sync > worker for sequence mentions the table in the LOG message: "LOG: > logical replication table synchronization worker for subscription > "mysub", table "s" has finished". Won't it be better here to refer to > it as a sequence? > Thanks, I'll fix that. regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
В списке pgsql-hackers по дате отправления:
Следующее
От: Ashutosh BapatДата:
Сообщение: Re: logical decoding and replication of sequences, take 2