Thread

  1. modify error report in mdwrite/mdextend

    suyu.cmj <mengjuan.cmj@alibaba-inc.com> — 2021-10-18T08:14:39Z

    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
    
  2. Re: modify error report in mdwrite/mdextend

    Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2021-10-18T08:29:00Z

    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.
    
    
    
    
  3. Re: modify error report in mdwrite/mdextend

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2021-10-18T14:45:28Z

    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)