From: Greg Nancarrow <[email protected]>
> 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 attached the updated patches.
Thank you for taking over this patch. Your arrangement has made the patches
much easier to read!
I've finished reviewing, and my comments are below. Unfortunately, 0003 failed
to apply (I guess only slight modification is needed to apply to HEAD.) I'd
like to proceed to testing when the revised patch becomes available.
(1) 0001
+ /*
+ * Requested type is prefer-read, then record
this host index
+ * and try the other before considering it
later. If requested
+ * type of connection is read-only, ignore this
connection.
+ */
+ if (conn->requested_session_type ==
SESSION_TYPE_PREFER_READ ||
+ conn->requested_session_type ==
SESSION_TYPE_READ_ONLY)
{
This if statement seems unnecessary, because the following part at the
beginning of the CONNECTION_CHECK_TARGET case block precludes entering the if
block. Cases other than "any" are handled first here.
if (conn->sversion >= 70400 &&
- conn->target_session_attrs != NULL &&
- strcmp(conn->target_session_attrs,
"read-write") == 0)
+ conn->requested_session_type !=
SESSION_TYPE_ANY)
+ {
(2) 0002
-} TargetSessionAttrsType;
+} TargetSessionAttrsType;
One space after } is replaced with three tabs. I guess this is an
unintentional change.
(3) 0002
+reject_checked_read_or_write_connection(PGconn *conn)
To follow the naming style of most internal functions in this file, I find it
better to change the name to rejectCheckedReadOrWriteConnection.
(4) 0003
+reject_checked_recovery_connection(PGconn *conn)
The same as the previous one.
(5) 0003
Don't we have to describe in_recovery in both or either of
high-availability.sgml and config.sgml? transaction_read_only is touched in
the former.
Regards
Takayuki Tsunakawa
