Re: dsm_unpin_segment

On Wed, Aug 10, 2016 at 10:38 AM, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com> wrote:
> On 8/9/16 1:01 PM, Robert Haas wrote:
>>
>> However, I don't see the need for a full-blown request
>> counter here; we've had this API for several releases now and to my
>> knowledge nobody has complained about the fact that you aren't
>> supposed to call dsm_pin_segment() multiple times for the same
>> segment.
>
>
> Could a couple of static variables be used to ensure multiple pin/unpin and
> attach/detach calls throw an assert() (or become a no-op if asserts are
> disabled)? It would be nice if we could protect users from this.

The can't be static, they need to be in shared memory, because we also
want to protect against two *different* backends pinning it. The v2
patch protects users from this, by adding 'pinned' flag to the
per-segment control object that is visible everywhere, and then doing:

+ if (dsm_control->item[seg->control_slot].pinned)
+ elog(ERROR, "cannot pin a segment that is already pinned");

... and:

+ if (!dsm_control->item[i].pinned)
+ elog(ERROR, "cannot unpin a segment that is not pinned");

Those checks could arguably be assertions rather than errors, but I
don't think that pin/unpin operations will ever be high frequency
enough for it to be worth avoiding those instructions in production
builds. The whole reason for pinning segments is likely to be able to
reuse them repeatedly, so nailing it down is likely something you'd
want to do immediately after creating it.

--
Thomas Munro
http://www.enterprisedb.com