| From: | Melanie Plageman <melanieplageman(at)gmail(dot)com> |
|---|---|
| To: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
| Cc: | Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de> |
| Subject: | Re: Checkpointer write combining |
| Date: | 2025-11-19 22:12:49 |
| Message-ID: | CAAKRu_ZBzTp-o4pu1UwmpLWkFmAmP7dyGFo867HxMo-AF+=MDw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Thanks for the review!
On Tue, Nov 18, 2025 at 9:10 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>
> > On Nov 19, 2025, at 10:00, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>
> > I just reviewed 0007. It removes the second parameter "bool skip_recently_used” from SyncOneBuffer. The function is static and is only called in one place with skip_recently_used=true, thus removing the parameter seems reasonable, and without considering pinned buffer, the function is simplified a little bit.
> >
> > I only got a tiny comment:
> > ```
> > + * We can make these check without taking the buffer content lock so
> > ```
> >
> > As you changed “this” to “these”, “check” should be changed to “checks” accordingly.
I've made this change. Attached v10 has this.
> I just got an compile error:
> ```
> bufmgr.c:3580:33: error: no member named 'dbId' in 'struct CkptSortItem'
> 3580 | batch.rlocator.dbOid = item.dbId;
> | ~~~~ ^
> bufmgr.c:3598:13: error: no member named 'dbId' in 'struct CkptSortItem'
> 3598 | if (item.dbId != batch.rlocator.dbOid)
> | ~~~~ ^
> 2 errors generated.
> make[4]: *** [bufmgr.o] Error 1
> ```
>
> I tried “make clean” and “make” again, which didn’t work. I think the error is introduced by 0006.
Are you sure you applied 0005? It is the one that adds dbId to CkptSortItem.
- Melanie
| Attachment | Content-Type | Size |
|---|---|---|
| v10-0001-Refactor-goto-into-for-loop-in-GetVictimBuffer.patch | text/x-patch | 12.2 KB |
| v10-0002-Split-FlushBuffer-into-two-parts.patch | text/x-patch | 7.7 KB |
| v10-0003-Eagerly-flush-bulkwrite-strategy-ring.patch | text/x-patch | 12.7 KB |
| v10-0004-Write-combining-for-BAS_BULKWRITE.patch | text/x-patch | 16.4 KB |
| v10-0005-Add-database-Oid-to-CkptSortItem.patch | text/x-patch | 1.9 KB |
| v10-0006-Implement-checkpointer-data-write-combining.patch | text/x-patch | 10.9 KB |
| v10-0007-Refactor-SyncOneBuffer-for-bgwriter-use-only.patch | text/x-patch | 6.1 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Peter Smith | 2025-11-19 22:18:59 | Re: CREATE/ALTER PUBLICATION improvements for syntax synopsis |
| Previous Message | Álvaro Herrera | 2025-11-19 22:03:59 | Re: Consistently use the XLogRecPtrIsInvalid() macro |