| 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
| 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 |