Neil Conway <neilc@samurai.com> writes:
> Tom Lane <tgl@sss.pgh.pa.us> writes:
>> The latter is a really bad idea IMHO. The io_in_progress lock can be
>> held for eons (in CPU terms) and should not be blocking people who
>> simply want to bump their refcount up and down.
> My reasoning was that the contention for the per-buffer meta data lock
> should be pretty low. The io_in_progress lock is held when we're
> either faulting a page in or flushing a page out. In the first case,
> we can't actually use the buffer no matter how we do the locking (its
> content is incomplete), so there's no effective loss in
> concurrency. In the second case, what kinds of concurrent activity can
> we allow on the buffer? (We can allow reads, of course, but I don't
> believe we can allow writes.)
True, there's no win in the read-busy case, but I think you
underestimate the value of the write-busy case. Multiple concurrent
readers are a very important consideration. In Postgres it is possible
for a reader to cause a write to occur (because it sets commit hint
bits, as per the SetBufferCommitInfoNeedsSave() business), and so you
could have a situation like
Reader pins page Reader examines some tuples Reader sets a commit bit and dirties page ...
Writerstarts write ... Reader examines some more tuples Reader unpins page Writer finishes
write
If the reader can't unpin until the writer is done, then we will have
foreground readers blocked on the background writer process, which is
exactly what we do not want.
>> I think both of these are ill-considered micro-optimization. How do you
>> know that the pin count can't exceed 64K? Consider recursive plpgsql
>> functions for a likely counterexample.
> Fair enough -- I couldn't conceive of an actual scenario in which
> a single backend would acquire > 64K pins on a single buffer, but I'll
> take your word that it's not so far fetched. However, there is still
> room for improvement, IMHO: on a machine with 64-bit longs, we're
> plainly allocating 4 bytes more than we need do. Any objection if I
> change these to arrays of int32?
That seems like a reasonable compromise.
regards, tom lane