From: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
---|---|
To: | Thomas Munro <thomas(dot)munro(at)gmail(dot)com> |
Cc: | Andres Freund <andres(at)anarazel(dot)de>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Direct I/O |
Date: | 2023-01-25 07:57:04 |
Message-ID: | CALj2ACVDZ1y8k8EZd54eUHoZeuMjqOpa6BNFqSkLpmWqwELwSA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Dec 22, 2022 at 7:34 AM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
>
> On Wed, Dec 14, 2022 at 5:48 PM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
> > 0001 -- David's palloc_aligned() patch https://commitfest.postgresql.org/41/3999/
> > 0002 -- I/O-align almost all buffers used for I/O
> > 0003 -- Add the GUCs
> > 0004 -- Throwaway hack to make cfbot turn the GUCs on
>
> David pushed the first as commit 439f6175, so here is a rebase of the
> rest. I also fixed a couple of thinkos in the handling of systems
> where we don't know how to do direct I/O. In one place I had #ifdef
> PG_O_DIRECT, but that's always defined, it's just that it's 0 on
> Solaris and OpenBSD, and the check to reject the GUC wasn't quite
> right on such systems.
Thanks. I have some comments on
v3-0002-Add-io_direct-setting-developer-only.patch:
1. I think we don't need to overwrite the io_direct_string in
check_io_direct so that show_io_direct can be avoided.
2. check_io_direct can leak the flags memory - when io_direct is not
supported or for an invalid list syntax or an invalid option is
specified.
I have addressed my review comments as a delta patch on top of v3-0002
and added it here as v1-0001-Review-comments-io_direct-GUC.txt.
Some comments on the tests added:
1. Is there a way to know if Direct IO for WAL and data has been
picked up programmatically? IOW, can we know if the OS page cache is
bypassed? I know an external extension pgfincore which can help here,
but nothing in the core exists AFAICS.
+is('10000', $node->safe_psql('postgres', 'select count(*) from t1'),
"read back from shared");
+is('10000', $node->safe_psql('postgres', 'select * from t2count'),
"read back from local");
+$node->stop('immediate');
2. Can we combine these two append_conf to a single statement?
+$node->append_conf('io_direct', 'data,wal,wal_init');
+$node->append_conf('shared_buffers', '64kB'); # tiny to force I/O
3. A nitpick: Can we split these queries multi-line instead of in a single line?
+$node->safe_psql('postgres', 'begin; create temporary table t2 as
select 1 as i from generate_series(1, 10000); update t2 set i = i;
insert into t2count select count(*) from t2; commit;');
4. I don't think we need to stop the node before the test ends, no?
+$node->stop;
+
+done_testing();
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachment | Content-Type | Size |
---|---|---|
v1-0001-Review-comments-io_direct-GUC.txt | text/plain | 3.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Katsuragi Yuta | 2023-01-25 08:15:10 | Re: [Proposal] Add foreign-server health checks infrastructure |
Previous Message | Kyotaro Horiguchi | 2023-01-25 07:56:17 | Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?) |