From: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
---|---|
To: | Thomas Munro <thomas(dot)munro(at)gmail(dot)com> |
Cc: | Nathan Bossart <nathandbossart(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Use pg_pwritev_with_retry() instead of write() in dir_open_for_write() to avoid partial writes? |
Date: | 2022-09-30 02:39:04 |
Message-ID: | CALj2ACWKjREewMTDPj_5sCXESdXAmN7d6njDpAtkE8MFr0ZeQA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Sep 30, 2022 at 6:46 AM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
>
> +1 for just doing it always, with a one-liner comment like
> "pg_pwritev*() might move the file position". No reason to spam the
> source tree with more explanations of the exact reason.
+1 for resetting the file position in a platform-independent manner.
But, a description there won't hurt IMO and it saves time for the
hackers who spend time there and think why it's that way.
> If someone
> ever comes up with another case where p- and non-p- I/O functions are
> intermixed and it's really worth saving a system call (don't get me
> wrong, we call lseek() an obscene amount elsewhere and I'd like to fix
> that, but this case isn't hot?) then I like your idea of a macro to
> tell you whether you need to.
I don't think we go that route as the code isn't a hot path and an
extra system call wouldn't hurt performance much, a comment there
should work.
> Earlier I wondered why we'd want to include "pg_pwritev" in the name
> of this zero-filling function (pwritev being an internal
> implementation detail), but now it seems like maybe a good idea
> because it highlights the file position portability problem by being a
> member of that family of similarly-named functions.
Hm.
On Thu, Sep 29, 2022 at 10:57 PM Nathan Bossart
<nathandbossart(at)gmail(dot)com> wrote:
>
> + iov[0].iov_base = zbuffer.data;
>
> This seems superfluous, but I don't think it's hurting anything.
Yes, I removed it. Adding a comment, something like [1], would make it
more verbose, hence I've not added.
I'm attaching the v6 patch set, please review it further.
[1]
/*
* Use the first vector buffer to write the remaining size. Note that
* zero buffer was already pointed to it above, hence just specifying
* the size is enough here.
*/
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachment | Content-Type | Size |
---|---|---|
v6-0001-Move-pg_pwritev_with_retry-to-file_utils.c.patch | application/x-patch | 6.1 KB |
v6-0002-Use-pg_pwritev_with_retry-instead-of-write-in-wal.patch | application/x-patch | 7.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | kuroda.hayato@fujitsu.com | 2022-09-30 02:39:40 | RE: Question: test "aggregates" failed in 32-bit machine |
Previous Message | Bruce Momjian | 2022-09-30 02:30:54 | Re: disfavoring unparameterized nested loops |