Thread: flags argument for dsm_create
Discussion on the parallel sequential scan thread has revealed the need for a way to make dsm_create() return NULL, instead of failing, when we hit the system-wide maximum on the number of dynamic shared memory segments than can be created. I've developed a small patch for this which I attach here. It adds a second argument to dsm_create(), an integer flags argument. I would like to go ahead and commit this more or less immediately if there are not objections. One question I struggled with is whether to keep the existing dsm_create() signature intact and add a new function dsm_create_extended(). I eventually decided against it. The dsm_create() API is relatively new at this point, so there probably aren't too many people who will be inconvenienced by an API break now. If we go ahead and create dsm_create_extended() now, and then need to make another API change down the line, I doubt there will be much support for dsm_create_extended2() or whatever. So my gut is that it's better to just change this outright, and keep dsm_create_extended() as an idea for the future. But I could go the other way on that if people feel strongly about it. Thanks, -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Attachment
Hi, On 2015-03-19 11:21:45 -0400, Robert Haas wrote: > One question I struggled with is whether to keep the existing > dsm_create() signature intact and add a new function > dsm_create_extended(). I eventually decided against it. The > dsm_create() API is relatively new at this point, so there probably > aren't too many people who will be inconvenienced by an API break now. > If we go ahead and create dsm_create_extended() now, and then need > to make another API change down the line, I doubt there will be much > support for dsm_create_extended2() or whatever. So my gut is that > it's better to just change this outright, and keep > dsm_create_extended() as an idea for the future. But I could go the > other way on that if people feel strongly about it. +1 for a clear API break. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund wrote: > Hi, > > On 2015-03-19 11:21:45 -0400, Robert Haas wrote: > > One question I struggled with is whether to keep the existing > > dsm_create() signature intact and add a new function > > dsm_create_extended(). I eventually decided against it. The > > dsm_create() API is relatively new at this point, so there probably > > aren't too many people who will be inconvenienced by an API break now. > +1 for a clear API break. Seconded. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Thu, Mar 19, 2015 at 11:25 AM, Andres Freund <andres@2ndquadrant.com> wrote: > On 2015-03-19 11:21:45 -0400, Robert Haas wrote: >> One question I struggled with is whether to keep the existing >> dsm_create() signature intact and add a new function >> dsm_create_extended(). I eventually decided against it. The >> dsm_create() API is relatively new at this point, so there probably >> aren't too many people who will be inconvenienced by an API break now. >> If we go ahead and create dsm_create_extended() now, and then need >> to make another API change down the line, I doubt there will be much >> support for dsm_create_extended2() or whatever. So my gut is that >> it's better to just change this outright, and keep >> dsm_create_extended() as an idea for the future. But I could go the >> other way on that if people feel strongly about it. > > +1 for a clear API break. I'm slightly confused. Does that mean "just change it" or does that mean "add dsm_create_extended instead"? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> I'm slightly confused. Does that mean "just change it" or does that
> mean "add dsm_create_extended instead"?
FWIW, I vote for "just change it". We change C-level APIs all the time,
and this function has surely not got either the longevity nor the wide
usage that might argue for keeping its API sancrosanct.
regards, tom lane
On 2015-03-19 12:10:03 -0400, Robert Haas wrote: > On Thu, Mar 19, 2015 at 11:25 AM, Andres Freund <andres@2ndquadrant.com> wrote: > > On 2015-03-19 11:21:45 -0400, Robert Haas wrote: > >> One question I struggled with is whether to keep the existing > >> dsm_create() signature intact and add a new function > >> dsm_create_extended(). I eventually decided against it. The > >> dsm_create() API is relatively new at this point, so there probably > >> aren't too many people who will be inconvenienced by an API break now. > >> If we go ahead and create dsm_create_extended() now, and then need > >> to make another API change down the line, I doubt there will be much > >> support for dsm_create_extended2() or whatever. So my gut is that > >> it's better to just change this outright, and keep > >> dsm_create_extended() as an idea for the future. But I could go the > >> other way on that if people feel strongly about it. > > > > +1 for a clear API break. > > I'm slightly confused. Does that mean "just change it" or does that > mean "add dsm_create_extended instead"? The former. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
> > I'm slightly confused. Does that mean "just change it" or does that
> > mean "add dsm_create_extended instead"?
>
> FWIW, I vote for "just change it". We change C-level APIs all the time,
> and this function has surely not got either the longevity nor the wide
> usage that might argue for keeping its API sancrosanct.
Agreed.
Thanks,
Stephen
On Thu, Mar 19, 2015 at 12:21 PM, Stephen Frost <sfrost@snowman.net> wrote: > * Tom Lane (tgl@sss.pgh.pa.us) wrote: >> Robert Haas <robertmhaas@gmail.com> writes: >> > I'm slightly confused. Does that mean "just change it" or does that >> > mean "add dsm_create_extended instead"? >> >> FWIW, I vote for "just change it". We change C-level APIs all the time, >> and this function has surely not got either the longevity nor the wide >> usage that might argue for keeping its API sancrosanct. > > Agreed. OK, committed that way after, uh, actually testing it and fixing the bugs. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company