On Mon, Jan 17, 2011 at 1:43 PM, Chris Browne <cbbro...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
