public inbox for [email protected]
help / color / mirror / Atom feedFrom: Long Song <[email protected]>
To: pgsql-hackers <[email protected]>
Subject: [PATCH]A minor improvement to the error-report in SimpleLruWriteAll()
Date: Sat, 25 May 2024 18:29:00 +0800 (CST)
Message-ID: <[email protected]> (raw)
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.
Attachments:
[application/octet-stream] v1-0001-Fix-error-report-missing-of-SimpleLruWriteAll.patch (1.5K, ../[email protected]/3-v1-0001-Fix-error-report-missing-of-SimpleLruWriteAll.patch)
download | inline diff:
From 698d62e4f597d082a2213c03516f555df86eb1f1 Mon Sep 17 00:00:00 2001
From: solo <[email protected]>
Date: Sat, 25 May 2024 14:17:01 +0800
Subject: [PATCH v1] Fix the error-report missing in SimpleLruWriteAll
---
src/backend/access/transam/slru.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index 77b05cc0a..d0f0d68b3 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -1307,7 +1307,6 @@ SimpleLruWriteAll(SlruCtl ctl, bool allow_redirtied)
SlruWriteAllData fdata;
int64 pageno = 0;
int prevbank = SlotGetBankNumber(0);
- bool ok;
/* update the stats counter of flushes */
pgstat_count_slru_flush(shared->slru_stats_idx);
@@ -1356,7 +1355,6 @@ SimpleLruWriteAll(SlruCtl ctl, bool allow_redirtied)
/*
* Now close any files that were open
*/
- ok = true;
for (int i = 0; i < fdata.num_files; i++)
{
if (CloseTransientFile(fdata.fd[i]) != 0)
@@ -1364,11 +1362,9 @@ SimpleLruWriteAll(SlruCtl ctl, bool allow_redirtied)
slru_errcause = SLRU_CLOSE_FAILED;
slru_errno = errno;
pageno = fdata.segno[i] * SLRU_PAGES_PER_SEGMENT;
- ok = false;
+ SlruReportIOError(ctl, pageno, InvalidTransactionId);
}
}
- if (!ok)
- SlruReportIOError(ctl, pageno, InvalidTransactionId);
/* Ensure that directory entries for new files are on disk. */
if (ctl->sync_handler != SYNC_HANDLER_NONE)
--
2.43.0.windows.1
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