From: | Christoph Berg <myon(at)debian(dot)org> |
---|---|
To: | Thomas Munro <thomas(dot)munro(at)gmail(dot)com> |
Cc: | Quan Zongliang <quanzongliang(at)yeah(dot)net>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Available disk space per tablespace |
Date: | 2025-03-15 12:09:13 |
Message-ID: | Z9Vt6QYesJry7209@msg.df7cb.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Re: Thomas Munro
> Hah, I see this in my local FreeBSD man page. I guess this might be a
> reference to POSIX's 100% get-out clause "it is unspecified whether
> all members of the statvfs structure have meaningful values on all
> file systems".
Yeah I could hear someone being annoyed by POSIX echoed in that
paragraph.
> system that can't report free space to the "df" command, so what are
> we supposed to do? We could perhaps add a note to the documentation
> that this field relies on the OS providing meaningful "avail" field in
> statvfs(), but it's hard to imagine. Maybe just defer that until
> someone shows up with a real report? So +1 from me, go for it, call
> statvfs() and don't worry.
I was reading looking into gnulib's wrapper around this - it's also
basically calling statvfs() except on assorted older systems.
https://github.com/coreutils/gnulib/blob/master/lib/fsusage.c#L114
Do we care about any of these?
AIX
OSF/1
2.6 < glibc/Linux < 2.6.36
glibc/Linux < 2.6, 4.3BSD, SunOS 4, \
Mac OS X < 10.4, FreeBSD < 5.0, \
NetBSD < 3.0, OpenBSD < 4.4k
SunOS 4.1.2, 4.1.3, and 4.1.3_U1
4.4BSD and older NetBSD
SVR3, old Irix
If not, then statvfs seems safe.
> I also pushed your patch to CI and triggered the NetBSD and OpenBSD
> tasks and they passed your sanity test, though that only checks that
> the reported some number > 1MB.
I thought about making that test "between 1MB and 10PB", but that
seemed silly - it's not testing much, and some day, someone will try
to run the test on a system where it will still fail.
> What's the rationale for not raising an error if the system call
> fails?
That's mirroring the behavior of calculate_tablespace_size() in the
same file. I thought that's to allow \db+ to succeed even if some of
the tablespaces are botched/missing/whatever. But now on closer
inspection, I see that db_dir_size() is erroring out on problems, it
just ignores the top-level directory missing. Fixed in the attached
patch.
\db+
FEHLER: XX000: could not statvfs directory "pg_tblspc/16384/PG_18_202503111": Zu viele Ebenen aus symbolischen Links
LOCATION: calculate_tablespace_avail, dbsize.c:373
But this is actually something I wanted to address in a follow-up
patch: Currently, non-superusers cannot run \db+ because they lack
CREATE on pg_global (but `\db+ pg_default` works). Should we rather
make pg_database_size and pg_database_avail return NULL for
insufficient permissions instead of throwing an error?
> If someone complains that it's showing -1, doesn't that mean
(-1 is translated to NULL for the SQL level.)
> we'll have to ask them to trace the system calls to figure out why, or
> if it's Windows, likely abandon all hope of ever knowing why? Should
> statvfs() retry on EINTR?
Hmm. Is looping on EINTR worth the trouble?
> Style nit: maybe ! instead of == false?
Changed.
> Nice feature.
Thanks!
Christoph
Attachment | Content-Type | Size |
---|---|---|
v4-0001-Add-pg_tablespace_avail-functions.patch | text/x-diff | 10.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Laurenz Albe | 2025-03-15 12:17:18 | Re: Available disk space per tablespace |
Previous Message | Álvaro Herrera | 2025-03-15 11:58:24 | Re: PG_CFLAGS rpath Passthrough Issue |