On Wed, Nov 26, 2025 at 11:58 AM Shlok Kyal <[email protected]> wrote:
>
>
> I have also addressed the remaining comments and attached the updated patch.
>
Thanks. Please find a few comment:
1)
+ <structfield>slotsync_skip_reason</structfield><type>text</type>
+ </para>
+ <para>
+ The reason for the last slot synchronization skip. Slot synchronization
+ occurs only on standby servers and for synced slots (that is,
those whose
+ <structfield>synced</structfield> field is <literal>true</literal>).
+ Thus, this column has no meaning on the primary server and
slot which are
+ not synced. <literal>NULL</literal> if slot synchronization is not
+ performed yet or is successful. Possible values are:
Second line is confusing (..for synced slots..). Shall we rephrase the
complete thing to:
The reason for the last slot synchronization skip. Slot
synchronization occurs only on standby servers and thus this column
has no meaning on the primary server. It is relevant mainly for
logical slots on standby servers whose synced field is true. NULL if
slot synchronization is successful.
2)
Also in the 'possible values section', at one place we are using a
'failover slot on primary' and at another place 'remote slot'. We can
make these words also consistent. Shall we use 'failover slot on
primary' everywhere (as failover is a more widely used term than
remote in existing doc)?
3)
GetSlotSyncSkipReasonName()
+ default:
+ break;
+ }
+
+ Assert(false);
+ return NULL;
Shall we have :
default:
elog(ERROR, "unexpected slotsync skip reason: %d", (int) reason);
return NULL;
}
}
See other such functions: handle_streamed_transaction(),
RelationCreateStorage().
thanks
Shveta