On Tue, May 20, 2014 at 4:02 AM, Yuto HAYAMIZU <y.hayam...@gmail.com> wrote: > The failing assertion is for prohibiting memory allocation in a critical > section, which is introduced by commit 4a170ee9 on 2014-04-04. > > In my understanding, the root cause of the assertion failure is on-demand > allocation of lwlock_stats entry. For each LWLock, a lwlock_stats entry is > created at the first invocation of LWLockAcquire using MemoryContextAlloc. > If the first invocation is in a critical section, the assertion fails. > > For 'initdb' case I mentioned above, WALWriteLock locking in XLogFlush > function was the problem. > I also confirmed the assertion failure on starting postgres on a correctly > initialized database. In this case, locking CheckpointerCommLock in > AbsorbFsyncRequests function was the problem. > > ## A solution > > In order to avoid memory allocation during critical sections, lwlock_stats > hash table should be populated at the initialization of each process. > The attached patch populate lwlock_stats entries of MainLWLockArray at the > end of CreateLWLocks, InitProcess and InitAuxiliaryProcess. > > With this patch, all regression tests can be passed so far, but I think this > patch is not perfect because it does not cover LWLocks outside of > MainLWLockArray. I'm not sure where is the right place to initialize > lwlock_stats entries for that locks. So I feel it needs some refinements by > you hackers.
Prior to my commit ea9df812d8502fff74e7bc37d61bdc7d66d77a7f, which introduced LWLockTranche, we used to allocate enough storage for all of the LWLOCK_STATS entries in the system; that commit changed things so that we allocate entries for particular LWLocks on an as-needed basis. Although that wasn't the main point of that patch, I thought it was a nice idea, since it might save you quite a bit of memory if you have a lot of backends that don't touch very many LWLocks. But maybe we need to give up on that in view of this report. I don't think we should adopt the approach proposed in this patch, though, because if we're going to preallocate all of the entries anyway there's little reason to use a hash table instead of an array. If we're going to go with the approach of preallocating all the entries, maybe we should change the definition of LWLockTranche to include the number of lwlocks in the tranche. We could then add another array parallel to LWLockTrancheArray which would point to an appropriately-sized array of lwlock_stats objects for each tranche. -- 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
