Message-ID: <4D397FE4.2020709@enterprisedb.com>
Date: Fri, 21 Jan 2011 14:45:24 +0200
From: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; fi-FI;
	rv:1.9.1.15) Gecko/20101030 Icedove/3.0.10
MIME-Version: 1.0
To: Simon Riggs <simon@2ndQuadrant.com>
CC: pgsql-hackers@postgresql.org
Subject: Re: Sync Rep for 2011CF1
References: <1295127631.3282.100.camel@ebony>
In-Reply-To: <1295127631.3282.100.camel@ebony>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

(grr, I wrote this on Monday already, but just found it in my drafts 
folder, unsent)

On 15.01.2011 23:40, Simon Riggs wrote:
>
> Here's the latest patch for sync rep.
>
>> From here, I will be developing the patch further on public git
> repository towards commit. My expectation is that commit is at least 2
> weeks away, though there are no major unresolved problems. I expect
> essential follow on patches to continue for a further 2-4 weeks after
> that first commit.

Thanks! Some quick observations after first read-through:

* The docs for synchronous_replication still claim that it means two 
different things in master and standby. Looking at the code, I believe 
that's not true anymore.

* it seems like overkill to not let clients to even connect when 
allow_standalone_primary=off and no synchronous standbys are available. 
What if you just want to run a read-only query?

* Please separate the hot standby feedback loop into a separate patch on 
top of the synch rep patch. I know it's not a lot of code, but it's 
still easier to handle features separately.

* The UI differs from what was agreed on here: 
http://archives.postgresql.org/message-id/4D1DCF5A.7070808@enterprisedb.com.

* Instead of the short-circuit for autovacuum in SyncRepWaitOnQueue(), 
it's probably better to set synchronous_commit=off locally when the 
autovacuum process starts.

* the "queue id" thing is dead code at the moment, as there is only one 
queue. I gather this is a leftover from having different queues for 
"apply", "sync", "write" modes, but I think it would be better to just 
remove it for now.

PS, I'm surprised how small this patch is. Thinking about it some more, 
I don't know why I expected this to be a big patch.

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com