pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
pgsql: Address set of issues with errno handling
11+ messages / 2 participants
[nested] [flat]

Commits from this thread:
  2018-08-04 5a23c74b63 58673b4a5f 7124e64520 f5b4bb8812 afd5fde856 e69a3ac4a3 69599cc49e  Reset properly errno before calling write()
* pgsql: Address set of issues with errno handling
@ 2018-06-25 02:23 Michael Paquier <michael@paquier.xyz>
  2018-08-03 20:04 ` Re: pgsql: Address set of issues with errno handling Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 11+ messages in thread

From: Michael Paquier @ 2018-06-25 02:23 UTC (permalink / raw)
  To: pgsql-committers@lists.postgresql.org

Address set of issues with errno handling

System calls mixed up in error code paths are causing two issues which
several code paths have not correctly handled:
1) For write() calls, sometimes the system may return less bytes than
what has been written without errno being set.  Some paths were careful
enough to consider that case, and assumed that errno should be set to
ENOSPC, other calls missed that.
2) errno generated by a system call is overwritten by other system calls
which may succeed once an error code path is taken, causing what is
reported to the user to be incorrect.

This patch uses the brute-force approach of correcting all those code
paths.  Some refactoring could happen in the future, but this is let as
future work, which is not targeted for back-branches anyway.

Author: Michael Paquier
Reviewed-by: Ashutosh Sharma
Discussion: https://postgr.es/m/20180622061535.GD5215@paquier.xyz

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/6cb3372411fd6ed8ba0f8d36ae578a2daa517c16

Modified Files
--------------
src/backend/access/heap/rewriteheap.c           |  5 ++++
src/backend/access/transam/twophase.c           | 23 +++++++++++++++++
src/backend/access/transam/xlog.c               |  7 +++++
src/backend/access/transam/xlogutils.c          |  6 +++--
src/backend/replication/basebackup.c            |  3 +++
src/backend/replication/logical/origin.c        | 15 +++++++++++
src/backend/replication/logical/reorderbuffer.c |  4 ++-
src/backend/replication/logical/snapbuild.c     | 20 +++++++++++++++
src/backend/replication/slot.c                  |  7 ++++-
src/bin/pg_basebackup/walmethods.c              | 34 +++++++++++++++++++++++--
10 files changed, 118 insertions(+), 6 deletions(-)



^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: pgsql: Address set of issues with errno handling
  2018-06-25 02:23 pgsql: Address set of issues with errno handling Michael Paquier <michael@paquier.xyz>
@ 2018-08-03 20:04 ` Tom Lane <tgl@sss.pgh.pa.us>
  2018-08-04 12:18   ` Re: pgsql: Address set of issues with errno handling Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 11+ messages in thread

From: Tom Lane @ 2018-08-03 20:04 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: pgsql-committers@lists.postgresql.org

Michael Paquier <michael@paquier.xyz> writes:
> Address set of issues with errno handling

I happened to look at this patch while working up the release notes,
and I don't think it's right at all for this aspect:

> 1) For write() calls, sometimes the system may return less bytes than
> what has been written without errno being set.  Some paths were careful
> enough to consider that case, and assumed that errno should be set to
> ENOSPC, other calls missed that.

AFAICS, what you did about that was like this:

@@ -816,7 +828,12 @@ tar_close(Walfile f, WalCloseMethod method)
    if (!tar_data->compression)
    {
        if (write(tar_data->fd, tf->header, 512) != 512)
+       {
+           /* if write didn't set errno, assume problem is no disk space */
+           if (errno == 0)
+               errno = ENOSPC;
            return -1;
+       }
    }
 #ifdef HAVE_LIBZ
    else

That's not good enough, because there is no reason to suppose that errno
is initially zero; in reality it'll be whatever was left over from the
last failed syscall, perhaps far distant from here.  The places that do
this correctly do it like so:

        errno = 0;
        if (write(...) != expected-length)
        {
            /* if write didn't set errno, assume problem is no disk space */
            if (errno == 0)
                errno = ENOSPC;
            ...
        }

You need to go back and add the pre-clearing of errno in each of these
places, otherwise the added code is basically useless.

			regards, tom lane




^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: pgsql: Address set of issues with errno handling
  2018-06-25 02:23 pgsql: Address set of issues with errno handling Michael Paquier <michael@paquier.xyz>
  2018-08-03 20:04 ` Re: pgsql: Address set of issues with errno handling Tom Lane <tgl@sss.pgh.pa.us>
@ 2018-08-04 12:18   ` Michael Paquier <michael@paquier.xyz>
  2018-08-04 15:21     ` Re: pgsql: Address set of issues with errno handling Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 11+ messages in thread

From: Michael Paquier @ 2018-08-04 12:18 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-committers@lists.postgresql.org

On Fri, Aug 03, 2018 at 04:04:36PM -0400, Tom Lane wrote:
> That's not good enough, because there is no reason to suppose that errno
> is initially zero; in reality it'll be whatever was left over from the
> last failed syscall, perhaps far distant from here.  The places that do
> this correctly do it like so:
>
> [... code ...]

Yes, I have visibly assumed that the last syscall was already failing
so those conditions would not be hit.  If the code gets changed again
that would be a problem.

> You need to go back and add the pre-clearing of errno in each of these
> places, otherwise the added code is basically useless.

I looked at all code paths enforcing ENOSPC on write() calls, and
attached is a patch to address this issue for all of them.  What do you
think?  I can of course get that addressed before the next set of minor
releases.
--
Michael

Attachments:

errno-write-init.patch (text/x-diff, 5.0K) download diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index ed7ba181c7..85f92973c9 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -1166,6 +1166,7 @@ heap_xlog_logical_rewrite(XLogReaderState *r) len = xlrec->num_mappings * sizeof(LogicalRewriteMappingData); /* write out tail end of mapping file (again) */ + errno = 0; pgstat_report_wait_start(WAIT_EVENT_LOGICAL_REWRITE_MAPPING_WRITE); if (write(fd, data, len) != len) { diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index 306861bb79..0ae0722794 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -1669,6 +1669,7 @@ RecreateTwoPhaseFile(TransactionId xid, void *content, int len) errmsg("could not recreate file \"%s\": %m", path))); /* Write content and CRC */ + errno = 0; pgstat_report_wait_start(WAIT_EVENT_TWOPHASE_FILE_WRITE); if (write(fd, content, len) != len) { diff --git a/src/backend/replication/logical/origin.c b/src/backend/replication/logical/origin.c index 822c96d1c2..bf97dcdee4 100644 --- a/src/backend/replication/logical/origin.c +++ b/src/backend/replication/logical/origin.c @@ -576,6 +576,7 @@ CheckPointReplicationOrigin(void) tmppath))); /* write magic */ + errno = 0; if ((write(tmpfd, &magic, sizeof(magic))) != sizeof(magic)) { int save_errno = errno; @@ -619,6 +620,7 @@ CheckPointReplicationOrigin(void) /* make sure we only write out a commit that's persistent */ XLogFlush(local_lsn); + errno = 0; if ((write(tmpfd, &disk_state, sizeof(disk_state))) != sizeof(disk_state)) { @@ -641,6 +643,7 @@ CheckPointReplicationOrigin(void) /* write out the CRC */ FIN_CRC32C(crc); + errno = 0; if ((write(tmpfd, &crc, sizeof(crc))) != sizeof(crc)) { int save_errno = errno; diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c index 9b55b94227..1d43a165ad 100644 --- a/src/backend/replication/logical/reorderbuffer.c +++ b/src/backend/replication/logical/reorderbuffer.c @@ -2421,6 +2421,7 @@ ReorderBufferSerializeChange(ReorderBuffer *rb, ReorderBufferTXN *txn, ondisk->size = sz; + errno = 0; pgstat_report_wait_start(WAIT_EVENT_REORDER_BUFFER_WRITE); if (write(fd, rb->outbuf, ondisk->size) != ondisk->size) { diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 1359d9b20a..a6cd6c67d1 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -1609,6 +1609,7 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn) ereport(ERROR, (errmsg("could not open file \"%s\": %m", path))); + errno = 0; pgstat_report_wait_start(WAIT_EVENT_SNAPBUILD_WRITE); if ((write(fd, ondisk, needed_length)) != needed_length) { diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c index 6c36398058..19978d9a9e 100644 --- a/src/backend/replication/slot.c +++ b/src/backend/replication/slot.c @@ -1275,6 +1275,7 @@ SaveSlotToPath(ReplicationSlot *slot, const char *dir, int elevel) SnapBuildOnDiskChecksummedSize); FIN_CRC32C(cp.checksum); + errno = 0; pgstat_report_wait_start(WAIT_EVENT_REPLICATION_SLOT_WRITE); if ((write(fd, &cp, sizeof(cp))) != sizeof(cp)) { diff --git a/src/bin/pg_basebackup/walmethods.c b/src/bin/pg_basebackup/walmethods.c index fbfee05a5a..42e3f3023a 100644 --- a/src/bin/pg_basebackup/walmethods.c +++ b/src/bin/pg_basebackup/walmethods.c @@ -122,6 +122,7 @@ dir_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_ zerobuf = pg_malloc0(XLOG_BLCKSZ); for (bytes = 0; bytes < pad_to_size; bytes += XLOG_BLCKSZ) { + errno = 0; if (write(fd, zerobuf, XLOG_BLCKSZ) != XLOG_BLCKSZ) { int save_errno = errno; @@ -445,6 +446,7 @@ tar_write_compressed_data(void *buf, size_t count, bool flush) { size_t len = ZLIB_OUT_SIZE - tar_data->zp->avail_out; + errno = 0; if (write(tar_data->fd, tar_data->zlibOut, len) != len) { /* @@ -629,6 +631,7 @@ tar_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_ if (!tar_data->compression) { + errno = 0; if (write(tar_data->fd, tar_data->currentfile->header, 512) != 512) { save_errno = errno; @@ -829,6 +832,7 @@ tar_close(Walfile f, WalCloseMethod method) return -1; if (!tar_data->compression) { + errno = 0; if (write(tar_data->fd, tf->header, 512) != 512) { /* if write didn't set errno, assume problem is no disk space */ @@ -901,6 +905,7 @@ tar_finish(void) MemSet(zerobuf, 0, sizeof(zerobuf)); if (!tar_data->compression) { + errno = 0; if (write(tar_data->fd, zerobuf, sizeof(zerobuf)) != sizeof(zerobuf)) { /* if write didn't set errno, assume problem is no disk space */ @@ -933,6 +938,7 @@ tar_finish(void) { size_t len = ZLIB_OUT_SIZE - tar_data->zp->avail_out; + errno = 0; if (write(tar_data->fd, tar_data->zlibOut, len) != len) { /*
signature.asc (application/pgp-signature, 832B) download ^ permalink raw reply [nested|flat] 11+ messages in thread

* Re: pgsql: Address set of issues with errno handling
  2018-06-25 02:23 pgsql: Address set of issues with errno handling Michael Paquier <michael@paquier.xyz>
  2018-08-03 20:04 ` Re: pgsql: Address set of issues with errno handling Tom Lane <tgl@sss.pgh.pa.us>
  2018-08-04 12:18   ` Re: pgsql: Address set of issues with errno handling Michael Paquier <michael@paquier.xyz>
@ 2018-08-04 15:21     ` Tom Lane <tgl@sss.pgh.pa.us>
  2018-08-04 20:36       ` Re: pgsql: Address set of issues with errno handling Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 11+ messages in thread

From: Tom Lane @ 2018-08-04 15:21 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: pgsql-committers@lists.postgresql.org

Michael Paquier <michael@paquier.xyz> writes:
> On Fri, Aug 03, 2018 at 04:04:36PM -0400, Tom Lane wrote:
>> You need to go back and add the pre-clearing of errno in each of these
>> places, otherwise the added code is basically useless.

> I looked at all code paths enforcing ENOSPC on write() calls, and
> attached is a patch to address this issue for all of them.  What do you
> think?  I can of course get that addressed before the next set of minor
> releases.

That looks good as far as it goes.  I didn't cross-check that you
hit everyplace that needs this, but if you grepped for references to
ENOSPC then you presumably found them all.

IMO, it's OK to push this this weekend if you have time.

			regards, tom lane




^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: pgsql: Address set of issues with errno handling
  2018-06-25 02:23 pgsql: Address set of issues with errno handling Michael Paquier <michael@paquier.xyz>
  2018-08-03 20:04 ` Re: pgsql: Address set of issues with errno handling Tom Lane <tgl@sss.pgh.pa.us>
  2018-08-04 12:18   ` Re: pgsql: Address set of issues with errno handling Michael Paquier <michael@paquier.xyz>
  2018-08-04 15:21     ` Re: pgsql: Address set of issues with errno handling Tom Lane <tgl@sss.pgh.pa.us>
@ 2018-08-04 20:36       ` Michael Paquier <michael@paquier.xyz>
  2018-08-04 20:48         ` Re: pgsql: Address set of issues with errno handling Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 11+ messages in thread

From: Michael Paquier @ 2018-08-04 20:36 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-committers@lists.postgresql.org

On Sat, Aug 04, 2018 at 11:21:28AM -0400, Tom Lane wrote:
> That looks good as far as it goes.  I didn't cross-check that you
> hit everyplace that needs this, but if you grepped for references to
> ENOSPC then you presumably found them all.

That's exactly how the history goes.

> IMO, it's OK to push this this weekend if you have time.

OK, done now down to 9.3.
--
Michael

Attachments:

  signature.asc (application/pgp-signature, 832B) download

^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: pgsql: Address set of issues with errno handling
  2018-06-25 02:23 pgsql: Address set of issues with errno handling Michael Paquier <michael@paquier.xyz>
  2018-08-03 20:04 ` Re: pgsql: Address set of issues with errno handling Tom Lane <tgl@sss.pgh.pa.us>
  2018-08-04 12:18   ` Re: pgsql: Address set of issues with errno handling Michael Paquier <michael@paquier.xyz>
  2018-08-04 15:21     ` Re: pgsql: Address set of issues with errno handling Tom Lane <tgl@sss.pgh.pa.us>
  2018-08-04 20:36       ` Re: pgsql: Address set of issues with errno handling Michael Paquier <michael@paquier.xyz>
@ 2018-08-04 20:48         ` Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 0 replies; 11+ messages in thread

From: Tom Lane @ 2018-08-04 20:48 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: pgsql-committers@lists.postgresql.org

Michael Paquier <michael@paquier.xyz> writes:
> On Sat, Aug 04, 2018 at 11:21:28AM -0400, Tom Lane wrote:
>> IMO, it's OK to push this this weekend if you have time.

> OK, done now down to 9.3.

Thanks!

			regards, tom lane




^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* pgsql: Address set of issues with errno handling
@ 2018-06-25 02:23 Michael Paquier <michael@paquier.xyz>
  0 siblings, 0 replies; 11+ messages in thread

From: Michael Paquier @ 2018-06-25 02:23 UTC (permalink / raw)
  To: pgsql-committers@lists.postgresql.org

Address set of issues with errno handling

System calls mixed up in error code paths are causing two issues which
several code paths have not correctly handled:
1) For write() calls, sometimes the system may return less bytes than
what has been written without errno being set.  Some paths were careful
enough to consider that case, and assumed that errno should be set to
ENOSPC, other calls missed that.
2) errno generated by a system call is overwritten by other system calls
which may succeed once an error code path is taken, causing what is
reported to the user to be incorrect.

This patch uses the brute-force approach of correcting all those code
paths.  Some refactoring could happen in the future, but this is let as
future work, which is not targeted for back-branches anyway.

Author: Michael Paquier
Reviewed-by: Ashutosh Sharma
Discussion: https://postgr.es/m/20180622061535.GD5215@paquier.xyz

Branch
------
REL_10_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/6eec6724ff4519ff67c9d402d65ad8a6c27e1d0e

Modified Files
--------------
src/backend/access/heap/rewriteheap.c           |  5 ++++
src/backend/access/transam/twophase.c           | 23 +++++++++++++++++
src/backend/access/transam/xlog.c               |  7 +++++
src/backend/access/transam/xlogutils.c          |  4 +++
src/backend/replication/basebackup.c            |  3 +++
src/backend/replication/logical/origin.c        | 15 +++++++++++
src/backend/replication/logical/reorderbuffer.c |  4 ++-
src/backend/replication/logical/snapbuild.c     | 20 +++++++++++++++
src/backend/replication/slot.c                  |  7 ++++-
src/bin/pg_basebackup/walmethods.c              | 34 +++++++++++++++++++++++--
10 files changed, 118 insertions(+), 4 deletions(-)



^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* pgsql: Address set of issues with errno handling
@ 2018-06-25 02:23 Michael Paquier <michael@paquier.xyz>
  0 siblings, 0 replies; 11+ messages in thread

From: Michael Paquier @ 2018-06-25 02:23 UTC (permalink / raw)
  To: pgsql-committers@lists.postgresql.org

Address set of issues with errno handling

System calls mixed up in error code paths are causing two issues which
several code paths have not correctly handled:
1) For write() calls, sometimes the system may return less bytes than
what has been written without errno being set.  Some paths were careful
enough to consider that case, and assumed that errno should be set to
ENOSPC, other calls missed that.
2) errno generated by a system call is overwritten by other system calls
which may succeed once an error code path is taken, causing what is
reported to the user to be incorrect.

This patch uses the brute-force approach of correcting all those code
paths.  Some refactoring could happen in the future, but this is let as
future work, which is not targeted for back-branches anyway.

Author: Michael Paquier
Reviewed-by: Ashutosh Sharma
Discussion: https://postgr.es/m/20180622061535.GD5215@paquier.xyz

Branch
------
REL9_6_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/7fdf56b0a796f9e6f625c8fa236ece72b91b2270

Modified Files
--------------
src/backend/access/heap/rewriteheap.c           |  5 +++++
src/backend/access/transam/twophase.c           | 23 +++++++++++++++++++++++
src/backend/access/transam/xlog.c               |  7 +++++++
src/backend/access/transam/xlogutils.c          |  4 ++++
src/backend/replication/basebackup.c            |  3 +++
src/backend/replication/logical/origin.c        | 15 +++++++++++++++
src/backend/replication/logical/reorderbuffer.c |  4 +++-
src/backend/replication/logical/snapbuild.c     | 20 ++++++++++++++++++++
src/backend/replication/slot.c                  |  7 ++++++-
src/bin/pg_basebackup/receivelog.c              | 13 ++++++++++++-
10 files changed, 98 insertions(+), 3 deletions(-)



^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* pgsql: Address set of issues with errno handling
@ 2018-06-25 02:23 Michael Paquier <michael@paquier.xyz>
  0 siblings, 0 replies; 11+ messages in thread

From: Michael Paquier @ 2018-06-25 02:23 UTC (permalink / raw)
  To: pgsql-committers@lists.postgresql.org

Address set of issues with errno handling

System calls mixed up in error code paths are causing two issues which
several code paths have not correctly handled:
1) For write() calls, sometimes the system may return less bytes than
what has been written without errno being set.  Some paths were careful
enough to consider that case, and assumed that errno should be set to
ENOSPC, other calls missed that.
2) errno generated by a system call is overwritten by other system calls
which may succeed once an error code path is taken, causing what is
reported to the user to be incorrect.

This patch uses the brute-force approach of correcting all those code
paths.  Some refactoring could happen in the future, but this is let as
future work, which is not targeted for back-branches anyway.

Author: Michael Paquier
Reviewed-by: Ashutosh Sharma
Discussion: https://postgr.es/m/20180622061535.GD5215@paquier.xyz

Branch
------
REL9_5_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/910e2aca129cf0cda759c57920141c044aa1a8ba

Modified Files
--------------
src/backend/access/heap/rewriteheap.c           |  5 +++++
src/backend/access/transam/twophase.c           | 23 +++++++++++++++++++++++
src/backend/access/transam/xlog.c               |  7 +++++++
src/backend/replication/basebackup.c            |  3 +++
src/backend/replication/logical/origin.c        | 15 +++++++++++++++
src/backend/replication/logical/reorderbuffer.c |  4 +++-
src/backend/replication/logical/snapbuild.c     | 20 ++++++++++++++++++++
src/backend/replication/slot.c                  |  7 ++++++-
src/bin/pg_basebackup/receivelog.c              | 13 ++++++++++++-
9 files changed, 94 insertions(+), 3 deletions(-)



^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* pgsql: Address set of issues with errno handling
@ 2018-06-25 02:23 Michael Paquier <michael@paquier.xyz>
  0 siblings, 0 replies; 11+ messages in thread

From: Michael Paquier @ 2018-06-25 02:23 UTC (permalink / raw)
  To: pgsql-committers@lists.postgresql.org

Address set of issues with errno handling

System calls mixed up in error code paths are causing two issues which
several code paths have not correctly handled:
1) For write() calls, sometimes the system may return less bytes than
what has been written without errno being set.  Some paths were careful
enough to consider that case, and assumed that errno should be set to
ENOSPC, other calls missed that.
2) errno generated by a system call is overwritten by other system calls
which may succeed once an error code path is taken, causing what is
reported to the user to be incorrect.

This patch uses the brute-force approach of correcting all those code
paths.  Some refactoring could happen in the future, but this is let as
future work, which is not targeted for back-branches anyway.

Author: Michael Paquier
Reviewed-by: Ashutosh Sharma
Discussion: https://postgr.es/m/20180622061535.GD5215@paquier.xyz

Branch
------
REL9_4_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/79b5b101f99238a3f3cac84e6323895db41d8410

Modified Files
--------------
src/backend/access/heap/rewriteheap.c           |  5 +++++
src/backend/access/transam/twophase.c           | 23 +++++++++++++++++++++++
src/backend/access/transam/xlog.c               |  7 +++++++
src/backend/replication/basebackup.c            |  3 +++
src/backend/replication/logical/reorderbuffer.c |  4 +++-
src/backend/replication/logical/snapbuild.c     | 20 ++++++++++++++++++++
src/backend/replication/slot.c                  |  7 ++++++-
src/bin/pg_basebackup/receivelog.c              | 16 +++++++++++++++-
8 files changed, 82 insertions(+), 3 deletions(-)



^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* pgsql: Address set of issues with errno handling
@ 2018-06-25 02:23 Michael Paquier <michael@paquier.xyz>
  0 siblings, 0 replies; 11+ messages in thread

From: Michael Paquier @ 2018-06-25 02:23 UTC (permalink / raw)
  To: pgsql-committers@lists.postgresql.org

Address set of issues with errno handling

System calls mixed up in error code paths are causing two issues which
several code paths have not correctly handled:
1) For write() calls, sometimes the system may return less bytes than
what has been written without errno being set.  Some paths were careful
enough to consider that case, and assumed that errno should be set to
ENOSPC, other calls missed that.
2) errno generated by a system call is overwritten by other system calls
which may succeed once an error code path is taken, causing what is
reported to the user to be incorrect.

This patch uses the brute-force approach of correcting all those code
paths.  Some refactoring could happen in the future, but this is let as
future work, which is not targeted for back-branches anyway.

Author: Michael Paquier
Reviewed-by: Ashutosh Sharma
Discussion: https://postgr.es/m/20180622061535.GD5215@paquier.xyz

Branch
------
REL9_3_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/f53ed82b7feeb1eee7388a24aa792e1d1b33e650

Modified Files
--------------
src/backend/access/transam/twophase.c | 23 +++++++++++++++++++++++
src/backend/access/transam/xlog.c     |  7 +++++++
src/backend/replication/basebackup.c  |  3 +++
src/bin/pg_basebackup/receivelog.c    | 16 +++++++++++++++-
4 files changed, 48 insertions(+), 1 deletion(-)



^ permalink  raw  reply  [nested|flat] 11+ messages in thread


end of thread, other threads:[~2018-08-04 20:48 UTC | newest]

Thread overview: 11+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-06-25 02:23 pgsql: Address set of issues with errno handling Michael Paquier <michael@paquier.xyz>
2018-08-03 20:04 ` Tom Lane <tgl@sss.pgh.pa.us>
2018-08-04 12:18   ` Michael Paquier <michael@paquier.xyz>
2018-08-04 15:21     ` Tom Lane <tgl@sss.pgh.pa.us>
2018-08-04 20:36       ` Michael Paquier <michael@paquier.xyz>
2018-08-04 20:48         ` Tom Lane <tgl@sss.pgh.pa.us>
2018-06-25 02:23 pgsql: Address set of issues with errno handling Michael Paquier <michael@paquier.xyz>
2018-06-25 02:23 pgsql: Address set of issues with errno handling Michael Paquier <michael@paquier.xyz>
2018-06-25 02:23 pgsql: Address set of issues with errno handling Michael Paquier <michael@paquier.xyz>
2018-06-25 02:23 pgsql: Address set of issues with errno handling Michael Paquier <michael@paquier.xyz>
2018-06-25 02:23 pgsql: Address set of issues with errno handling Michael Paquier <michael@paquier.xyz>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox