On Tue, Nov 13, 2018 at 7:26 AM Laurenz Albe <laurenz.a...@cybertec.at> wrote:
> Tom Lane wrote: > > Laurenz Albe <laurenz.a...@cybertec.at> writes: > > > Haribabu Kommi wrote: > > > > Added comments along the lines that you mentioned. And also try > > > > to update some more comments. > > > Looks ok to me, I'll mark it as "ready for committer". > > > > I don't like this patch at all: the business with keeping two connections > > open seems impossibly fragile and full of race conditions. (For > instance, > > by the time you return the read-only session to the application, it might > > not be read-only any more. I also wonder what inquiry functions like > > PQsocket ought to return while in this state.) I think the feature > > definition needs to be re-thought to make that unnecessary. > > As it is now, the patch doesn't keep two connections open. It remembers > the index of the host of the first successful writable connection, but > closes the connection, and opens another one to that host if no read-only > host can be found. > > If the read-only connection turns writable after it has been tested, > but before it is returned, that can hardly be avoided. > I don't think that's so bad - after all, you asked for a read-only > connection *if possible*. > If you demand that the server be not promoted until the connection has > been returned to the client, you'd somehow have to block the server > from being promoted, right? > I also have the same opinion of Laurenz, that this option is letting the application to connect to read-only server if possible, otherwise let it connect to read-write server. I feel that any of the state changes during the connection and after connection, needs not to be reflected on the existing connection for these type of connections. > > Also, we really need to consider the interaction between this and the > > feature(s) being discussed in the thread at > > > > > https://www.postgresql.org/message-id/flat/1700970.cRWpxnom9y%40hammer.magicstack.net > > That's a good point. > Thanks for the link. Based on the conclusion on the other thread of GUC_REPORT, this patch also can use that logic, but that is limited only till the connection establishment for these connection types. Regards, Haribabu Kommi Fujitsu Australia
