On Monday, September 15, 2025 12:55 PM shveta malik <shveta.ma...@gmail.com>
wrote:
>
> One concern:
>
> if (should_stop_conflict_info_retention(rdt_data))
> + {
> + /*
> + * Stop retention if not yet. Otherwise, reset to the initial phase to
> + * retry all phases. This is required to recalculate the current wait
> + * time and resume retention if the time falls within
> + * max_retention_duration.
> + */
> + if (MySubscription->retentionactive)
> + rdt_data->phase = RDT_STOP_CONFLICT_INFO_RETENTION;
> + else
> + reset_retention_data_fields(rdt_data);
> +
> return;
> + }
>
> Instead of above code changes, shall we have:
>
> if (should_stop_conflict_info_retention(rdt_data))
> rdt_data->phase = RDT_STOP_CONFLICT_INFO_RETENTION;
> (always)
>
> And then stop_conflict_info_retention() should have these checks:
>
> if (MySubscription->retentionactive)
> {
> ...update flag and perform stop (current functionality)
> }
> else
> {
>
> Assert(!TransactionIdIsValid(MyLogicalRepWorker->oldest_nonremovable_xi
> d));
> reset_retention_data_fields(rdt_data);
> }Thanks for the suggestion. I refactored the codes according to this. Here is V65 patch which also addressed Amit's comments[1]. Best Regards, Hou zj
v75-0001-Allow-conflict-relevant-data-retention-to-resume.patch
Description: v75-0001-Allow-conflict-relevant-data-retention-to-resume.patch
