Context
Adversarial code review of the fork patch (bb/fix-r2-multipart-uniform-parts: f45f59d, 39f4c52, 6eccfb3) performed 2026-06-10 before promoting the CNPG overlay image to mainnet. The patch fixes intermittent Cloudflare R2 backup failures — InvalidPart on CompleteMultipartUpload — by emitting uniform fixed-size multipart parts (upstream issue: EnterpriseDB#954). Runtime diff vs barman 3.17.0 is confined to barman/cloud.py (+85/−25) plus tests (tests/test_cloud.py +174).
Review verdict: safe. Verified correct: byte-stream slicing (no drop/dup/reorder), sequential part numbering in the main process, parts materialized to disk before enqueue and sorted by PartNumber before CompleteMultipartUpload (safe with --jobs 2), no empty trailing part on exact part-size multiples, 10000-part max and 5 MiB min enforced, abort path exits nonzero, WAL archiving path untouched (uses upload_fileobj(), not CloudTarUploader). 239 tests pass including 9 new.
Validated in production: testnet canary (on-demand + scheduled backups, full restore drill with pg_amcheck --heapallindexed clean) and mainnet (41-minute backup 20260611T010836, zero retries/errors).
Non-blocking follow-ups
Pre-existing issues observed (not introduced by the patch)
Strategic
Context
Adversarial code review of the fork patch (
bb/fix-r2-multipart-uniform-parts: f45f59d, 39f4c52, 6eccfb3) performed 2026-06-10 before promoting the CNPG overlay image to mainnet. The patch fixes intermittent Cloudflare R2 backup failures —InvalidPartonCompleteMultipartUpload— by emitting uniform fixed-size multipart parts (upstream issue: EnterpriseDB#954). Runtime diff vs barman 3.17.0 is confined tobarman/cloud.py(+85/−25) plus tests (tests/test_cloud.py+174).Review verdict: safe. Verified correct: byte-stream slicing (no drop/dup/reorder), sequential part numbering in the main process, parts materialized to disk before enqueue and sorted by
PartNumberbeforeCompleteMultipartUpload(safe with--jobs 2), no empty trailing part on exact part-size multiples, 10000-part max and 5 MiB min enforced, abort path exits nonzero, WAL archiving path untouched (usesupload_fileobj(), notCloudTarUploader). 239 tests pass including 9 new.Validated in production: testnet canary (on-demand + scheduled backups, full restore drill with
pg_amcheck --heapallindexedclean) and mainnet (41-minute backup20260611T010836, zero retries/errors).Non-blocking follow-ups
barman/cloud.py— the comment near_upload_part_buffersuggests retry semantics that don't match the actual call sites; reword to describe what theis_finalflag actually guards.CloudTarUploaderwith a compressor configured (gzip/bzip2) to lock in uniform part sizing on the compressed stream.close()-on-exact-multiple test — assert no empty trailing part is uploaded when the stream length is an exact multiple of the part size.Pre-existing issues observed (not introduced by the patch)
barman/cloud.py:325-328) — on the already-fatal abort path, part temp files are not cleaned up. Process is exiting nonzero anyway, but worth tidying.close()(barman/cloud.py:339-348) — a zero-byte stream would create a multipart upload with no parts; cannot occur in practice (tar streams are never empty) but cheap to guard.Strategic
barmanObjectStoresupport is removed in CNPG 1.30 (deprecation warning already emitted on 1.29). The replacement barman-cloud plugin sidecar will need this same fix — track whether upstream lands it before we migrate, otherwise the overlay-image approach needs rethinking for the plugin image.