On Mon, Feb 21, 2011 at 6:06 PM, Fujii Masao <masao.fu...@gmail.com> wrote:
> I've read about a tenth of the patch, so I'll submit another comments
> about the rest later.
Here are another comments:
SyncRepReleaseWaiters should be called when walsender exits. Otherwise,
if the standby crashes while a transaction is waiting for replication,
it waits infinitely.
sync_rep_service and potential_sync_standby are not required to be in the
WalSnd shmem because only walsender accesses them.
+static bool
+SyncRepServiceAvailable(void)
+{
+ bool result = false;
+
+ SpinLockAcquire(&WalSndCtl->ctlmutex);
+ result = WalSndCtl->sync_rep_service_available;
+ SpinLockRelease(&WalSndCtl->ctlmutex);
+
+ return result;
+}
volatile pointer needs to be used to prevent code rearrangement.
+ slock_t ctlmutex; /* locks shared variables shown
above */
cltmutex should be initialized by calling SpinLockInit.
+ /*
+ * Stop providing the sync rep service, even if there
are
+ * waiting backends.
+ */
+ {
+ SpinLockAcquire(&WalSndCtl->ctlmutex);
+ WalSndCtl->sync_rep_service_available = false;
+ SpinLockRelease(&WalSndCtl->ctlmutex);
+ }
sync_rep_service_available should be set to false only when
there is no synchronous walsender.
+ /*
+ * When we first start replication the standby will be behind the
primary.
+ * For some applications, for example, synchronous replication, it is
+ * important to have a clear state for this initial catchup mode, so we
+ * can trigger actions when we change streaming state later. We may stay
+ * in this state for a long time, which is exactly why we want to be
+ * able to monitor whether or not we are still here.
+ */
+ WalSndSetState(WALSNDSTATE_CATCHUP);
+
The above has already been committed. Please remove that from the patch.
I don't like calling SyncRepReleaseWaiters for each feedback because
I guess that it's too frequent. How about receiving all the feedbacks available
from the socket, and then calling SyncRepReleaseWaiters as well as
walreceiver does?
+ bool ownLatch; /* do we own the above latch? */
We can just remove the ownLatch flag.
I've read about two-tenths of the patch, so I'll submit another comments
about the rest later. Sorry for the slow reviewing...
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
