Re: Sync Rep v19

On Fri, Mar 4, 2011 at 12:02 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> Though I've not read whole of the patch yet, here is the current comment:

Here are another comments:

+#replication_timeout_client = 120 # 0 means wait forever

Typo: s/replication_timeout_client/sync_replication_timeout

+ else if (timeout > 0 &&
+ TimestampDifferenceExceeds(GetCurrentTransactionStopTimestamp(),
+ wait_start, timeout))

If SetCurrentTransactionStopTimestamp() is called before (i.e., COMMIT case),
the return value of GetCurrentTransactionStopTimestamp() is the same as
"wait_start". So, in this case, the timeout never expires.

+ strcpy(new_status + len, " waiting for sync rep");
+ set_ps_display(new_status, false);

How about changing the message to something like "waiting for %X/%X"
(%X/%X indicates the LSN which the backend is waiting for)?

Please initialize MyProc->procWaitLink to NULL in InitProcess() as well as
do MyProc->lwWaitLink.

+ /*
+ * We're a potential sync standby. Release waiters if we are the
+ * highest priority standby. We do this even if the standby is not yet
+ * caught up, in case this is a restart situation and
+ * there are backends waiting for us. That allows backends to exit the
+ * wait state even if new backends cannot yet enter the wait state.
+ */

I don't think that it's good idea to switch the high priority standby which has
not caught up, to the sync one, especially when there is already another
sync standby. Because that degrades replication from sync to async for
a while, even though there is sync standby which has caught up.

+ if (walsnd->pid != 0 &&
+ walsnd->sync_standby_priority > 0 &&
+ (priority == 0 ||
+ priority < walsnd->sync_standby_priority))
+ {
+ priority = walsnd->sync_standby_priority;
+ syncWalSnd = walsnd;
+ }

According to the code, the last named standby has highest priority. But the
document says the opposite.

ISTM the waiting backends can be sent the wake-up signal by the
walsender multiple times since the walsender doesn't remove any
entry from the queue. Isn't this unsafe? waste of the cycle?

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center