public inbox for [email protected]  

help / color / mirror / Atom feed

From: Long Song <[email protected]>
To: pgsql-hackers <[email protected]>
Subject: Re:[PATCH]A minor improvement to the error-report in SimpleLruWriteAll()
Date: Tue, 28 May 2024 20:15:59 +0800 (CST)
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>


Hi,
Actually, I still wonder why only the error message
of the last failure to close the file was recorded.
For this unusual situation, it is acceptable to
record all failure information without causing
too much logging.
Was it designed that way on purpose?




At 2024-05-25 17:29:00, "Long Song" <[email protected]> wrote:



Hi hackers,
When I read the code, I noticed that in SimpleLruWriteAll(), only the last error is 
recorded when the file fails to close. Like the following,
```void
SimpleLruWriteAll(SlruCtl ctl, bool allow_redirtied)
{
        SlruShared      shared = ctl->shared;
        SlruWriteAllData fdata;
        int64           pageno = 0;
        int                     prevbank = SlotGetBankNumber(0);
        bool            ok;
...
        /*
         * Now close any files that were open
         */
        ok = true;
        for (int i = 0; i < fdata.num_files; i++)
        {
                if (CloseTransientFile(fdata.fd[i]) != 0)
                {
                        slru_errcause = SLRU_CLOSE_FAILED;
                        slru_errno = errno;
                        pageno = fdata.segno[i] * SLRU_PAGES_PER_SEGMENT;
                        ok = false;
                }
        }
        if (!ok)
                SlruReportIOError(ctl, pageno, InvalidTransactionId);
```
// Here, SlruReportIOError() is called only once, meaning that the last error message is
 recorded. In my opinion, since failure to close a file is not common, logging an error 
message every time a failure occurs will not result in much log growth, but detailed error 
logging will help in most cases.

So, I changed the code to move the call to SlruReportIOError() inside the while loop.

Attached is the patch, I hope it can help.

view thread (2+ messages)

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]
  Subject: Re:[PATCH]A minor improvement to the error-report in SimpleLruWriteAll()
  In-Reply-To: <[email protected]>

* 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