At Tue, 5 Apr 2016 20:17:21 +0900, Fujii Masao <masao.fu...@gmail.com> wrote in 
<CAHGQGwE8_F79BUpC5TmJ7aazXU=uju0vznfcckdk57-wnph...@mail.gmail.com>
> >> list_member_int() performs the loop internally. So I'm not sure how much
> >> adding extra list_member_int() here can optimize this processing.
> >> Another idea is to make SyncRepGetSyncStandby() check whether I'm sync
> >> standby or not. In this idea, without adding extra loop, we can exit 
> >> earilier
> >> in the case where I'm not a sync standby. Does this make sense?
> >
> > The list_member_int() is also performed in the "(snip)" part. So
> > SyncRepGetSyncStandbys() returning am_sync seems making sense.
> >
> > sync_standbys = SyncRepGetSyncStandbys(am_sync);
> >
> > /*
> >  *  Quick exit if I am not synchronous or there's not
> >  *  enough synchronous standbys
> >  * /
> > if (!*am_sync || list_length(sync_standbys) < SyncRepConfig->num_sync)
> > {
> >   list_free(sync_standbys);
> >   return false;
> > }
> 
> Thanks for the comment! I changed SyncRepGetSyncStandbys() so that
> it checks whether we're managing a sync standby or not.
> Attached is the updated version of the patch. I also applied several
> review comments to the patch.

It still does list_member_int but it can be gotten rid of as the
attached patch.

regards,
diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c
index 9b2137a..6998bb8 100644
--- a/src/backend/replication/syncrep.c
+++ b/src/backend/replication/syncrep.c
@@ -590,6 +590,10 @@ SyncRepGetSyncStandbys(bool *am_sync)
 		if (XLogRecPtrIsInvalid(walsnd->flush))
 			continue;
 
+		/* Notify myself as 'synchonized' if I am */
+		if (am_sync != NULL && walsnd == MyWalSnd)
+			*am_sync = true;
+
 		/*
 		 * If the priority is equal to 1, consider this standby as sync
 		 * and append it to the result. Otherwise append this standby
@@ -598,8 +602,6 @@ SyncRepGetSyncStandbys(bool *am_sync)
 		if (this_priority == 1)
 		{
 			result = lappend_int(result, i);
-			if (am_sync != NULL && walsnd == MyWalSnd)
-				*am_sync = true;
 			if (list_length(result) == SyncRepConfig->num_sync)
 			{
 				list_free(pending);
@@ -630,9 +632,6 @@ SyncRepGetSyncStandbys(bool *am_sync)
 	{
 		bool		needfree = (result != NIL && pending != NIL);
 
-		if (am_sync != NULL && !(*am_sync))
-			*am_sync = list_member_int(pending, MyWalSnd->slotno);
-
 		result = list_concat(result, pending);
 		if (needfree)
 			pfree(pending);
@@ -640,6 +639,13 @@ SyncRepGetSyncStandbys(bool *am_sync)
 	}
 
 	/*
+	 * The pending list contains eventually potentially-synchronized standbys
+	 * and this walsender may be one of them. So once reset am_sync.
+	 */
+	if (am_sync != NULL)
+		*am_sync = false;
+
+	/*
 	 * Find the sync standbys from the pending list.
 	 */
 	priority = next_highest_priority;
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to