On Mon, Jan 17, 2011 at 1:43 PM, Chris Browne <cbbrowne@acm.org> wrote:
> (I observe that it wasn't all that obvious that "hash_search()"
> *adds* elements that are missing. I got confused and went
> looking for "hash_add() or similar. It's permissible to say "dumb
> Chris".)
I didn't invent that API. It is a little strange, but you get the hang of it.
> Question: Is there any further cleanup needed for the entries
> that got "dropped" out of BGWriterShmem->requests? It seems
> not, but a leak seems conceivable.
They're just holding integers, so what's to clean up?
> - Concurrent access...
>
> Is there anything that can write extra elements to
> BGWriterShmem->requests while this is running?
>
> I wonder if we need to have any sort of lock surrounding this?
It's called with the BgWriterCommLock already held, and there's an
assertion to this effect as well.
> With higher shared memory, I couldn't readily induce compaction,
> which is probably a concurrency matter of not having enough volume
> of concurrent work going on.
You can do it artificially by attaching gdb to the bgwriter.
> In principle, the only case where it should worsen performance
> is if the amount of time required to:
> - Set up a hash table
> - Insert an entry for each buffer
> - Walk the skip_slot array, shortening the request queue
> for each duplicate found
> exceeded the amount of time required to do the duplicate fsync()
> requests.
>
> That cost should be mighty low. It would be interesting to
> instrument CompactBgwriterRequestQueue() to see how long it runs.
>
> But note that this cost is also spread into a direction where it
> likely wouldn't matter, as it is typically invoked by the
> background writer process, so this would frequently not be paid
> by "on-line" active processes.
This is not correct. The background writer only ever does
AbsorbFsyncRequests(); this would substitute (to some degree) in cases
where the background writer fell down on the job.
> bgwriter.c: In function 'CompactBgwriterRequestQueue':
> bgwriter.c:1142: warning: 'num_skipped' may be used uninitialized in this function
OK, I can fix that.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
