Horton

Re: Conflict detection for update_deleted in logical replication

Amit Kapila <amit.kapila16@gmail.com>

View thread View on postgr.es
From: Amit Kapila <amit.kapila16@gmail.com>
To: shveta malik <shveta.malik@gmail.com>
Cc: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>, vignesh C <vignesh21@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Nisha Moond <nisha.moond412@gmail.com>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2025-05-23T08:56:59Z
Lists: pgsql-hackers 
On Fri, May 23, 2025 at 12:15 PM shveta malik <shveta.malik@gmail.com> wrote:
>
> Thanks you for v31 patch-set. Please find few comments on patch001:
>
> 1)
>
> wait_for_local_flush:
>
> + if (data->last_recv_time &&
> + TimestampDifferenceExceeds(data->flushpos_update_time,
> +    data->last_recv_time, WalWriterDelay))
> + {
> + XLogRecPtr writepos;
> + XLogRecPtr flushpos;
> + bool have_pending_txes;
> +
> + /* Fetch the latest remote flush position */
> + get_flush_position(&writepos, &flushpos, &have_pending_txes);
> +
> + if (flushpos > last_flushpos)
> + last_flushpos = flushpos;
> +
> + data->flushpos_update_time = data->last_recv_time;
> + }
>
> We should only get new flush-position, if 'last_flushpos' is still
> lesser than 'data->remote_lsn'. Since 'last_flushpos' is also updated
> by 'send_feedback' and we do not update 'data->flushpos_update_time'
> there, it is possible that we have latest flush position but still
> TimestampDifferenceExceeds gives 'true', making it re-read the flush
> position unnecessarily.
>
> Having said that, I think the correct way will be to move
> 'flushpos_update_time' out of RetainConflictInfoData() similar to
> last_flushpos. Let it be a static variable, then we can update it in
> send_feedback as well.
>

But then we may sometimes need to call GetCurrentTimestamp to set its
value, which is not required now. Because we expect to skip this check
only in case when we are frequently applying the changes.

-- 
With Regards,
Amit Kapila.

Commits

  1. Fix intermittent BF failures in 035_conflicts.

  2. Resume conflict-relevant data retention automatically.

  3. Fix intermittent test failure introduced in 6456c6e2c4.

  4. Fix Coverity issue reported in commit a850be2fe.

  5. Add test to prevent premature removal of conflict-relevant data.

  6. Post-commit review fixes for 228c370868.

  7. Add max_retention_duration option to subscriptions.

  8. Detect and report update_deleted conflicts.

  9. Preserve conflict-relevant data during logical replication.

  10. Integrate FullTransactionIds deeper into two-phase code

  11. Improve checks for GUC recovery_target_timeline

  12. Prevent excessive delays before launching new logrep workers.

  13. Keep WAL segments by slot's last saved restart LSN

  14. Rework some code handling pg_subscription data in psql and pg_dump

  15. Use generateClonedIndexStmt to propagate CREATE INDEX to partitions.