Hi,
On 2023-08-08 12:45:05 +0900, Masahiko Sawada wrote:
> > I think there could be a quite simple fix: Track by how much we've extended
> > the relation previously in the same bistate. If we already extended by many
> > blocks, it's very likey that we'll do so further.
> >
> > A simple prototype patch attached. The results for me are promising. I copied
> > a smaller file [1], to have more accurate throughput results at shorter runs
> > (15s).
>
> Thank you for the patch!
Attached is a mildly updated version of that patch. No functional changes,
just polished comments and added a commit message.
> > Any chance you could your benchmark? I don't see as much of a regression vs 16
> > as you...
>
> Sure. The results are promising for me too:
>
> nclients = 1, execution time = 13.743
> nclients = 2, execution time = 7.552
> nclients = 4, execution time = 4.758
> nclients = 8, execution time = 3.035
> nclients = 16, execution time = 2.172
> nclients = 32, execution time = 1.959
> nclients = 64, execution time = 1.819
> nclients = 128, execution time = 1.583
> nclients = 256, execution time = 1.631
Nice. We are consistently better than both 15 and "post integer parsing 16".
I'm really a bit baffled at myself for not using this approach from the get
go.
This also would make it much more beneficial to use a BulkInsertState in
nodeModifyTable.c, even without converting to table_multi_insert().
I was tempted to optimize RelationAddBlocks() a bit, by not calling
RelationExtensionLockWaiterCount() if we are already extending by
MAX_BUFFERS_TO_EXTEND_BY. Before this patch, it was pretty much impossible to
reach that case, because of the MAX_BUFFERED_* limits in copyfrom.c, but now
it's more common. But that probably ought to be done only HEAD, not 16.
A related oddity: RelationExtensionLockWaiterCount()->LockWaiterCount() uses
an exclusive lock on the lock partition - which seems not at all necessary?
Unless somebody sees a reason not to, I'm planning to push this?
Greetings,
Andres Freund