modify error report in mdwrite/mdextend

Started by 蔡梦娟(玊于)about 4 years ago3 messages
#1蔡梦娟(玊于)
mengjuan.cmj@alibaba-inc.com
1 attachment(s)
Hi, all
I noticed that the "else" is missing during the error report after FileWrite() of mdwrite()/mdextend(), short write error is supposed to be reported when written bytes is not less than 0.
I modified it in the attached patch:
diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c
index b4bca7eed6..dd60479b65 100644
--- a/src/backend/storage/smgr/md.c
+++ b/src/backend/storage/smgr/md.c
@@ -450,13 +450,14 @@ mdextend(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,
       errmsg("could not extend file \"%s\": %m",
        FilePathName(v->mdfd_vfd)),
       errhint("Check free disk space.")));
-  /* short write: complain appropriately */
-  ereport(ERROR,
-    (errcode(ERRCODE_DISK_FULL),
-     errmsg("could not extend file \"%s\": wrote only %d of %d bytes at block %u",
-      FilePathName(v->mdfd_vfd),
-      nbytes, BLCKSZ, blocknum),
-     errhint("Check free disk space.")));
+  else
+   /* short write: complain appropriately */
+   ereport(ERROR,
+     (errcode(ERRCODE_DISK_FULL),
+      errmsg("could not extend file \"%s\": wrote only %d of %d bytes at block %u",
+       FilePathName(v->mdfd_vfd),
+       nbytes, BLCKSZ, blocknum),
+      errhint("Check free disk space.")));
  }

Does this match your previous expectations? Hope to get your reply.
Thanks & Best Regard

Attachments:

0001-modify-error-report-in-mdwrite-mdextend.patchapplication/octet-streamDownload
From 59aad6be4ea3a7bf85b2d155a748dcc01be085d0 Mon Sep 17 00:00:00 2001
From: "suyu.cmj" <mengjuan.cmj@alibaba-inc.com>
Date: Mon, 18 Oct 2021 15:47:13 +0800
Subject: [PATCH] modify error report in mdwrite/mdextend

---
 src/backend/storage/smgr/md.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c
index b4bca7eed6..dd60479b65 100644
--- a/src/backend/storage/smgr/md.c
+++ b/src/backend/storage/smgr/md.c
@@ -450,13 +450,14 @@ mdextend(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,
 					 errmsg("could not extend file \"%s\": %m",
 							FilePathName(v->mdfd_vfd)),
 					 errhint("Check free disk space.")));
-		/* short write: complain appropriately */
-		ereport(ERROR,
-				(errcode(ERRCODE_DISK_FULL),
-				 errmsg("could not extend file \"%s\": wrote only %d of %d bytes at block %u",
-						FilePathName(v->mdfd_vfd),
-						nbytes, BLCKSZ, blocknum),
-				 errhint("Check free disk space.")));
+		else
+			/* short write: complain appropriately */
+			ereport(ERROR,
+					(errcode(ERRCODE_DISK_FULL),
+					 errmsg("could not extend file \"%s\": wrote only %d of %d bytes at block %u",
+							FilePathName(v->mdfd_vfd),
+							nbytes, BLCKSZ, blocknum),
+					 errhint("Check free disk space.")));
 	}
 
 	if (!skipFsync && !SmgrIsTemp(reln))
@@ -737,14 +738,15 @@ mdwrite(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,
 					(errcode_for_file_access(),
 					 errmsg("could not write block %u in file \"%s\": %m",
 							blocknum, FilePathName(v->mdfd_vfd))));
-		/* short write: complain appropriately */
-		ereport(ERROR,
-				(errcode(ERRCODE_DISK_FULL),
-				 errmsg("could not write block %u in file \"%s\": wrote only %d of %d bytes",
-						blocknum,
-						FilePathName(v->mdfd_vfd),
-						nbytes, BLCKSZ),
-				 errhint("Check free disk space.")));
+		else
+			/* short write: complain appropriately */
+			ereport(ERROR,
+					(errcode(ERRCODE_DISK_FULL),
+					 errmsg("could not write block %u in file \"%s\": wrote only %d of %d bytes",
+							blocknum,
+							FilePathName(v->mdfd_vfd),
+							nbytes, BLCKSZ),
+					 errhint("Check free disk space.")));
 	}
 
 	if (!skipFsync && !SmgrIsTemp(reln))
-- 
2.24.3 (Apple Git-128)

#2Bharath Rupireddy
bharath.rupireddyforpostgres@gmail.com
In reply to: 蔡梦娟(玊于) (#1)
Re: modify error report in mdwrite/mdextend

On Mon, Oct 18, 2021 at 1:45 PM 蔡梦娟(玊于) <mengjuan.cmj@alibaba-inc.com> wrote:

Hi, all
I noticed that the "else" is missing during the error report after FileWrite() of mdwrite()/mdextend(), short write error is supposed to be reported when written bytes is not less than 0.
I modified it in the attached patch:
diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c
index b4bca7eed6..dd60479b65 100644
--- a/src/backend/storage/smgr/md.c
+++ b/src/backend/storage/smgr/md.c
@@ -450,13 +450,14 @@ mdextend(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,
errmsg("could not extend file \"%s\": %m",
FilePathName(v->mdfd_vfd)),
errhint("Check free disk space.")));
-  /* short write: complain appropriately */
-  ereport(ERROR,
-    (errcode(ERRCODE_DISK_FULL),
-     errmsg("could not extend file \"%s\": wrote only %d of %d bytes at block %u",
-      FilePathName(v->mdfd_vfd),
-      nbytes, BLCKSZ, blocknum),
-     errhint("Check free disk space.")));
+  else
+   /* short write: complain appropriately */
+   ereport(ERROR,
+     (errcode(ERRCODE_DISK_FULL),
+      errmsg("could not extend file \"%s\": wrote only %d of %d bytes at block %u",
+       FilePathName(v->mdfd_vfd),
+       nbytes, BLCKSZ, blocknum),
+      errhint("Check free disk space.")));
}

Does this match your previous expectations? Hope to get your reply.

The control from the below ereport(ERROR, doesn't reach the short
write error part. IMO, the existing way does no harm, it is a mere
programming choice.

if (nbytes < 0)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not write block %u in file \"%s\": %m",
blocknum, FilePathName(v->mdfd_vfd))));
/* short write: complain appropriately */
ereport(ERROR,

Regards,
Bharath Rupireddy.

#3Alvaro Herrera
alvherre@alvh.no-ip.org
In reply to: Bharath Rupireddy (#2)
Re: modify error report in mdwrite/mdextend

On 2021-Oct-18, Bharath Rupireddy wrote:

On Mon, Oct 18, 2021 at 1:45 PM 蔡梦娟(玊于) <mengjuan.cmj@alibaba-inc.com> wrote:

Does this match your previous expectations? Hope to get your reply.

The control from the below ereport(ERROR, doesn't reach the short
write error part. IMO, the existing way does no harm, it is a mere
programming choice.

Yeah, this style is used extensively in many places of the backend code.

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"World domination is proceeding according to plan" (Andrew Morton)