Re: pg_ls_tmpdir to show directories and shared filesets (and pg_ls_*)

From: Justin Pryzby <pryzby(at)telsasoft(dot)com>
To: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, David Steele <david(at)pgmasters(dot)net>, pgsql-hackers(at)postgresql(dot)org, "Bossart, Nathan" <bossartn(at)amazon(dot)com>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Subject: Re: pg_ls_tmpdir to show directories and shared filesets (and pg_ls_*)
Date: 2020-03-13 13:12:32
Message-ID: 20200313131232.GO29065@telsasoft.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

@cfbot: rebased onto 085b6b6679e73b9b386f209b4d625c7bc60597c0

The merge conflict presents another opportunity to solicit comments on the new
approach. Rather than making "recurse into tmpdir" the end goal:

- add a function to show metadata of an arbitrary dir;
- add isdir arguments to pg_ls_* functions (including pg_ls_tmpdir but not
pg_ls_dir).
- maybe add pg_ls_dir_recurse, which satisfies the original need;
- retire pg_ls_dir (does this work with tuplestore?)
- profit

The alternative seems to be to go back to Alvaro's earlier proposal:
- not only add "isdir", but also recurse;

I think I would insist on adding a general function to recurse into any dir.
And *optionally* change ps_ls_* to recurse (either by accepting an argument, or
by making that a separate patch to debate). tuplestore is certainly better
than keeping a stack/List of DIRs for this.

On Tue, Mar 10, 2020 at 01:30:37PM -0500, Justin Pryzby wrote:
> I took a step back, and I wondered whether we should add a generic function for
> listing a dir with metadata, possibly instead of changing the existing
> functions. Then one could do pg_ls_dir_metadata('pg_wal',false,false);
>
> Since pg8.1, we have pg_ls_dir() to show a list of files. Since pg10, we've
> had pg_ls_logdir and pg_ls_waldir, which show not only file names but also
> (some) metadata (size, mtime). And since pg12, we've had pg_ls_tmpfile and
> pg_ls_archive_statusdir, which also show metadata.
>
> ...but there's no a function which lists the metadata of an directory other
> than tmp, wal, log.
>
> One can do this:
> |SELECT b.*, c.* FROM (SELECT 'base' a)a, LATERAL (SELECT a||'/'||pg_ls_dir(a.a)b)b, pg_stat_file(b)c;
> ..but that's not as helpful as allowing:
> |SELECT * FROM pg_ls_dir_metadata('.',true,true);
>
> There's also no function which recurses into an arbitrary directory, so it
> seems shortsighted to provide a function to recursively list a tmpdir.
>
> Also, since pg_ls_dir_metadata indicates whether the path is a dir, one can
> write a SQL function to show the dir recursively. It'd be trivial to plug in
> wal/log/tmp (it seems like tmpdirs of other tablespace's are not entirely
> trivial).
> |SELECT * FROM pg_ls_dir_recurse('base/pgsql_tmp');
>
> Also, on a neighboring thread[1], Tom indicated that the pg_ls_* functions
> should enumerate all files during the initial call, which sounds like a bad
> idea when recursively showing directories. If we add a function recursing into
> a directory, we'd need to discuss all the flags to expose to it, like recurse,
> ignore_errors, one_filesystem?, show_dotfiles (and eventually bikeshed all the
> rest of the flags in find(1)).
>
> My initial patch [2] changed ls_tmpdir to show metadata columns including
> is_dir, but not decend. It's pretty unfortunate if a function called
> pg_ls_tmpdir hides shared filesets, so maybe it really is best to change that
> (it's new in v12).
>
> I'm interested to in feedback on the alternative approach, as attached. The
> final patch to include all the rest of columns shown by pg_stat_file() is more
> of an idea/proposal and not sure if it'll be desirable. But pg_ls_tmpdir() is
> essentially the same as my v1 patch.
>
> This is intended to be mostly independent of any fix to the WARNING I reported
> [1]. Since my patch collapses pg_ls_dir into pg_ls_dir_files, we'd only need
> to fix one place. I'm planning to eventually look into Tom's suggestion of
> returning tuplestore to fix that, and maybe rebase this patchset on top of
> that.
>
> --
> Justin
>
> [1] https://www.postgresql.org/message-id/flat/20200308173103.GC1357%40telsasoft.com
> [2] https://www.postgresql.org/message-id/20191214224735.GA28433%40telsasoft.com

Attachment Content-Type Size
v10-0001-Document-historic-behavior-about-hiding-director.patch text/x-diff 1.7 KB
v10-0002-Document-historic-behavior-about-hiding-director.patch text/x-diff 906 bytes
v10-0003-Add-tests-exercizing-pg_ls_tmpdir.patch text/x-diff 4.3 KB
v10-0004-Add-pg_ls_dir_metadata-to-list-a-dir-with-file-m.patch text/x-diff 9.9 KB
v10-0005-pg_ls_tmpdir-to-show-isdir-argument.patch text/x-diff 7.6 KB
v10-0006-pg_ls_-dir-to-show-directories-and-isdir-column.patch text/x-diff 12.8 KB
v10-0007-Add-pg_ls_dir_recurse-to-show-dir-recursively.patch text/x-diff 2.7 KB
v10-0008-pg_ls_logdir-to-ignore-error-if-initial-top-dir-.patch text/x-diff 3.0 KB
v10-0009-pg_ls_-dir-to-return-all-the-metadata-from-pg_st.patch text/x-diff 15.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Victor Wagner 2020-03-13 13:16:10 Re: make check crashes on POWER8 machine
Previous Message Justin Pryzby 2020-03-13 12:43:59 Re: make check crashes on POWER8 machine