Re: Too many files in pg_replslot folder

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