> 1. Header comments in syncrep.c need changes, not just additions.
Okay, will consider this later. And I'd appreciate if you elaborate what changes are necessary specifically.
Some of the old header comments are now wrong.
> 2. We need tests to ensure that k >=1 and k<=N
The changes to replication test framework was included in the patch before, but I excluded it from the patch because I'd like to commit the core part of the patch first. Will review the test part later.
I meant tests of setting the parameters, not tests of the feature itself.
> > 3. There should be a WARNING if k == N to say that we don't yet provide a > level to give Apply consistency. (I mean if we specify 2 (n1, n2) or 3(n1, > n2, n3) etc
Sorry I failed to get your point. Could you tell me what Apply consistency and why we cannot provide it when k = N? > 4. How does it work? > It's pretty strange, but that isn't documented anywhere. It took me a while > to figure it out even though I know that code. My thought is its a lot > slower than before, which is a concern when we know by definition that k >=2 > for the new feature. I was going to mention the fact that this code only > needs to be executed by standbys mentioned in s_s_n, so we can avoid > overhead and contention for async standbys (But Masahiko just mentioned that > also).
Unless I'm missing something, the patch already avoids the overhead of async standbys. Please see the top of SyncRepReleaseWaiters(). Since async standbys exit at the beginning of SyncRepReleaseWaiters(), they don't need to perform any operations that the patch adds (e.g., find out which standbys are synchronous).
I was thinking about the overhead of scanning through the full list of WALSenders for each message, when it is a sync standby.
--
Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
