Re: [HACKERS] errno clobbering in reorderbuffer
Andres Freund wrote: > Not at all, thanks. Done, thanks both for the look-over. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] errno clobbering in reorderbuffer
On August 18, 2016 7:21:03 PM PDT, Alvaro Herrera wrote: >Andres Freund wrote: >> On 2016-08-18 19:06:02 -0300, Alvaro Herrera wrote: >> > While researching a customer issue with BDR I noticed that one >ereport() >> > call happens after clobbering errno, leading to the wrong strerror >being >> > reported. This patch fixes it by saving before calling >> > CloseTransientFile and restoring afterwards. >> > >> > I also threw in a missing errcode I noticed while looking for >similar >> > problems in the same file. >> > >> > This is to backpatch to 9.4. >> >> Makes sense - you're doing that or shall I? > >I am, if you don't mind. Not at all, thanks. -- Sent from my Android device with K-9 Mail. Please excuse my brevity. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] errno clobbering in reorderbuffer
Andres Freund wrote: > On 2016-08-18 19:06:02 -0300, Alvaro Herrera wrote: > > While researching a customer issue with BDR I noticed that one ereport() > > call happens after clobbering errno, leading to the wrong strerror being > > reported. This patch fixes it by saving before calling > > CloseTransientFile and restoring afterwards. > > > > I also threw in a missing errcode I noticed while looking for similar > > problems in the same file. > > > > This is to backpatch to 9.4. > > Makes sense - you're doing that or shall I? I am, if you don't mind. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] errno clobbering in reorderbuffer
Hi,
On 2016-08-18 19:06:02 -0300, Alvaro Herrera wrote:
> if (write(fd, rb->outbuf, ondisk->size) != ondisk->size)
> {
> + int save_errno = errno;
> +
> CloseTransientFile(fd);
> + errno = save_errno;
> ereport(ERROR,
> (errcode_for_file_access(),
>errmsg("could not write to data file for XID
> %u: %m",
Independent of this specific case I kinda wish we had a better way to
deal with exactly this pattern. I even wonder whether having a close
variant not clobbering errno might be worthwhile.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] errno clobbering in reorderbuffer
Alvaro Herrera writes: > While researching a customer issue with BDR I noticed that one ereport() > call happens after clobbering errno, leading to the wrong strerror being > reported. This patch fixes it by saving before calling > CloseTransientFile and restoring afterwards. > I also threw in a missing errcode I noticed while looking for similar > problems in the same file. Looks sane to me. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] errno clobbering in reorderbuffer
On 2016-08-18 19:06:02 -0300, Alvaro Herrera wrote: > While researching a customer issue with BDR I noticed that one ereport() > call happens after clobbering errno, leading to the wrong strerror being > reported. This patch fixes it by saving before calling > CloseTransientFile and restoring afterwards. > > I also threw in a missing errcode I noticed while looking for similar > problems in the same file. > > This is to backpatch to 9.4. Makes sense - you're doing that or shall I? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] errno clobbering in reorderbuffer
While researching a customer issue with BDR I noticed that one ereport()
call happens after clobbering errno, leading to the wrong strerror being
reported. This patch fixes it by saving before calling
CloseTransientFile and restoring afterwards.
I also threw in a missing errcode I noticed while looking for similar
problems in the same file.
This is to backpatch to 9.4.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 7ff6f9b..a6d44d5 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -2135,7 +2135,10 @@ ReorderBufferSerializeChange(ReorderBuffer *rb, ReorderBufferTXN *txn,
if (write(fd, rb->outbuf, ondisk->size) != ondisk->size)
{
+ int save_errno = errno;
+
CloseTransientFile(fd);
+ errno = save_errno;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not write to data file for XID %u: %m",
@@ -2864,7 +2867,8 @@ ApplyLogicalMappingFile(HTAB *tuplecid_data, Oid relid, const char *fname)
fd = OpenTransientFile(path, O_RDONLY | PG_BINARY, 0);
if (fd < 0)
ereport(ERROR,
-(errmsg("could not open file \"%s\": %m", path)));
+(errcode_for_file_access(),
+ errmsg("could not open file \"%s\": %m", path)));
while (true)
{
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
