Re: Add “FOR UPDATE NOWAIT” lock details to the log.

From: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
To: cca5507 <cca5507(at)qq(dot)com>
Cc: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>, Yuki Seino <seinoyu(at)oss(dot)nttdata(dot)com>, Jelte Fennema-Nio <postgres(at)jeltef(dot)nl>, pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Add “FOR UPDATE NOWAIT” lock details to the log.
Date: 2026-06-12 07:27:56
Message-ID: CAHGQGwEVd9bU_WYyJYV-d71XucEQSchiQUdzD1wp0m7boRZfYA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, May 14, 2026 at 2:28 PM cca5507 <cca5507(at)qq(dot)com> wrote:
>
> Hi,
>
> > Yes, logging every autovacuum lock failure would be too noisy.
> > I was thinking that log_lock_failure should apply only to LOCK TABLE ... NOWAIT,
> > but per the current code, restricting it that way seems difficult.
>
> I wrote a simple patch to support LOCK NOWAIT only.
>
> Looking forward to your comments!

Thanks for the patch! Could you add it to the next CommitFest so we
don't forget about it?

RVR_MISSING_OK = 1 << 0, /* don't error if relation doesn't exist */
RVR_NOWAIT = 1 << 1, /* error if relation cannot be locked */
RVR_SKIP_LOCKED = 1 << 2, /* skip if relation cannot be locked */
+ RVR_LOG_LOCK_FAILURE = 1 << 3, /* log failure if relation cannot be locked */
} RVROption;

I understand this approach. But RVROption currently controls
lookup/locking behavior such as MISSING_OK, NOWAIT, and SKIP_LOCKED,
while RVR_LOG_LOCK_FAILURE is different in nature because it controls
logging side effects rather than lookup/lock semantics themselves.

Because of that, it feels a bit out of place in this enum and makes
the API boundary less clear. So I'd prefer to avoid adding
RVR_LOG_LOCK_FAILURE to RVROption.

Instead, how about a simpler approach: update
ConditionalLockRelationOid() to pass log_lock_failures to
LockAcquireExtended()? That would allow not only LOCK TABLE NOWAIT,
but also VACUUM SKIP LOCKED, ALTER ALL IN TABLESPACE NOWAIT, and
REPACK to follow log_lock_failures.

Of course, that would also cause every autovacuum relation lock failure to be
logged, which would likely be too noisy. To avoid that, how about
forcibly disabling log_lock_failures in autovacuum workers? We already
override some parameters there, so perhaps we could add something like
this as well:

/*
* Force log_lock_failures OFF in autovacuum workers to avoid verbose
* logging when maintenance skips contended relations.
*/
SetConfigOption("log_lock_failures", "false", PGC_SUSET, PGC_S_OVERRIDE);

I think this approach is simpler. Thoughts?

Regards,

--
Fujii Masao

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Taylor 2026-06-12 07:47:19 Re: Remove redundant DISTINCT when GROUP BY already guarantees uniqueness
Previous Message David E. Wheeler 2026-06-12 07:19:51 Re: Why our Valgrind reports suck