On 4 April 2016 at 10:35, Simon Riggs <si...@2ndquadrant.com> wrote: > On 4 April 2016 at 09:28, Fujii Masao <masao.fu...@gmail.com> wrote: > > >> Barring any objections, I'll commit this patch. >> > > That sounds good. > > May I have one more day to review this? Actually more like 3-4 hours. >
What we have here is useful and elegant. I love the simplicity and backwards compatibility of the design. Very nice, chef. I am in favour of committing something for 9.6, though I do have some objective comments 1. Header comments in syncrep.c need changes, not just additions. 2. We need tests to ensure that k >=1 and k<=N 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 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). 5. Timing – k > 1 will be slower by definition and more complex to configure, yet there is no timing facility to measure the effect of this, even though we have a new timing facility in 9.6. It would be useful to have a track_syncrep option to keep track of typical response times from nodes. 6. Meaning of k (n1, n2, n3) with N servers It's clearly documented that this means k replies IN SEQUENCE. I believe the typical meaning of would be “any k out of N”, which would be faster than what we have, e.g. 3 (n1, n2, n3) would release as soon as (n1, n2) or (n2, n3) or (n1, n3) acknowledge. The “any k” option is not currently possible, but could be fairly easily. The syntax should also be easily extensible. I would call what we have now “first” semantics, and we could have both of these... * first k (n1, n2, n3) – does the same as k (n1, n2, n3) does now * any k (n1, n2, n3) – would release waiters as soon as we have the responses from k out of N standbys. “any k” would be faster, so is desirable for performance and resilience >>> So I am suggesting we put an extra keyword in front of the “k”, to explain how the k responses should be gathered as an extension to the the syntax. I also think implementing “any k” is actually fairly trivial and could be done for 9.6 (rather than just "first k"). Future thoughts that relate to syntax choices now, not for 9.6 Eventually I would want to be able to specify this… 2 ( any (london1, london2), any (nyc1, nyc2)) meaning I want a response from at least 1 London server and at least one NYC server, but whichever one responds first doesn't matter. And I also want to be able to specify node groups in there. So elsewhere we would specify London node group as (London1, London2) and NYC node group as (NYC1, NYC2) and then specify any 2 (London, NYC, Tokyo). Good work -- Simon Riggs http://www.2ndQuadrant.com/ <http://www.2ndquadrant.com/> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
