On Wed, 3 Jul 2019 at 19:35, Michael Paquier <mich...@paquier.xyz> wrote: > > On Tue, Jul 02, 2019 at 01:26:26AM +1200, David Rowley wrote: > > I've pushed the original patch plus a small change to only call > > table_finish_bulk_insert() for the target of the copy when we're using > > bulk inserts. > > Yes, sorry for coming late at the party here. What I meant previously > is that I did not find the version published at [1] to be natural with > its structure to define an executor callback which then calls a > callback for table AMs, still I get your point that it would be better > to try to avoid unnecessary fsync calls on partitions where no tuples > have been redirected with a COPY. The version 1 of the patch attached > at [2] felt much more straight-forward and cleaner by keeping all the > table AM callbacks within copy.c. > > This has been reverted as of f5db56f, still it seems to me that this > was moving in the right direction.
I think the only objection to doing it the way [2] did was, if there are more than MAX_PARTITION_BUFFERS partitions then we may end up evicting the CopyMultiInsertBuffer out of the CopyMultiInsertInfo and thus cause a call to table_finish_bulk_insert() before we're done with the copy. It's not impossible that this could happen many times for a given partition. I agree that a working version of [2] is cleaner than [1] but it's just the thought of those needless calls. For [1], I wasn't very happy with the way it turned out which is why I ended up suggesting a few other ideas. I just don't really like either of them any better than [1], so I didn't chase those up, and that's why I ended up going for [2]. If you think any of the other ideas I suggested are better (apart from [2]) then let me know and I can see about writing a patch. Otherwise, I plan to just fix [2] and push. > [1]: > https://postgr.es/m/CAKJS1f95sB21LBF=1mcsev+xlta_jc3mtxx5kgduhdsogow...@mail.gmail.com > [2]: > https://postgr.es/m/cakjs1f_0t-k0_3xe+erxpq-jgaob6trzayercxf2rpgduvm...@mail.gmail.com -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
