On Mon, Jan 4, 2016 at 8:28 PM, Robert Haas <robertmh...@gmail.com> wrote: > > On Mon, Jan 4, 2016 at 1:20 AM, Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Mon, Jan 4, 2016 at 4:49 AM, Robert Haas <robertmh...@gmail.com> wrote: > >> On Thu, Dec 31, 2015 at 3:36 AM, Amit Kapila <amit.kapil...@gmail.com> > >> wrote: > >> > LWLock *LWLockAssignFromTranche(const char *tranche_name) will > >> > assign a lock for the specified tranche. This also ensures that no > >> > more than requested number of LWLocks can be assigned from a > >> > specified tranche. > >> > >> However, this seems like an awkward API, because if there are many > >> LWLocks you're going to need to look up the tranche name many times, > >> and then you're going to need to make an array of pointers to them > >> somehow, introducing a thoroughly unnecessary layer of indirection. > >> Instead, I suggest just having a function LWLockPadded > >> *GetLWLockAddinTranche(const char *tranche_name) that returns a > >> pointer to the base of the array. > > > > If we do that way, then user of API needs to maintain the interlock > > guarantee that the requested number of locks is same as allocations > > done from that tranche and also if it is not allocating all the > > LWLocks in one function, it needs to save the base address of the > > array and then allocate from it by maintaining some counter. > > I agree that looking up for tranche names multiple times is not good, > > if there are many number of lwlocks, but that is done at startup > > time and not at operation-level. Also if we look currently at > > the extensions in contrib, then just one of them allocactes one > > LWLock in this fashion, now there could be extnsions apart from > > extensions in contrib, but not sure if they require many number of > > LWLocks, so I feel it is better to give an API which is more > > convinient for user to use. > > Well, I agree with you that we should provide the most convenient API > possible, but I don't agree that yours is more convenient than the one > I proposed. I think it is less convenient. In most cases, if the > user asks for a large number of LWLocks, they aren't going to be each > for a different purpose - they will all be for the same purpose, like > one per buffer or one per hash partition. The case where the user > wants to allocate 8 lwlocks from an extension, each for a different > purpose, and spread those allocations across a bunch of different > functions probably does not exist.
Probably, but the point is to make user of API do what he or she wants to accomplish without much knowledge of underlying stuff. However, I think it is not too much details for user to know, so I have changed the API as per your suggestion. > > *Maybe* there is somebody > allocating lwlocks from an extension for unrelated purposes, but I'd > be willing to bet that, if so, all of those allocations happen one > right after the other in a single function, because anything else > would be completely nuts. > > >> > Also I have retained NUM_USER_DEFINED_LWLOCKS in > >> > MainLWLockArray so that if any extensions want to assign a LWLock > >> > after startup, it can be used from this pool. The tranche for such > >> > locks > >> > will be reported as main. > >> > > I'd be interested in knowing whether there are cases where useful > extensions can be loaded apart from shared_preload_libraries because > of NUM_USER_DEFINED_LWLOCKS and our tendency to allocate a little bit > of extra shared memory, but my suspicion is that it rarely works out > and is too flaky to be useful to anybody. > I am not aware of such cases, however the reason I have kept it was for backward-compatability, but now I have removed it in the attached patch. Apart from that, I have updated the docs to reflect the changes related to new API's. Fe things to Note - Some change is needed in LWLockCounter[1] if this goes after 2 other patches (separate tranche for PGProcs and separate tranche for ReplicationSlots). Also, LWLockAssign() can be removed after those patches With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
separate_tranche_extensions_v2.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
