Hi Hari-san,

I've reviewed all the files.  The patch would be OK when the following have 
been fixed, except for the complexity of fe-connect.c (which probably cannot be 
improved.)

Unfortunately, I'll be absent next week.  The earliest date I can do the test 
will be April 8 or 9.  I hope someone could take care of this patch...


(1) 0001
With this read-only option type, application can connect to
to a read-only server in the list of hosts, in case
...
before issuing the SHOW transaction_readonly to find out whether


"to" appears twice in a row.
transaction_readonly -> transaction_read_only


(2) 0001
+        succesful connection or failure.

succesful -> successful


(3) 0008
to conenct to a standby server with a faster check instead of

conenct -> connect


(4) 0008
Logically, recovery exit should be notified after the following statement:

    XLogCtl->SharedRecoveryInProgress = false;


(5) 0008
+               /* Update in_recovery status. */
+               if (LocalRecoveryInProgress)
+                       SetConfigOption("in_recovery",
+                                                       "on",
+                                                       PGC_INTERNAL, 
PGC_S_OVERRIDE);
+

This SetConfigOption() is called for every RecoveryInProgress() call on the 
standby.  It may cause undesirable overhead.  How about just calling 
SetConfigOption() once in InitPostgres() to set the initial value for 
in_recovery?  InitPostgres() and its subsidiary functions call 
SetConfigOption() likewise.


(6) 0008
async.c is for LISTEN/UNLISTEN/NOTIFY.  How about adding the new functions in 
postgres.c like RecoveryConflictInterrupt()?


(7) 0008
+               if (pid != 0)
+               {
+                       (void) SendProcSignal(pid, reason, procvxid.backendId);
+               }

The braces are not necessary because the block only contains a single statement.


Regards
Takayuki Tsunakawa

Reply via email to