public inbox for [email protected]
help / color / mirror / Atom feedFrom: Nathan Bossart <[email protected]>
To: Thomas Munro <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Bharath Rupireddy <[email protected]>
Cc: Bossart, Nathan <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Julien Rouhaud <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Avoid erroring out when unable to remove or parse logical rewrite files to save checkpoint work
Date: Wed, 30 Mar 2022 09:21:30 -0700
Message-ID: <20220330162008.GA784267@nathanxps13> (raw)
In-Reply-To: <20220329224832.GA560657@nathanxps13>
References: <CALj2ACXGwc5UBx1Jc2wacqEM1oBmFHjRHxvTDA+X48sE7ObcFQ@mail.gmail.com>
<20220204000340.GA1164333@nathanxps13>
<CALj2ACWRbT2LxQq+7so3NdKest0beoYUaZShDy7OvojcstnQOA@mail.gmail.com>
<[email protected]>
<20220215175752.GA2413813@nathanxps13>
<[email protected]>
<20220215183758.GA2435469@nathanxps13>
<20220215231123.GA2584239@nathanxps13>
<CA+hUKGKFZ3GH8OXDDewamRNU5LBxkham=RVv3xTZd1kcqs5i-A@mail.gmail.com>
<20220329224832.GA560657@nathanxps13>
On Tue, Mar 29, 2022 at 03:48:32PM -0700, Nathan Bossart wrote:
> On Thu, Mar 24, 2022 at 01:17:01PM +1300, Thomas Munro wrote:
>> /* we're only handling directories here, skip if it's not ours */
>> - if (lstat(path, &statbuf) == 0 && !S_ISDIR(statbuf.st_mode))
>> + if (lstat(path, &statbuf) != 0)
>> + ereport(ERROR,
>> + (errcode_for_file_access(),
>> + errmsg("could not stat file \"%s\": %m", path)));
>> + else if (!S_ISDIR(statbuf.st_mode))
>> return;
>>
>> Why is this a good place to silently ignore non-directories?
>> StartupReorderBuffer() is already in charge of skipping random
>> detritus found in the directory, so would it be better to do "if
>> (get_dirent_type(...) != PGFILETYPE_DIR) continue" there, and then
>> drop the lstat() stanza from ReorderBufferCleanupSeralizedTXNs()
>> completely? Then perhaps its ReadDirExtended() shoud be using ERROR
>> instead of INFO, so that missing/non-dir/b0rked directories raise an
>> error.
>
> My guess is that this was done because ReorderBufferCleanupSerializedTXNs()
> is also called from ReorderBufferAllocate() and ReorderBufferFree().
> However, it is odd that we just silently return if the slot path isn't a
> directory in those cases. I think we could use get_dirent_type() in
> StartupReorderBuffer() as you suggested, and then we could let ReadDir()
> ERROR for non-directories for the other callers of
> ReorderBufferCleanupSerializedTXNs(). WDYT?
>
>> I don't understand why it's reporting readdir() errors at INFO
>> but unlink() errors at ERROR, and as far as I can see the other paths
>> that reach this code shouldn't be sending in paths to non-directories
>> here unless something is seriously busted and that's ERROR-worthy.
>
> I agree. I'll switch it to ReadDir() in the next revision so that we ERROR
> instead of INFO.
Here is an updated patch set.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
view thread (45+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Avoid erroring out when unable to remove or parse logical rewrite files to save checkpoint work
In-Reply-To: <20220330162008.GA784267@nathanxps13>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox