Re: [PATCH] Support automatic sequence replication

Some review comments for v3-0001.

======
.../replication/logical/sequencesync.c

copy_sequences:

1.
- if (sync_status == COPYSEQ_SUCCESS)
+
+ /*
+ * For sequences in READY state, only sync if there's drift
+ */
+ if (relstate == SUBREL_STATE_READY && sync_status == COPYSEQ_SUCCESS)
+ {
+ should_sync = check_sequence_drift(seqinfo);
+ if (should_sync)
+ drift_detected = true;
+ }
+
+ if (sync_status == COPYSEQ_SUCCESS && should_sync)
sync_status = copy_sequence(seqinfo,
- sequence_rel->rd_rel->relowner);
+ sequence_rel->rd_rel->relowner,
+ relstate);
+ else if (sync_status == COPYSEQ_SUCCESS && !should_sync)
+ sync_status = COPYSEQ_NOWORK;

I'm struggling somewhat to follow this logic.

For example, every condition refers to COPYSEQ_SUCCESS -- is that
really necessary; can't this all be boiled down to something like
below?

SUGGESTION

/*
* For sequences in INIT state, always sync.
* Otherwise, for sequences in READY state, only sync if there's drift.
*/
if (sync_status == COPYSEQ_SUCCESS)
{
if ((relstate == SUBREL_STATE_INIT) || check_sequence_drift(seqinfo))
sync_status = copy_sequence(...);
else
sync_status = COPYSEQ_NOWORK;
}

======
Kind Regards,
Peter Smith.
Fujitsu Australia