Re: shared memory message queues

On Fri, Dec 20, 2013 at 1:11 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On 2013-10-31 12:21:31 -0400, Robert Haas wrote:
>> Patch #2, shm-toc-v1.patch, provides a facility for sizing a dynamic
>> shared memory segment before creation, and for dividing it up into
>> chunks after it's been created. It therefore serves a function quite
>> similar to RequestAddinShmemSpace, except of course that there is only
>> one main shared memory segment created at postmaster startup time,
>> whereas new dynamic shared memory segments can come into existence on
>> the fly; and it serves even more conspicuously the function of
>> ShmemIndex, which enables backends to locate particular data
>> structures within the shared memory segment. It is however quite a
>> bit simpler than the ShmemIndex mechanism: we don't need the same
>> level of extensibility here that we do for the main shared memory
>> segment, because a new extension need not piggyback on an existing
>> dynamic shared memory segment, but can create a whole segment of its
>> own.
>
> So, without the argument of having per-extension dsm segments, I'd say
> that a purely integer key sucks, because it's hard to manage and
> debug. This way it's still not too nice, but I don't see a all that good
> alternative.
>
> Comments about shm-toc-v1.patch:
>
> Since you're embedding spinlocks in struct shm_toc, this module will be
> in conflict with platforms that do --disable-spinlocks, since the number
> of spinlocks essentially needs to be predetermined there. I personally
> still think the solution to that is getting rid of --disable-spinlocks.

Yep. We can either deprecate --disable-spinlocks, or we can add an
API that is the reverse of S_INIT_LOCK().

> I vote for removing all the shm_toc_estimator() knowledge from the
> header, there seems little reason to expose it that way. That just
> exposes unneccessary details and makes fixes after releases harder
> (requiring extensions to recompile). Function call costs hardly can
> matter, right?

Oh, you're saying to make it a function rather than a macro? Sure, I
could do that.

> Do you assume that lookup speed isn't that important? I have a bit of a
> hard time imagining that linear search over the keys isn't going to bite
> us. At the least there should be a comment somewhere documenting that
> the indented usecase is having a relatively few keys.

Well, as previously noted, in the use cases I imagine for this, it'll
be nworkers + somesmallconstant. I can add a comment to that effect.

> Wouldn't it make sense to have a function that does the combined job of
> shm_toc_insert() and shm_toc_allocate()?

We could, but I don't really feel compelled. It's not hard to call
them individually if that's the behavior you happen to want.

> What's the assumption about the use of the magic in create/attach? Just
> statically defined things in user code?

Yeah.

> Ok, cooking now, then I'll have a look at the queue itself,

Thanks.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company