Horton

Thread

Commits

  1. Remove some remaining traces of dsm_resize().

  2. Remove dsm_resize() and dsm_remap().

  3. Allow DSM allocation to be interrupted.

  1. Strange failure in LWLock on skink in REL9_5_STABLE

    Thomas Munro <thomas.munro@enterprisedb.com> — 2018-09-21T02:39:43Z

    Hello,

Andres pinged me off-list to point out this failure after my commit fb389498be:

https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2018-09-20%2005%3A24%3A34

Change Set for this build:
fb389498be Tue Sep 18 11:19:22 2018 UTC  Allow DSM allocation to be interrupted.

The failure looks like this:

! FATAL:  semop(id=332464133) failed: Invalid argument
! CONTEXT:  SQL statement "CREATE TEMP TABLE brin_result (cid tid)"
! PL/pgSQL function inline_code_block line 22 at SQL statement
! PANIC:  queueing for lock while waiting on another one
! server closed the connection unexpectedly
! This probably means the server terminated abnormally
! before or while processing the request.
! connection to server was lost

I don't immediately see any connection between that particular commit,
which relates to the treatment of signals while allocating a DSM
segment, and the location of the first failure, which is in a
statement that is creating a temporary table.  On the other hand skink
has been very stable lately.  I'm also not sure how the FATAL error
and the PANIC are related (LWLockQueueSelf() has discovered that
MyProc->lwWaiting is already set).  Though it's possible that the root
problem was something happening in any of the other parallel tests
running, I don't see how any of those (lock security_label tablesample
object_address rowsecurity collate spgist privileges matview
replica_identity brin gin gist groupingsets) would reach code touched
by that commit in 9.5, but I don't currently have any other ideas
about what happened here.

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

  2. Re: Strange failure in LWLock on skink in REL9_5_STABLE

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-09-21T02:59:29Z

    Thomas Munro <thomas.munro@enterprisedb.com> writes:
> Andres pinged me off-list to point out this failure after my commit fb389498be:

> ! FATAL:  semop(id=332464133) failed: Invalid argument

I was just looking at that, and my guess is that it was caused by
something doing an ipcrm or equivalent, and is unrelated to your patch.
Especially since skink has succeeded with that patch in several other
branches.

If it's repeatable, then it would be time to get excited.

			regards, tom lane

  3. Re: Strange failure in LWLock on skink in REL9_5_STABLE

    Thomas Munro <thomas.munro@enterprisedb.com> — 2018-09-21T03:11:04Z

    On Fri, Sep 21, 2018 at 2:59 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Thomas Munro <thomas.munro@enterprisedb.com> writes:
> > Andres pinged me off-list to point out this failure after my commit fb389498be:
>
> > ! FATAL:  semop(id=332464133) failed: Invalid argument
>
> I was just looking at that, and my guess is that it was caused by
> something doing an ipcrm or equivalent, and is unrelated to your patch.
> Especially since skink has succeeded with that patch in several other
> branches.
>
> If it's repeatable, then it would be time to get excited.

I found this case from January:

https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2018-01-05%2000%3A10%3A03

! FATAL:  semop(id=1313210374) failed: Invalid argument
! LINE 1: CREATE TABLE as_select1 AS SELECT * FROM pg_class WHERE relk...

       EINVAL The semaphore set doesn't exist, or semid is less than zero,
              or nsops has a nonpositive value.

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

  4. Re: Strange failure in LWLock on skink in REL9_5_STABLE

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-09-21T03:15:45Z

    Thomas Munro <thomas.munro@enterprisedb.com> writes:
> On Fri, Sep 21, 2018 at 2:59 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I was just looking at that, and my guess is that it was caused by
>> something doing an ipcrm or equivalent, and is unrelated to your patch.
>> Especially since skink has succeeded with that patch in several other
>> branches.

> I found this case from January:
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2018-01-05%2000%3A10%3A03
> ! FATAL:  semop(id=1313210374) failed: Invalid argument

Uh-huh.

https://www.postgresql.org/docs/devel/static/kernel-resources.html#SYSTEMD-REMOVEIPC

			regards, tom lane

  5. Re: Strange failure in LWLock on skink in REL9_5_STABLE

    Andres Freund <andres@anarazel.de> — 2018-09-21T03:21:38Z

    On 2018-09-20 22:59:29 -0400, Tom Lane wrote:
> Thomas Munro <thomas.munro@enterprisedb.com> writes:
> > Andres pinged me off-list to point out this failure after my commit fb389498be:
> 
> > ! FATAL:  semop(id=332464133) failed: Invalid argument
> 
> I was just looking at that, and my guess is that it was caused by
> something doing an ipcrm or equivalent, and is unrelated to your patch.
> Especially since skink has succeeded with that patch in several other
> branches.

I'm (hopefully) the only person with access to that machine, and I
certainly didn't do so. Nor are there script I know of that'd do
so. There's not been a lot of instability on skink, so it's certainly
quite weird.

I'm quite suspicious of the logic around:

		/*
		 * If we received a query cancel or termination signal, we will have
		 * EINTR set here.  If the caller said that errors are OK here, check
		 * for interrupts immediately.
		 */
		if (errno == EINTR && elevel >= ERROR)
			CHECK_FOR_INTERRUPTS();

because it seems far from guaranteed to do anything meaningful as I
don't see a guarantee that interrupts are active at that point (e.g. it
seems quite reasonable to hold an lwlock while resizing).

Afaict that might cause problems at a later stage, because at that point
we've not adjusted the actual mapping, but *have* ftruncate()ed it. If
there's actual data in the mapping, that certainly could cause trouble.

In fact, while this commit has expanded the size of the problem, I fail
to see how the error handling for resizing is correct. It's fine to fail
in the ftruncate() itself - at that point no changes have been made -,
but I don't think it's currently ok for posix_fallocate() to ever error
out.

It's not clear to me how that'd be problematic in 9.5 of all releases
however.

> If it's repeatable, then it would be time to get excited.

Yea, I guess we'll have to wait :/.

Greetings,

Andres Freund

  6. Re: Strange failure in LWLock on skink in REL9_5_STABLE

    Andres Freund <andres@anarazel.de> — 2018-09-21T03:36:12Z

    On 2018-09-20 23:15:45 -0400, Tom Lane wrote:
> Thomas Munro <thomas.munro@enterprisedb.com> writes:
> > On Fri, Sep 21, 2018 at 2:59 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> I was just looking at that, and my guess is that it was caused by
> >> something doing an ipcrm or equivalent, and is unrelated to your patch.
> >> Especially since skink has succeeded with that patch in several other
> >> branches.
> 
> > I found this case from January:
> > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2018-01-05%2000%3A10%3A03
> > ! FATAL:  semop(id=1313210374) failed: Invalid argument

> Uh-huh.
> 
> https://www.postgresql.org/docs/devel/static/kernel-resources.html#SYSTEMD-REMOVEIPC

That shouldn't be relevant here - I'm not running the buildfarm from an
interactive session and then logging out. So that code shouldn't
trigger.  I've made sure that the setting is off now however, I'm not
trusting the related logic very much...

Greetings,

Andres Freund

  7. Re: Strange failure in LWLock on skink in REL9_5_STABLE

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-09-21T03:46:54Z

    Andres Freund <andres@anarazel.de> writes:
> On 2018-09-20 23:15:45 -0400, Tom Lane wrote:
>> https://www.postgresql.org/docs/devel/static/kernel-resources.html#SYSTEMD-REMOVEIPC

> That shouldn't be relevant here - I'm not running the buildfarm from an
> interactive session and then logging out. So that code shouldn't
> trigger.

Well, the reason that systemd behavior is so brain-dead is exactly that
it will trigger against processes that you didn't launch interactively.
IIUC, you just have to log in and out of that account, and anything that
was running in the background (eg cron jobs) under the same userID gets
clobbered.

> I've made sure that the setting is off now however, I'm not
> trusting the related logic very much...

Was it on before?

			regards, tom lane

  8. Re: Strange failure in LWLock on skink in REL9_5_STABLE

    Thomas Munro <thomas.munro@enterprisedb.com> — 2018-09-21T04:03:18Z

    On Fri, Sep 21, 2018 at 3:21 PM Andres Freund <andres@anarazel.de> wrote:
> I'm quite suspicious of the logic around:
>
>                 /*
>                  * If we received a query cancel or termination signal, we will have
>                  * EINTR set here.  If the caller said that errors are OK here, check
>                  * for interrupts immediately.
>                  */
>                 if (errno == EINTR && elevel >= ERROR)
>                         CHECK_FOR_INTERRUPTS();
>
> because it seems far from guaranteed to do anything meaningful as I
> don't see a guarantee that interrupts are active at that point (e.g. it
> seems quite reasonable to hold an lwlock while resizing).

Right, in that case CFI does nothing and you get the following
ereport() instead, so the user sees an unsightly "Interrupt system
call" message (or however your strerror() spells it).  That would
actually happen in the dsa.c case for example (something that should
be improved especially now that dsm_create() is so slow on Linux,
independently of all this).  That's probably not a great user
experience, but I'm not sure if the fact that it "works around" the
suppression of interrupts while LWLocks are held by converting them
into errors is a more serious problem or not.  The caller has to be
ready for errors to be raised here in any case.

> Afaict that might cause problems at a later stage, because at that point
> we've not adjusted the actual mapping, but *have* ftruncate()ed it. If
> there's actual data in the mapping, that certainly could cause trouble.
>
> In fact, while this commit has expanded the size of the problem, I fail
> to see how the error handling for resizing is correct. It's fine to fail
> in the ftruncate() itself - at that point no changes have been made -,
> but I don't think it's currently ok for posix_fallocate() to ever error
> out.

Right, independently of this commit, dsm_resize() might have done the
actual truncation when the error is reported.  That's not good if the
caller plans to do anything other than abandon ship.  None of this
applies to dsm_create() though, which uses the same code path but
knows how to clean up.  There may be ways to fix the dsm_resize() path
based on the observation that you don't need to fallocate() if you
made the mapping smaller, and if you made it bigger then you could
always undo that on error (or not) and you haven't thrown away any
data.  Hmm... I note that there are actually no callers of
dsm_resize(), and it's not implemented on Windows or SystemV.

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

  9. Re: Strange failure in LWLock on skink in REL9_5_STABLE

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-09-21T04:06:39Z

    Thomas Munro <thomas.munro@enterprisedb.com> writes:
> ... There may be ways to fix the dsm_resize() path
> based on the observation that you don't need to fallocate() if you
> made the mapping smaller, and if you made it bigger then you could
> always undo that on error (or not) and you haven't thrown away any
> data.  Hmm... I note that there are actually no callers of
> dsm_resize(), and it's not implemented on Windows or SystemV.

Why would we fix it rather than just removing it?

			regards, tom lane

  10. Re: Strange failure in LWLock on skink in REL9_5_STABLE

    Andres Freund <andres@anarazel.de> — 2018-09-21T04:20:05Z

    On 2018-09-20 23:46:54 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > I've made sure that the setting is off now however, I'm not
> > trusting the related logic very much...
> 
> Was it on before?

Yes.

  11. Re: Strange failure in LWLock on skink in REL9_5_STABLE

    Thomas Munro <thomas.munro@enterprisedb.com> — 2018-09-21T04:36:06Z

    On Fri, Sep 21, 2018 at 4:06 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Thomas Munro <thomas.munro@enterprisedb.com> writes:
> > ... There may be ways to fix the dsm_resize() path
> > based on the observation that you don't need to fallocate() if you
> > made the mapping smaller, and if you made it bigger then you could
> > always undo that on error (or not) and you haven't thrown away any
> > data.  Hmm... I note that there are actually no callers of
> > dsm_resize(), and it's not implemented on Windows or SystemV.

Erm, actually you probably only need to do ftruncate() *or*
posix_fallocate(), depending on the direction of the resize.  Doing
both is redundant and introduces this theoretical hazard (in practice
I'd be surprised if fallocate() really can fail after you shrank a
file that was already fully allocated).

> Why would we fix it rather than just removing it?

I assumed we wouldn't remove an extern C function extension code
somewhere might use.  Though admittedly I'd be surprised if anyone
used this one.

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

  12. Re: Strange failure in LWLock on skink in REL9_5_STABLE

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-09-21T04:43:31Z

    Thomas Munro <thomas.munro@enterprisedb.com> writes:
> On Fri, Sep 21, 2018 at 4:06 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Why would we fix it rather than just removing it?

> I assumed we wouldn't remove an extern C function extension code
> somewhere might use.  Though admittedly I'd be surprised if anyone
> used this one.

Unless it looks practical to support this behavior in the Windows
and SysV cases, I think we should get rid of it rather than expend
effort on supporting it for just some platforms.

			regards, tom lane

  13. Re: Strange failure in LWLock on skink in REL9_5_STABLE

    Thomas Munro <thomas.munro@enterprisedb.com> — 2018-09-21T20:54:57Z

    On Fri, Sep 21, 2018 at 4:43 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Thomas Munro <thomas.munro@enterprisedb.com> writes:
> > On Fri, Sep 21, 2018 at 4:06 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> Why would we fix it rather than just removing it?
>
> > I assumed we wouldn't remove an extern C function extension code
> > somewhere might use.  Though admittedly I'd be surprised if anyone
> > used this one.
>
> Unless it looks practical to support this behavior in the Windows
> and SysV cases, I think we should get rid of it rather than expend
> effort on supporting it for just some platforms.

We can remove it in back-branches without breaking API compatibility:

1.  Change dsm_impl_can_resize() to return false unconditionally (I
suppose client code is supposed to check this before using
dsm_resize(), though I'm not sure why it has an "impl" in its name if
it's part of the public interface of this module).
2.  Change dsm_resize() and dsm_remap() to raise an error conditionally.
3.  Rip out the DSM_OP_RESIZE cases from various places.

Then in master, remove all of those functions completely.  However,
I'd feel like a bit of a vandal.  Robert and Amit probably had plans
for that code...?

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

  14. Re: Strange failure in LWLock on skink in REL9_5_STABLE

    Andres Freund <andres@anarazel.de> — 2018-09-21T20:58:27Z

    On 2018-09-22 08:54:57 +1200, Thomas Munro wrote:
> On Fri, Sep 21, 2018 at 4:43 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Thomas Munro <thomas.munro@enterprisedb.com> writes:
> > > On Fri, Sep 21, 2018 at 4:06 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > >> Why would we fix it rather than just removing it?
> >
> > > I assumed we wouldn't remove an extern C function extension code
> > > somewhere might use.  Though admittedly I'd be surprised if anyone
> > > used this one.
> >
> > Unless it looks practical to support this behavior in the Windows
> > and SysV cases, I think we should get rid of it rather than expend
> > effort on supporting it for just some platforms.
> 
> We can remove it in back-branches without breaking API compatibility:
> 
> 1.  Change dsm_impl_can_resize() to return false unconditionally (I
> suppose client code is supposed to check this before using
> dsm_resize(), though I'm not sure why it has an "impl" in its name if
> it's part of the public interface of this module).
> 2.  Change dsm_resize() and dsm_remap() to raise an error conditionally.
> 3.  Rip out the DSM_OP_RESIZE cases from various places.
> 
> Then in master, remove all of those functions completely.  However,
> I'd feel like a bit of a vandal.  Robert and Amit probably had plans
> for that code...?

Robert, Amit: ^

Greetings,

Andres Freund

  15. Re: Strange failure in LWLock on skink in REL9_5_STABLE

    Amit Kapila <amit.kapila16@gmail.com> — 2018-09-22T04:52:36Z

    On Sat, Sep 22, 2018 at 2:28 AM Andres Freund <andres@anarazel.de> wrote:
>
> On 2018-09-22 08:54:57 +1200, Thomas Munro wrote:
> > On Fri, Sep 21, 2018 at 4:43 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > > Thomas Munro <thomas.munro@enterprisedb.com> writes:
> > > > On Fri, Sep 21, 2018 at 4:06 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > > >> Why would we fix it rather than just removing it?
> > >
> > > > I assumed we wouldn't remove an extern C function extension code
> > > > somewhere might use.  Though admittedly I'd be surprised if anyone
> > > > used this one.
> > >
> > > Unless it looks practical to support this behavior in the Windows
> > > and SysV cases, I think we should get rid of it rather than expend
> > > effort on supporting it for just some platforms.
> >
> > We can remove it in back-branches without breaking API compatibility:
> >
> > 1.  Change dsm_impl_can_resize() to return false unconditionally (I
> > suppose client code is supposed to check this before using
> > dsm_resize(), though I'm not sure why it has an "impl" in its name if
> > it's part of the public interface of this module).
> > 2.  Change dsm_resize() and dsm_remap() to raise an error conditionally.
> > 3.  Rip out the DSM_OP_RESIZE cases from various places.
> >
> > Then in master, remove all of those functions completely.  However,
> > I'd feel like a bit of a vandal.  Robert and Amit probably had plans
> > for that code...?
>
> Robert, Amit: ^
>

I went through and check the original proposal [1] to see if any use
case is mentioned there, but nothing related has been discussed.   I
couldn't think of much use of this facility except maybe for something
like parallelizing correalated sub-queries where the size of outer var
can change across executions and we might need to resize the initially
allocated memory.  This is just a wild thought, I don't have any
concrete idea about this.   Having said that, I don't object to
removing this especially because the implementation doesn't seem to be
complete.  In future, if someone needs such a facility, they can first
develop a complete version of this API.

[1] - https://www.postgresql.org/message-id/CA%2BTgmoaDqDUgt%3D4Zs_QPOnBt%3DEstEaVNP%2B5t%2Bm%3DFPNWshiPR3A%40mail.gmail.com

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

  16. Re: Strange failure in LWLock on skink in REL9_5_STABLE

    Thomas Munro <thomas.munro@enterprisedb.com> — 2018-09-25T06:22:19Z

    On Sat, Sep 22, 2018 at 4:52 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Sat, Sep 22, 2018 at 2:28 AM Andres Freund <andres@anarazel.de> wrote:
> > On 2018-09-22 08:54:57 +1200, Thomas Munro wrote:
> > > On Fri, Sep 21, 2018 at 4:43 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > > > Unless it looks practical to support this behavior in the Windows
> > > > and SysV cases, I think we should get rid of it rather than expend
> > > > effort on supporting it for just some platforms.
> > >
> > > We can remove it in back-branches without breaking API compatibility:
> > >
> > > 1.  Change dsm_impl_can_resize() to return false unconditionally (I
> > > suppose client code is supposed to check this before using
> > > dsm_resize(), though I'm not sure why it has an "impl" in its name if
> > > it's part of the public interface of this module).
> > > 2.  Change dsm_resize() and dsm_remap() to raise an error conditionally.
> > > 3.  Rip out the DSM_OP_RESIZE cases from various places.
> > >
> > > Then in master, remove all of those functions completely.  However,
> > > I'd feel like a bit of a vandal.  Robert and Amit probably had plans
> > > for that code...?
> >
> > Robert, Amit: ^
>
> I went through and check the original proposal [1] to see if any use
> case is mentioned there, but nothing related has been discussed.   I
> couldn't think of much use of this facility except maybe for something
> like parallelizing correalated sub-queries where the size of outer var
> can change across executions and we might need to resize the initially
> allocated memory.  This is just a wild thought, I don't have any
> concrete idea about this.   Having said that, I don't object to
> removing this especially because the implementation doesn't seem to be
> complete.  In future, if someone needs such a facility, they can first
> develop a complete version of this API.

Thanks for looking into that.  Here's a pair of draft patches to
disable and then remove dsm_resize() and dsm_map().

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

  17. Re: Strange failure in LWLock on skink in REL9_5_STABLE

    Michael Paquier <michael@paquier.xyz> — 2018-11-05T07:28:09Z

    On Tue, Sep 25, 2018 at 06:22:19PM +1200, Thomas Munro wrote:
> On Sat, Sep 22, 2018 at 4:52 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>> I went through and check the original proposal [1] to see if any use
>> case is mentioned there, but nothing related has been discussed.   I
>> couldn't think of much use of this facility except maybe for something
>> like parallelizing correalated sub-queries where the size of outer var
>> can change across executions and we might need to resize the initially
>> allocated memory.  This is just a wild thought, I don't have any
>> concrete idea about this.   Having said that, I don't object to
>> removing this especially because the implementation doesn't seem to be
>> complete.  In future, if someone needs such a facility, they can first
>> develop a complete version of this API.
> 
> Thanks for looking into that.  Here's a pair of draft patches to
> disable and then remove dsm_resize() and dsm_map().

Hm.  Don't we need to worry about anybody potentially using these APIs
in a custom module on platforms where it was actually working?  I
imagine that their reaction is not going be nice if any code breaks
suddenly after a minor release.  No issues with removing the interface
on HEAD of course.
--
Michael

  18. Re: Strange failure in LWLock on skink in REL9_5_STABLE

    Robert Haas <robertmhaas@gmail.com> — 2018-11-05T21:17:53Z

    On Mon, Nov 5, 2018 at 2:28 AM Michael Paquier <michael@paquier.xyz> wrote:
> Hm.  Don't we need to worry about anybody potentially using these APIs
> in a custom module on platforms where it was actually working?  I
> imagine that their reaction is not going be nice if any code breaks
> suddenly after a minor release.  No issues with removing the interface
> on HEAD of course.

+1.

The fact that the code exists there at all is my fault.  I thought it
might be useful someday, but now I don't think so any more.  Thomas's
solution -- in the DSA machinery -- of allocating entirely new
segments seems like a better approach for now, and in the long run, I
think we should convert the whole backend to use threads,
nonwithstanding the TODO entry that says otherwise.  Even if we never
do that, extending a segment in place is pretty difficult to make
practical, since it may involve remapping the segment, which
invalidates cached pointers to anything in the segment.  And then
there are the portability problems on top of that.  So I'm not very
optimistic about this any more.

But ripping it out in the back branches seems unnecessary.  I'd just
leave the bugs unfixed there.  Most likely nobody is using that stuff,
but if they are, let's not break it.

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

  19. Re: Strange failure in LWLock on skink in REL9_5_STABLE

    Thomas Munro <thomas.munro@enterprisedb.com> — 2018-11-06T04:29:36Z

    On Tue, Nov 6, 2018 at 10:18 AM Robert Haas <robertmhaas@gmail.com> wrote:
> On Mon, Nov 5, 2018 at 2:28 AM Michael Paquier <michael@paquier.xyz> wrote:
> > Hm.  Don't we need to worry about anybody potentially using these APIs
> > in a custom module on platforms where it was actually working?  I
> > imagine that their reaction is not going be nice if any code breaks
> > suddenly after a minor release.  No issues with removing the interface
> > on HEAD of course.
>
> +1.
>
> The fact that the code exists there at all is my fault.  I thought it
> might be useful someday, but now I don't think so any more.  Thomas's
> solution -- in the DSA machinery -- of allocating entirely new
> segments seems like a better approach for now, and in the long run, I
> think we should convert the whole backend to use threads,
> nonwithstanding the TODO entry that says otherwise.  Even if we never
> do that, extending a segment in place is pretty difficult to make
> practical, since it may involve remapping the segment, which
> invalidates cached pointers to anything in the segment.  And then
> there are the portability problems on top of that.  So I'm not very
> optimistic about this any more.
>
> But ripping it out in the back branches seems unnecessary.  I'd just
> leave the bugs unfixed there.  Most likely nobody is using that stuff,
> but if they are, let's not break it.

Thanks.  Pushed to master only.

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

  20. Re: Strange failure in LWLock on skink in REL9_5_STABLE

    Michael Paquier <michael@paquier.xyz> — 2018-11-06T05:15:51Z

    On Tue, Nov 06, 2018 at 05:29:36PM +1300, Thomas Munro wrote:
> Thanks.  Pushed to master only.

Just a wild idea while this thread is hot: could we add in the
description of the broken APIs or in their headers more information
about how to not use them so as users are warned if trying to use them
in certain circumstances?  This idea is just for the stable branches.
--
Michael

  21. Re: Strange failure in LWLock on skink in REL9_5_STABLE

    Thomas Munro <thomas.munro@enterprisedb.com> — 2018-11-06T05:45:02Z

    On Tue, Nov 6, 2018 at 6:15 PM Michael Paquier <michael@paquier.xyz> wrote:
> On Tue, Nov 06, 2018 at 05:29:36PM +1300, Thomas Munro wrote:
> > Thanks.  Pushed to master only.
>
> Just a wild idea while this thread is hot: could we add in the
> description of the broken APIs or in their headers more information
> about how to not use them so as users are warned if trying to use them
> in certain circumstances?  This idea is just for the stable branches.

Like this?

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

  22. Re: Strange failure in LWLock on skink in REL9_5_STABLE

    Michael Paquier <michael@paquier.xyz> — 2018-11-06T06:13:21Z

    On Tue, Nov 06, 2018 at 06:45:02PM +1300, Thomas Munro wrote:
> Like this?

Ah, my mistake.  I thought that the limitations with dsm_resize were not
documented but those actually return an error if trying to use
DSM_OP_RESIZE and I did not notice that, so we may be fine without a
comment or such in back-branches.  Sorry for the noise.
--
Michael