Re: Avoid erroring out when unable to remove or parse logical rewrite files to save checkpoint work
От | Nathan Bossart |
---|---|
Тема | Re: Avoid erroring out when unable to remove or parse logical rewrite files to save checkpoint work |
Дата | |
Msg-id | 20220330162008.GA784267@nathanxps13 обсуждение исходный текст |
Ответ на | Re: Avoid erroring out when unable to remove or parse logical rewrite files to save checkpoint work (Nathan Bossart <nathandbossart@gmail.com>) |
Ответы |
Re: Avoid erroring out when unable to remove or parse logical rewrite files to save checkpoint work
|
Список | pgsql-hackers |
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
Вложения
В списке pgsql-hackers по дате отправления: