Thread: modify error report in mdwrite/mdextend

modify error report in mdwrite/mdextend

From
"蔡梦娟(玊于)"
Date:

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

Attachment

Re: modify error report in mdwrite/mdextend

From
Bharath Rupireddy
Date:
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
erroris 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.



Re: modify error report in mdwrite/mdextend

From
Alvaro Herrera
Date:
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)