Subject: Re: Too many files in pg_replslot folder
To: Andres Freund <andres@anarazel.de>
References: <1457621358.355011041@f382.i.mail.ru>
 <20160310153412.GA53338@alvherre.pgsql> <1457684211.431682571@f351.i.mail.ru>
 <56E93E3B.8000104@bk.ru> <20160317232137.wdg726cwsea5tmwd@alap3.anarazel.de>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>,
 PostgreSQL Bugs <pgsql-bugs@postgresql.org>
From: Dmitriy Sarafannikov <d.sarafannikov@bk.ru>
Message-ID: <56EBB87C.3000606@bk.ru>
Date: Fri, 18 Mar 2016 11:12:44 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Icedove/38.6.0
MIME-Version: 1.0
In-Reply-To: <20160317232137.wdg726cwsea5tmwd@alap3.anarazel.de>
Content-Type: multipart/mixed;
 boundary="------------020303010708020908060706"
Precedence: bulk
Sender: pgsql-bugs-owner@postgresql.org

This is a multi-part message in MIME format.
--------------020303010708020908060706
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit


On 03/18/2016 02:21 AM, Andres Freund wrote:
> Hm, interesting. I wonder, could you try this with a source checkout 
> of 9.4? There's some fixes in the upcoming minor release that might be 
> related. If not, I'll look into it. - Andres 
Thank you for you attention!

It's reproducible for REL9_4_STABLE 
(17a250b189a94a470e37ce14d0ebf72390c86d4d).

I looked a little at the code. And i have reproduced this case with 
debugger in case of 5000 rows.

There was xact with 5000 subxacts and 5000 ->nentries.
Because 5000 > max_changes_in_memory (4096) then xact was spilled to 
disk partially.
Then in ReorderBufferCommit xact was spilled to disk with all their 
subxacts by
ReorderBufferSerializeTXN and had ->nentries = 5000, ->nentries_mem = 0.

     if (txn->nentries_mem != txn->nentries)
         ReorderBufferSerializeTXN(rb, txn);


Each subxact had ->nentries = 1 and ->nentries_mem = 1 before it was 
spilled and ->nentries = 1, ->nentries_mem = 0 after.

Then xact with all subxacts was restored in ReorderBufferIterTXNInit. 
And each subxact had ->nentries = 1 and ->nentries_mem = 1, and main 
xact had ->nentries = 5000, ->nentries_mem = 4096
after.

As i understood, it implied that all files will be deleted in 
ReorderBufferCleanupTXN
which recursively called for each subxact.
And for subxacts ReorderBufferRestoreCleanup has not been called because 
they had ->nentries = 1 == ->nentries_mem = 1.

     /* remove entries spilled to disk */
     if (txn->nentries != txn->nentries_mem)
         ReorderBufferRestoreCleanup(rb, txn);

I have a suggestion, but i'm not sure that it will be fully correct, 
because i don't know yet this logic fully:
add condition for subxact to "if"

     /* remove entries spilled to disk */
     if (txn->nentries != txn->nentries_mem || txn->is_known_as_subxact)
         ReorderBufferRestoreCleanup(rb, txn);


And, perhaps, i found a typo in ReorderBufferIterTXNInit:

     /* add subtransactions if they contain changes */
     dlist_foreach(cur_txn_i, &txn->subtxns)
     {
         ReorderBufferTXN *cur_txn;

         cur_txn = dlist_container(ReorderBufferTXN, node, cur_txn_i.cur);

         if (cur_txn->nentries > 0)
         {
             ReorderBufferChange *cur_change;

             if (txn->nentries != txn->nentries_mem)
                 ReorderBufferRestoreChanges(rb, cur_txn,
&state->entries[off].fd,
&state->entries[off].segno);

             cur_change = dlist_head_element(ReorderBufferChange, node,
&cur_txn->changes);

             state->entries[off].lsn = cur_change->lsn;
             state->entries[off].change = cur_change;
             state->entries[off].txn = cur_txn;

             binaryheap_add_unordered(state->heap, Int32GetDatum(off++));
         }
     }

maybe, it was implied as

             if (cur_txn->nentries != cur_txn->nentries_mem)
                 ReorderBufferRestoreChanges(rb, cur_txn,
&state->entries[off].fd,
&state->entries[off].segno);
instead of

             if (txn->nentries != txn->nentries_mem)
                 ReorderBufferRestoreChanges(rb, cur_txn,
&state->entries[off].fd,
&state->entries[off].segno);

Or i'm wrong?


Just in case, the patch for REL9_4_STABLE attached.

-- 
Best Regards,
Dmitriy Sarafannikov


--------------020303010708020908060706
Content-Type: text/x-patch;
 name="cleanup_subxacts_94.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="cleanup_subxacts_94.patch"

diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 1c8a554..aa984f9 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -895,7 +895,7 @@ ReorderBufferIterTXNInit(ReorderBuffer *rb, ReorderBufferTXN *txn)
 		{
 			ReorderBufferChange *cur_change;
 
-			if (txn->nentries != txn->nentries_mem)
+			if (cur_txn->nentries != cur_txn->nentries_mem)
 				ReorderBufferRestoreChanges(rb, cur_txn,
 											&state->entries[off].fd,
 											&state->entries[off].segno);
@@ -1114,7 +1114,7 @@ ReorderBufferCleanupTXN(ReorderBuffer *rb, ReorderBufferTXN *txn)
 	Assert(found);
 
 	/* remove entries spilled to disk */
-	if (txn->nentries != txn->nentries_mem)
+	if (txn->nentries != txn->nentries_mem || txn->is_known_as_subxact)
 		ReorderBufferRestoreCleanup(rb, txn);
 
 	/* deallocate */

--------------020303010708020908060706
Content-Type: text/plain
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

--------------020303010708020908060706--