> On Wed, Sep 11, 2019 at 10:17 AM Alvaro Herrera from 2ndQuadrant > <alvherre@alvh.no-ip.org> wrote: > > > > Oh, oops. Here they are then. > > With the permission of the original patch author, Haribabu Kommi, I’ve > rationalized the existing 8 patches into 3 patches, merging patches > 1-5 and 6-7, and tidying up some documentation and code comments. I > also rebased them to the latest PG12 source code (as of October 1, > 2019). The patch code itself is the same, except for some version > checks that I have updated to target the features for PG13 instead of > PG12.
I've spent some time the last few days going over these patches and the prior discussion.
I'm not sure I understand why we end up with "prefer-read" in addition to "prefer-standby" (and similar seeming redundancy between "primary" and "read-write"). Do we really need more than one way to identify hosts' roles? It seems 0001 adds the "prefer-read" modes by checking transaction_read_only, and later 0002 adds the "prefer-standby" modes by checking in_recovery. I'm not sure that we're serving our users very well by giving them choice that ends up being confusing. In other words I think we should do only one of these things, not both. Maybe merge 0001 and 0002 in a single patch, and get rid of redundant modes.
There were other comments that I think went largely unaddressed, such as the point that the JDBC driver seems to offer a different syntax for the configuration, and should we offer a compatibility shim of some sort. (Frankly, I don't think we need to stress over this too much, but it seems that it wasn't even discussed.)
We seem to ignore prior work here I agree. It would be wonderful if there were only one
syntax. Is it too late to change the syntax for this patch as that ship has sailed for JDBC