The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: tested, failed
Spec compliant: not tested
Documentation: not tested
Hi,
I've been playing with the patch. It worked as intended. I have a few minor review comments on the code and test:
1. There was some indent issue when applying the v6-0001 patch:
v6-0001-Introduce-feature-to-start-WAL-receiver-eagerly.patch:342: space before tab in indent.
gettext_noop("When to start WAL receiver."),
v6-0001-Introduce-feature-to-start-WAL-receiver-eagerly.patch:343: space before tab in indent.
NULL,
warning: 2 lines add whitespace errors.
2. There was a whitespace issue when applying the v6-0002 test patch:
v6-0002-Test-WAL-receiver-early-start-upon-reaching-consi.patch:126: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
3. Test number for "046_walreciver_start.pl" collided with a recently added test "046_checkpoint_logical_slot.pl" so
needsanother number.
4. Some text needs wraparound:
* Archiving from the restore command does not holds the control lock
- * and enabling XLogCtl->InstallXLogFileSegmentActive for wal reciever early start
- * will create a race condition with the checkpointer process as fixed in
cc2c7d65fc27e877c9f407587b0b92d46cd6dd16.
- * Hence skipping early start of the wal receiver in case of archive recovery.
+ * and enabling XLogCtl->InstallXLogFileSegmentActive for wal reciever
+ * early start will create a race condition with the checkpointer process
+ * as fixed in cc2c7d65fc27e877c9f407587b0b92d46cd6dd16. Hence skipping
+ * early start of the wal receiver in case of archive recovery.
*/
5. Extra ";"
@@ -3820,7 +3821,7 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
* eagerly).
*/
currentSource = XLOG_FROM_STREAM;
- startWalReceiver = !WalRcvStreaming();;
+ startWalReceiver = !WalRcvStreaming();
Thanks,
Huansong
Broadcom Inc.
The new status of this patch is: Waiting on Author
