The following review has been posted through the commitfest application:
make installcheck-world:  not tested
Implements feature:       tested, passed
Spec compliant:           not tested
Documentation:            not tested

Hi Andrey and Nick,

I reviewed the v3 two-patch series against PostgreSQL commit 94670ba6d56.

Both patches applied cleanly, and git diff --check reported no errors. I 
configured the build with assertions, debug support, TAP tests, and injection 
points enabled. The build completed successfully without warnings or errors.

The implementation moves creation of the WAL-streaming connection and 
replication slot before the BASE_BACKUP request. It then passes the same 
connection to StartLogStreamer(). This closes the interval in which the backup 
startpoint could previously become unprotected before the requested slot was 
created.

I also reviewed the new 057_basebackup_slot_race.pl test. It deterministically 
pauses BASE_BACKUP before the startpoint is returned, forces WAL generation and 
a checkpoint, and verifies that the startpoint segment remains available. The 
test covers both permanent and temporary replication slots and verifies 
successful backup completion and slot cleanup.

Test results:

057_basebackup_slot_race.pl: PASS, 10 tests
Complete src/bin/pg_basebackup test suite: PASS, 5 files and 351 tests
Full make check: PASS

The first component-suite and full-check attempts encountered macOS 
temporary-install Mach-O paths referring to /usr/local/pgsql/lib/libpq.5.dylib. 
After correcting those paths only in the disposable temporary installation, the 
affected tests and complete suites passed. This was a local build-environment 
issue and did not require any source changes.

I did not find any functional or test-coverage issues with the v3 series. The 
patch appears ready for committer review.

Regards,
Shashishekar Hullahally Anantharamu

The new status of this patch is: Ready for Committer

Reply via email to