Thread

  1. Re: [PATCH] Avoid pallocs in async.c's SignalBackends critical section

    Joel Jacobson <joel@compiler.org> — 2025-11-25T10:15:58Z

    On Mon, Nov 24, 2025, at 22:53, Joel Jacobson wrote:
    > On Mon, Nov 24, 2025, at 17:06, Tom Lane wrote:
    >> Unfortunately, releasing locks, sending notifies, etc is not all
    >> that "noncritical" if you want the DB to keep functioning well.
    >> But there's a good deal of code in there and making it all obey
    >> the critical-section rules looks painful.
    >
    > I see why a critical-section is probably too painful. But since the
    > direction in [1] is to avoid adding new possibly risky operations to
    > AtCommit_Notify, I don't think it's completely unreasonable to consider
    > moving some existing ones into PreCommit_Notify, when feasible.
    >
    > If it's preferable, I'm fine dropping this standalone patch and folding
    > any such adjustments into v29 in [1], or I can just leave the existing
    > code untouched?
    
    With the following three changes, I think the only remaining
    potentially-risky code in AtCommit_Notify, is the acquire/release of
    locks.
    
    * 0001-async-avoid-pallocs-in-critical-section-v2.patch:
    Preallocate signal arrays to avoid pallocs AtCommit.
    
    * 0002-async-avoid-pallocs-in-critical-section-v2.patch:
    Move asyncQueueAdvanceTail from AtCommit to PreCommit.
    
    * 0003-async-avoid-pallocs-in-critical-section-v2.patch
    Convert listenChannels to hash table.
    This is based on Heikki's suggestion
    
        "We really should turn that into a hash table."
    
    from the bug fix thread [2] combined with Tom's idea of a boolean
    
        "is it REALLY listening?"
    
    field [1].
    
    Together, these patches allows us to gets rid of the following comments:
    
    0001:
    -	 * XXX in principle these pallocs could fail, which would be bad. Maybe
    -	 * preallocate the arrays?  They're not that large, though.
    
    0002:
    - * This is (usually) called during CommitTransaction(), so it's important for
    - * it to have very low probability of failure.
    
    0003:
    -	 * XXX It is theoretically possible to get an out-of-memory failure here,
    -	 * which would be bad because we already committed.  For the moment it
    -	 * doesn't seem worth trying to guard against that, but maybe improve this
    -	 * later.
    
    Please advise if we want these changes, and if so, if they should be
    folded into [1] i.e. closing this thread, or if we want to keep this thread.
    
    /Joel
    
    [1] https://www.postgresql.org/message-id/flat/6899c044-4a82-49be-8117-e6f669765f7e@app.fastmail.com
    
    [2] https://www.postgresql.org/message-id/flat/CAK98qZ3wZLE-RZJN_Y%2BTFjiTRPPFPBwNBpBi5K5CU8hUHkzDpw%40mail.gmail.com