Re: [HACKERS] dsm_unpin_segment

2016-08-23 Thread Robert Haas
On Mon, Aug 22, 2016 at 10:04 AM, Amit Kapila wrote: > On Mon, Aug 22, 2016 at 6:06 PM, Thomas Munro > wrote: >> On Tue, Aug 23, 2016 at 12:07 AM, Amit Kapila >> wrote: >>> + int control_slot = -1; >>> ... >>> + if (control_slot == -1) >>> + elog(ERROR, "cannot unpin unknown segment handle"); >

Re: [HACKERS] dsm_unpin_segment

2016-08-22 Thread Amit Kapila
On Mon, Aug 22, 2016 at 6:06 PM, Thomas Munro wrote: > On Tue, Aug 23, 2016 at 12:07 AM, Amit Kapila wrote: >> + int control_slot = -1; >> ... >> + if (control_slot == -1) >> + elog(ERROR, "cannot unpin unknown segment handle"); >> >> Isn't it better to use INVALID_CONTROL_SLOT for control_slot a

Re: [HACKERS] dsm_unpin_segment

2016-08-22 Thread Thomas Munro
On Tue, Aug 23, 2016 at 12:07 AM, Amit Kapila wrote: > + int control_slot = -1; > ... > + if (control_slot == -1) > + elog(ERROR, "cannot unpin unknown segment handle"); > > Isn't it better to use INVALID_CONTROL_SLOT for control_slot and use > datatype as uint32 (same is used for dsm_segment->con

Re: [HACKERS] dsm_unpin_segment

2016-08-22 Thread Amit Kapila
On Mon, Aug 22, 2016 at 5:24 AM, Thomas Munro wrote: > On Sat, Aug 20, 2016 at 11:37 PM, Amit Kapila wrote: >> On Tue, Aug 9, 2016 at 10:07 AM, Thomas Munro >> wrote: >>> On Tue, Aug 9, 2016 at 12:53 PM, Tom Lane wrote: The larger picture here is that Robert is exhibiting a touching but >>

Re: [HACKERS] dsm_unpin_segment

2016-08-21 Thread Robert Haas
On Sun, Aug 21, 2016 at 7:54 PM, Thomas Munro wrote: > Here's the rationale I'm using: if it's helpful to programmers of > client code, especially client code that might include extensions, and > nowhere near a hot code path, then why not use elog rather than > Assert? I took inspiration for that

Re: [HACKERS] dsm_unpin_segment

2016-08-21 Thread Thomas Munro
On Sat, Aug 20, 2016 at 11:37 PM, Amit Kapila wrote: > On Tue, Aug 9, 2016 at 10:07 AM, Thomas Munro > wrote: >> On Tue, Aug 9, 2016 at 12:53 PM, Tom Lane wrote: >>> The larger picture here is that Robert is exhibiting a touching but >>> unfounded faith that extensions using this feature will co

Re: [HACKERS] dsm_unpin_segment

2016-08-20 Thread Amit Kapila
On Sat, Aug 20, 2016 at 6:13 PM, Robert Haas wrote: > On Sat, Aug 20, 2016 at 7:37 AM, Amit Kapila wrote: >> 2. >> + if (dsm_control->item[seg->control_slot].pinned) >> + elog(ERROR, "cannot pin a segment that is already pinned"); >> >> Shouldn't this be a user facing error (which means we should

Re: [HACKERS] dsm_unpin_segment

2016-08-20 Thread Robert Haas
On Sat, Aug 20, 2016 at 7:37 AM, Amit Kapila wrote: > 2. > + if (dsm_control->item[seg->control_slot].pinned) > + elog(ERROR, "cannot pin a segment that is already pinned"); > > Shouldn't this be a user facing error (which means we should use ereport)? Uh, certainly not. This can't happen becaus

Re: [HACKERS] dsm_unpin_segment

2016-08-20 Thread Amit Kapila
On Tue, Aug 9, 2016 at 10:07 AM, Thomas Munro wrote: > On Tue, Aug 9, 2016 at 12:53 PM, Tom Lane wrote: >> The larger picture here is that Robert is exhibiting a touching but >> unfounded faith that extensions using this feature will contain zero bugs. >> IMO there needs to be some positive defen

Re: [HACKERS] dsm_unpin_segment

2016-08-09 Thread Thomas Munro
On Wed, Aug 10, 2016 at 2:43 PM, Jim Nasby wrote: > On 8/9/16 6:14 PM, Thomas Munro wrote: >> The can't be static, they need to be in shared memory, because we also >> want to protect against two *different* backends pinning it. > > Right, this would strictly protect from it happening within a sin

Re: [HACKERS] dsm_unpin_segment

2016-08-09 Thread Jim Nasby
On 8/9/16 6:14 PM, Thomas Munro wrote: 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

Re: [HACKERS] dsm_unpin_segment

2016-08-09 Thread Thomas Munro
On Wed, Aug 10, 2016 at 10:38 AM, Jim Nasby 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 >> suppos

Re: [HACKERS] dsm_unpin_segment

2016-08-09 Thread Jim Nasby
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.

Re: [HACKERS] dsm_unpin_segment

2016-08-09 Thread Robert Haas
On Mon, Aug 8, 2016 at 8:53 PM, Tom Lane wrote: > Thomas Munro writes: >> Yeah, I was considering unbalanced pin/unpin requests to be a >> programming error. To be more defensive about that, how about I add a >> boolean 'pinned' to dsm_control_item, and elog(ERROR, ...) if it's not >> in the exp

Re: [HACKERS] dsm_unpin_segment

2016-08-08 Thread Thomas Munro
On Tue, Aug 9, 2016 at 12:53 PM, Tom Lane wrote: > The larger picture here is that Robert is exhibiting a touching but > unfounded faith that extensions using this feature will contain zero bugs. > IMO there needs to be some positive defense against mistakes in using the > pin/unpin API. As thing

Re: [HACKERS] dsm_unpin_segment

2016-08-08 Thread Tom Lane
Thomas Munro writes: > Yeah, I was considering unbalanced pin/unpin requests to be a > programming error. To be more defensive about that, how about I add a > boolean 'pinned' to dsm_control_item, and elog(ERROR, ...) if it's not > in the expected state when you try to pin or unpin? Well, what y

Re: [HACKERS] dsm_unpin_segment

2016-08-08 Thread Thomas Munro
On Tue, Aug 9, 2016 at 12:22 PM, Robert Haas wrote: > On Mon, Aug 8, 2016 at 7:33 PM, Tom Lane wrote: >> Thomas Munro writes: >>> Please find attached a patch to add a corresponding operation >>> 'dsm_unpin_segment'. This gives you a way to ask for the segment to >>> survive only until you deci

Re: [HACKERS] dsm_unpin_segment

2016-08-08 Thread Robert Haas
On Mon, Aug 8, 2016 at 7:33 PM, Tom Lane wrote: > Thomas Munro writes: >> DSM segments have a concept of 'pinning'. Normally, segments are >> destroyed when they are no longer mapped by any backend, using a >> reference counting scheme. If you call dsm_pin_segment(segment), that >> is disabled

Re: [HACKERS] dsm_unpin_segment

2016-08-08 Thread Tom Lane
Thomas Munro writes: > DSM segments have a concept of 'pinning'. Normally, segments are > destroyed when they are no longer mapped by any backend, using a > reference counting scheme. If you call dsm_pin_segment(segment), that > is disabled so that the segment won't be destroyed until the cluste

[HACKERS] dsm_unpin_segment

2016-08-08 Thread Thomas Munro
Hi hackers, DSM segments have a concept of 'pinning'. Normally, segments are destroyed when they are no longer mapped by any backend, using a reference counting scheme. If you call dsm_pin_segment(segment), that is disabled so that the segment won't be destroyed until the cluster is shut down.