Haribabu Kommi wrote: > On Wed, Jan 24, 2018 at 9:01 AM Jing Wang <jingwangian@gmail.com> wrote: > > Hi All, > > > > Recently I put a proposal to support 'prefer-read' parameter in target_session_attrs in libpq. Now I updated the patch with adding content in the sgml and regression test case. > > > > Some people may have noticed there is already another patch (https://commitfest.postgresql.org/15/1148/ ) which looks similar with this. But I would say this patch is more complex than my proposal. > > > > It is better separate these 2 patches to consider. > > I also feel prefer-read and read-only options needs to take as two different options. > prefer-read is simple to support than read-only. > > Here I attached an updated patch that is rebased to the latest master and also > fixed some of the corner scenarios.
Thanks for the review.
The patch applies, builds and passes "make check-world".
I think the "prefer-read" functionality is desirable: It is exactly what you need if you want to use replication for load balancing, and your application supports different database connections for reading and writing queries.
"read-only" does not have a clear use case in my opinion.
With the patch, PostgreSQL behaves as expected if I have a primary and a standby and run:
But if I stop the standby (port 5434), libpq goes into an endless loop.
There was a problem in reusing the primary host index and it leads to loop.
Attached patch fixed the issue.
Concerning the code:
- The documentation needs some attention. Suggestion:
If this parameter is set to <literal>prefer-read</literal>, connections where <literal>SHOW transaction_read_only</literal> returns off are preferred. If no such connection can be found, a connection that allows read-write transactions will be accepted.
updated as per you comment.
- I think the construction with "read_write_host_index" makes the code even more complicated than it already is.
What about keeping the first successful connection open and storing it in a variable if we are in "prefer-read" mode. If we get the read-only connection we desire, close that cached connection, otherwise use it.
Even if we add a variable to cache the connection, I don't think the logic of checking
the next host for the read-only host logic may not change, but the extra connection
request to the read-write host again will be removed.