| From: | Alexandre Felipe <o(dot)alexandre(dot)felipe(at)gmail(dot)com> |
|---|---|
| To: | Andres Freund <andres(at)anarazel(dot)de> |
| Cc: | Merlin Moncure <mmoncure(at)gmail(dot)com>, shihao zhong <zhong950419(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Throwing away unnecessary spin-locks |
| Date: | 2026-10-04 16:06:20 |
| Message-ID: | CAE8JnxP=GBWwc3+C4YJ-OctCiNf0Y_0Y3Ph+jW-QojHag-v92Q@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Fri, Oct 2, 2026 at 7:05 PM Andres Freund <andres(at)anarazel(dot)de> wrote:
> I'd not do many of these at once. Do one simple conversion, with the
> relevant
> analysis, and see whether it goes anywhere and whether you need to adjust
> your
> approach. After that maybe 2 at once.
>
> I don't think one commit per block is quite right either. You'll typically
> have to add comments to the read and write side of some specific state
> together, splitting that into separate commits doesn't make sense.
>
> And the code needs to document the rules, not just the commit message.
>
>
> > The analysis will be way longer than the changes themselves.
>
> Of course.
>
Trying the checkpointer, I don't know much about it as a whole, I identified
the logic from the code, combined flags and a version number in a
pg_atomic_u32 state.
the { .ckpt_failed++; .ckpt_done = .ckpt_started } update
handled by inserting a memory barrier, to ensure .ckpt_failed is properly
sampled.
the {.ckpt_flags = 0, .ckpt_started++} was handled by detecting the race
afterwards
read .ckpt_started before and after setting the flags, if it changed, check
the state's
version to determine whether it started before or after RequestCheckpoint
setted
its flags.
Identified something unexpected (to me): RequestCheckpoint can fail due to a
failure on a checkpoint started before checkpointer seeing the updated
flags.
Added a slightly different error message for that condition, just in case.
I imaging that this will not impact performance at all, but I hope it helps
us
Regards,
Alexandre.
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-Remove-ckpt_lck.patch | application/octet-stream | 17.2 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tatsuya Kawata | 2026-10-04 16:07:39 | Re: Material node can report incorrect "Maximum Storage" in EXPLAIN |
| Previous Message | Rui Zhao | 2026-10-04 15:50:44 | Re: Serverside SNI support in libpq |