From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Andres Freund <andres@anarazel.de>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: logical decoding / rewrite map vs. maxAllocatedDescs
Date: Sat, 11 Aug 2018 15:43:55 +0200
Message-ID: <79a0c7f4-89fb-cf8d-82eb-5a7ca8e08d56@2ndquadrant.com> (raw)
In-Reply-To: <20180810211340.nlgyfa7jlmkjcs5m@alap3.anarazel.de>
References: <20180809174746.aa6rbpsu4zjipzif@alvherre.pgsql>
<b346c3eb-0e65-29ec-47db-0d9a46feaa2c@2ndquadrant.com>
<20180810211340.nlgyfa7jlmkjcs5m@alap3.anarazel.de>
On 08/10/2018 11:13 PM, Andres Freund wrote:
> On 2018-08-10 22:57:57 +0200, Tomas Vondra wrote:
>>
>>
>> On 08/09/2018 07:47 PM, Alvaro Herrera wrote:
>>> On 2018-Aug-09, Tomas Vondra wrote:
>>>
>>>> I suppose there are reasons why it's done this way, and admittedly the test
>>>> that happens to trigger this is a bit extreme (essentially running pgbench
>>>> concurrently with 'vacuum full pg_class' in a loop). I'm not sure it's
>>>> extreme enough to deem it not an issue, because people using many temporary
>>>> tables often deal with bloat by doing frequent vacuum full on catalogs.
>>>
>>> Actually, it seems to me that ApplyLogicalMappingFile is just leaking
>>> the file descriptor for no good reason. There's a different
>>> OpenTransientFile call in ReorderBufferRestoreChanges that is not
>>> intended to be closed immediately, but the other one seems a plain bug,
>>> easy enough to fix.
>>>
>>
>> Indeed. Adding a CloseTransientFile to ApplyLogicalMappingFile
>> solves the issue with hitting maxAllocatedDecs. Barring objections
>> I'll commit this shortly.
>
> Yea, that's clearly a bug. I've not seen a patch, so I can't quite
> formally sign off, but it seems fairly obvious.
>
I think the fix can be as simple as attached ... I'm mostly afk for the
weekend, so I'll commit & backpatch on Monday or so.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
[text/x-patch] reorderbuffer-close.patch (434B, ../79a0c7f4-89fb-cf8d-82eb-5a7ca8e08d56@2ndquadrant.com/2-reorderbuffer-close.patch)
download | inline diff:diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.cindex 47e669578f..1a1a0fa469 100644--- a/src/backend/replication/logical/reorderbuffer.c+++ b/src/backend/replication/logical/reorderbuffer.c@@ -3327,6 +3327,8 @@ ApplyLogicalMappingFile(HTAB *tuplecid_data, Oid relid, const char *fname)
new_ent->combocid = ent->combocid;
}
}
++ CloseTransientFile(fd);
}
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tomas.vondra@2ndquadrant.com, andres@anarazel.de, alvherre@2ndquadrant.com
Subject: Re: logical decoding / rewrite map vs. maxAllocatedDescs
In-Reply-To: <79a0c7f4-89fb-cf8d-82eb-5a7ca8e08d56@2ndquadrant.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
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