Heikki Linnakangas <heikki@enterprisedb.com> writes:
> Tom Lane wrote:
>> A point I have not figured out how to deal with is that in the patch as
>> given, UnpinBuffer needs to know the strategy; and getting it that info
>> would make the changes far more invasive. But the patch's behavior here
>> is pretty risky anyway, since the strategy global at the time of unpin
>> might have nothing to do with how it was set when the buffer was
>> acquired.
> It's assumed that the same strategy is used when unpinning, which is
> true for the current usage (and apparently needs to be documented).
I don't believe that for a moment. Even in the trivial heapscan case,
the last pin is typically dropped during a tupleslot clear operation
sometime after the scan itself has moved on to the next page. In a more
general context (such as parallel heapscan and indexscan on a rel) it's
certainly too risky to assume it.
> Normally buffers that are in the ring are recycled quickly, in which
> case the usage_count will always be increased from 0->1 in UnpinBuffer,
> regardless of the check. The check is there to avoid inflating the
> usage_count on buffers that happened to be already in shared_buffers.
A heapscan would pin the buffer only once and hence bump its count at
most once, so I don't see a big problem here. Also, I'd argue that
buffers that had a positive usage_count shouldn't get sucked into the
ring to begin with.
> If we figure out another way to deal with the COPY usage_count, maybe we
> could remove the check altogether. I've been thinking of changing COPY
> anyway so that it always kept the last page it inserted to pinned, to
> avoid the traffic to buffer manager on each tuple.
This is getting fairly invasive for a part of the patch you've not
justified at all yet...
> Let me know if you want me to make changes and submit a new version.
I can work on this stuff; please do the tests to show whether it's worth
handling COPY TO REL, and keep on with Jeff's patch.
regards, tom lane
