effective_wal_level is not decreasing after using REPACK (CONCURRENTLY)

First seen: 2026-05-21 16:32:36+00:00 · Messages: 14 · Participants: 4

Latest Update

2026-06-01 · claude-opus-4-6

Monthly Summary: effective_wal_level Not Decreasing After REPACK (CONCURRENTLY) — May 2026

Problem Statement

A bug was identified in the interaction between PostgreSQL's dynamic WAL level toggling feature (commit 67c2097) and the REPACK (CONCURRENTLY) command (commit 28d534e). When REPACK (CONCURRENTLY) executes, it creates a temporary logical replication slot internally for online table reorganization. This correctly elevates effective_wal_level to logical. However, when REPACK completes and drops the temporary slot during cleanup (repack_cleanup_logical_decoding), it fails to call RequestDisableLogicalDecoding(), leaving effective_wal_level permanently stuck at logical until a server restart.

This causes unnecessary WAL volume increases (full tuple data for UPDATE/DELETE), increased I/O and replication bandwidth, and violates the design contract of the dynamic WAL level toggling feature.

Solution Evolution

v1: Point Fix

The initial patch simply added RequestDisableLogicalDecoding() to repack_cleanup_logical_decoding() after the slot is dropped.

v2: API Redesign

The discussion evolved into a broader infrastructure change. Rather than patching individual call sites, the signature of ReplicationSlotDropAcquired() was changed to:

ReplicationSlotDropAcquired(bool disable_logical_decoding)

This "pit of success" API design forces every caller to explicitly declare whether logical decoding should be re-evaluated upon slot drop. The boolean flag exists to support ReplicationSlotsDropDBSlots(), which drops multiple slots in a loop and only needs a single disable request after completion, rather than N redundant calls.

v3: Final Refinements (Reviewed and Approved)

Three improvements from code review:

  1. Updated function comment documenting the new parameter's interaction with the checkpointer
  2. Runtime assertion: Assert(!disable_logical_decoding || SlotIsLogical(MyReplicationSlot)) — prevents nonsensical requests to disable logical decoding when dropping a physical slot
  3. Explanatory comment for the false case in drop_local_obsolete_slots() on physical standbys, where logical decoding state is derived from the upstream primary

Current Status

The v3 patch has been reviewed and approved by Shveta Malik. It awaits committer attention. The thread has converged on the API-level approach, which serves as both a fix and a forced audit of all slot-dropping code paths in the system.

History (1 prior analysis)
2026-06-01 · claude-opus-4-6

Incremental Analysis: Patch Committed, Buildfarm Failure, and Follow-up Fix

Summary

The v3 patch has been committed by Álvaro Herrera (alvherre) with cosmetic changes, followed immediately by a buildfarm failure in the new test, which was diagnosed and fixed by Masahiko Sawada within hours.

Key Events

  1. Commit by Álvaro Herrera (2026-05-27): Pushed with cosmetic changes. Notably, he added an XXX comment in ApplyLauncherMain() because he wasn't sure why that caller passes false for disable_logical_decoding.

  2. Clarification from Sawada: The ApplyLauncherMain() case passes false because it only ever acquires a CONFLICT_DETECTION_SLOT, which is a physical slot — so requesting logical decoding disable would be nonsensical (and would trip the Assert added in v3). This resolves the XXX comment's question.

  3. Buildfarm failures (loach, goldfish): The new test in 051_effective_wal_level.pl failed on builds without injection points. Root cause: Álvaro moved the test to the end of the file for cosmetic reasons, but this broke an implicit dependency — the test assumes a logical slot "test_slot" exists, but in builds with injection points, earlier tests remove that slot. The original patch had intentionally placed the test at an "awkward" position to avoid this interaction.

  4. Fix committed by Sawada: Moved the test earlier in the script so it no longer depends on slot state left over from injection-point tests. Álvaro acknowledged the issue and gave go-ahead to push directly.

Technical Insight: CI Coverage Gap

Both developers noted that the buildfarm caught what CI missed — the no-injection-points build configuration isn't adequately covered in CI pipelines. This is a meta-issue about test infrastructure coverage that may warrant future attention.

Thread Status

Closed/Resolved. The feature patch and its test fix are both committed. The only remaining minor item is the XXX comment in ApplyLauncherMain() which Sawada has already explained (physical slot, not logical).