Re: [PATCH] "could not reattach to shared memory" on Windows

Hello,

This patch using VirtualAlloc()/VirtualFree() to avoid failing in
reattach to shared memory.

Can this be added to CommitFest ?

Recent threads in pgsql-bugs are
http://archives.postgresql.org/pgsql-bugs/2009-07/msg00036.php

This fix is almost same as previous patch. debug code is deleted.
http://archives.postgresql.org/pgsql-bugs/2009-07/msg00078.php

Regards,

--
Tsutomu Yamada
SRA OSS, Inc. Japan

On Tue, Jul 14, 2009 at 6:22 AM, Tsutomu Yamada<tsutomu(at)sraoss(dot)co(dot)jp> wrote:
> Hello,
>
> This patch using VirtualAlloc()/VirtualFree() to avoid failing in
> reattach to shared memory.
>
> Can this be added to CommitFest ?

Patches for CommitFest should be added here:

http://commitfest.postgresql.org/action/commitfest_view/open

...Robert

Tsutomu Yamada wrote:

> This patch using VirtualAlloc()/VirtualFree() to avoid failing in
> reattach to shared memory.
>
> Can this be added to CommitFest ?

Since this fixes a very annoying bug present in older versions, I think
this should be backpatched all the way back to 8.2.

Some notes about the patch itself:

- please use ereport() instead of elog() for error messages
- Are you really putting the pgwin32_ReserveSharedMemory declaration
inside a function? Please move that into the appropriate header file.
- Failure to reserve memory in pgwin32_ReserveSharedMemory should be a
FATAL error I think, not simply LOG.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> Tsutomu Yamada wrote:
>> This patch using VirtualAlloc()/VirtualFree() to avoid failing in
>> reattach to shared memory.

> Since this fixes a very annoying bug present in older versions, I think
> this should be backpatched all the way back to 8.2.

Agreed, but first we need some evidence that it actually fixes the
problem. How can we acquire such evidence?

> - please use ereport() instead of elog() for error messages

This is only appropriate if they're user-facing messages, which probably
errors in this area are not ...

regards, tom lane

Alvaro Herrera wrote:
> Tsutomu Yamada wrote:
>
>> This patch using VirtualAlloc()/VirtualFree() to avoid failing in
>> reattach to shared memory.
>>
>> Can this be added to CommitFest ?
>
> Since this fixes a very annoying bug present in older versions, I think
> this should be backpatched all the way back to 8.2.

That doesn't sound like a good idea, at least not before we have more
experience of how the patch is working in the field.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Tom Lane wrote:
> Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> > Tsutomu Yamada wrote:
> >> This patch using VirtualAlloc()/VirtualFree() to avoid failing in
> >> reattach to shared memory.
>
> > Since this fixes a very annoying bug present in older versions, I think
> > this should be backpatched all the way back to 8.2.
>
> Agreed, but first we need some evidence that it actually fixes the
> problem. How can we acquire such evidence?

Send the patch to the people who has reported trouble and see if it
seems gone? If somebody is able to build patched Win32 packages I could
point a couple of guys in the spanish list to them.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Tom Lane wrote:
> Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
>> Since this fixes a very annoying bug present in older versions, I think
>> this should be backpatched all the way back to 8.2.
>
> Agreed, but first we need some evidence that it actually fixes the
> problem. How can we acquire such evidence?

Apply to CVS HEAD and have people test it. I wouldn'ẗ be opposed to
back-patching to 8.4 where it would receive more testing in real life.
If we're really uneasy about it, provide a switch to turn it off if it
causes problems.

>> - please use ereport() instead of elog() for error messages
>
> This is only appropriate if they're user-facing messages, which probably
> errors in this area are not ...

Heh, that's what we hope :-).

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On Tue, Jul 14, 2009 at 10:28 AM, Alvaro
Herrera<alvherre(at)commandprompt(dot)com> wrote:
> Tom Lane wrote:
>> Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
>> > Tsutomu Yamada wrote:
>> >> This patch using VirtualAlloc()/VirtualFree() to avoid failing in
>> >> reattach to shared memory.
>>
>> > Since this fixes a very annoying bug present in older versions, I think
>> > this should be backpatched all the way back to 8.2.
>>
>> Agreed, but first we need some evidence that it actually fixes the
>> problem.  How can we acquire such evidence?
>
> Send the patch to the people who has reported trouble and see if it
> seems gone?  If somebody is able to build patched Win32 packages I could
> point a couple of guys in the spanish list to them.
>

- identify some people with the problem and talk to them for: 1) get a
way to reproduce the error (a lot dificult, IIRC we try a few times i
fail to fail) or 2) get their support for test
- commit it for the first alpha release, or the just talked nigthly
stable builds...
- let the tests begin :)

so, apply it just before the alpha and if it not works remove it just
after the alpha...
last time i build a win32 binary (not whole package) for windows users
to test a patch they disappear very quickly...

--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157

On Tuesday, July 14, 2009, Alvaro Herrera <alvherre(at)commandprompt(dot)com> wrote:
> Tom Lane wrote:
>> Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
>> > Tsutomu Yamada wrote:
>> >> This patch using VirtualAlloc()/VirtualFree() to avoid failing in
>> >> reattach to shared memory.
>>
>> > Since this fixes a very annoying bug present in older versions, I think
>> > this should be backpatched all the way back to 8.2.
>>
>> Agreed, but first we need some evidence that it actually fixes the
>> problem.  How can we acquire such evidence?
>
> Send the patch to the people who has reported trouble and see if it
> seems gone?  If somebody is able to build patched Win32 packages I could
> point a couple of guys in the spanish list to them.

I built a version which a guy is currently testing. He could reproduce
the bug easily, but last i heard, the patch was looking good.

Don't have the details here tho.

--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com

Jaime Casanova wrote:
> - identify some people with the problem and talk to them for: 1) get a
> way to reproduce the error (a lot dificult, IIRC we try a few times i
> fail to fail) or 2) get their support for test

For back-patching, we'd be maybe even more interested in getting people
who *don't* experience the problem to test it, to make sure it doesn't
break installations that work without it.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Hello,

Alvaro Herrera <alvherre(at)commandprompt(dot)com> wrote:
> Tsutomu Yamada wrote:
>
> > This patch using VirtualAlloc()/VirtualFree() to avoid failing in
> > reattach to shared memory.
> >
> > Can this be added to CommitFest ?
>
> Since this fixes a very annoying bug present in older versions, I think
> this should be backpatched all the way back to 8.2.
>
> Some notes about the patch itself:
>
> - please use ereport() instead of elog() for error messages
> - Are you really putting the pgwin32_ReserveSharedMemory declaration
> inside a function? Please move that into the appropriate header file.
> - Failure to reserve memory in pgwin32_ReserveSharedMemory should be a
> FATAL error I think, not simply LOG.

In this case,
the parent process operates child's memory by using VirtualAlloc().
If VirtualAlloc failed and be a FATAL error, master process will be stopped.

I think that is not preferable.
So, when VirtualAlloc failed, parent reports error and terminates child.

Revised patch

- move function declaration to include/port/win32.h
- add error check.
when VirtualAlloc failed, parent will terminate child process.

Thanks.

--
Tsutomu Yamada
SRA OSS, Inc. Japan

On Wed, Jul 15, 2009 at 11:20, Tsutomu Yamada<tsutomu(at)sraoss(dot)co(dot)jp> wrote:
> Hello,
>
> Alvaro Herrera <alvherre(at)commandprompt(dot)com> wrote:
>  > Tsutomu Yamada wrote:
>  >
>  > > This patch using VirtualAlloc()/VirtualFree() to avoid failing in
>  > > reattach to shared memory.
>  > >
>  > > Can this be added to CommitFest ?
>  >
>  > Since this fixes a very annoying bug present in older versions, I think
>  > this should be backpatched all the way back to 8.2.
>  >
>  > Some notes about the patch itself:
>  >
>  > - please use ereport() instead of elog() for error messages
>  > - Are you really putting the pgwin32_ReserveSharedMemory declaration
>  > inside a function?  Please move that into the appropriate header file.
>  > - Failure to reserve memory in pgwin32_ReserveSharedMemory should be a
>  > FATAL error I think, not simply LOG.
>
> In this case,
> the parent process operates child's memory by using VirtualAlloc().
> If VirtualAlloc failed and be a FATAL error, master process will be stopped.
>
> I think that is not preferable.
> So, when VirtualAlloc failed, parent reports error and terminates child.
>
> Revised patch
>
> - move function declaration to include/port/win32.h
> - add error check.
>  when VirtualAlloc failed, parent will terminate child process.

This patch looks a lot like one I've had sitting in my tree since
before I left for three weeks of vacation, based on the same
suggestion on the list. I will check if we have any actual functional
differences, and merge yours with mine. The one I had worked fine in
my testing.

Once that is done, I propose the following:

* Apply to HEAD. That will give us buildfarm coverage.
* Produce a modified 8.4.0 *and* 8.3.7 binary for this, and ask people
to test this. Both people with and without the problem.
* Assuming it works for all users, backpatch to 8.2, 8.3 and 8.4.

--
Magnus Hagander
Self: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Tue, Jul 21, 2009 at 14:06, Magnus Hagander<magnus(at)hagander(dot)net> wrote:
> On Wed, Jul 15, 2009 at 11:20, Tsutomu Yamada<tsutomu(at)sraoss(dot)co(dot)jp> wrote:
>> Hello,
>>
>> Alvaro Herrera <alvherre(at)commandprompt(dot)com> wrote:
>>  > Tsutomu Yamada wrote:
>>  >
>>  > > This patch using VirtualAlloc()/VirtualFree() to avoid failing in
>>  > > reattach to shared memory.
>>  > >
>>  > > Can this be added to CommitFest ?
>>  >
>>  > Since this fixes a very annoying bug present in older versions, I think
>>  > this should be backpatched all the way back to 8.2.
>>  >
>>  > Some notes about the patch itself:
>>  >
>>  > - please use ereport() instead of elog() for error messages
>>  > - Are you really putting the pgwin32_ReserveSharedMemory declaration
>>  > inside a function?  Please move that into the appropriate header file.
>>  > - Failure to reserve memory in pgwin32_ReserveSharedMemory should be a
>>  > FATAL error I think, not simply LOG.
>>
>> In this case,
>> the parent process operates child's memory by using VirtualAlloc().
>> If VirtualAlloc failed and be a FATAL error, master process will be stopped.
>>
>> I think that is not preferable.
>> So, when VirtualAlloc failed, parent reports error and terminates child.
>>
>> Revised patch
>>
>> - move function declaration to include/port/win32.h
>> - add error check.
>>  when VirtualAlloc failed, parent will terminate child process.
>
> This patch looks a lot like one I've had sitting in my tree since
> before I left for three weeks of vacation, based on the same
> suggestion on the list. I will check if we have any actual functional
> differences, and merge yours with mine. The one I had worked fine in
> my testing.
>
> Once that is done, I propose the following:
>
> * Apply to HEAD. That will give us buildfarm coverage.
> * Produce a modified 8.4.0 *and* 8.3.7 binary for this, and ask people
> to test this. Both people with and without the problem.
> * Assuming it works for all users, backpatch to 8.2, 8.3 and 8.4.

Attached are two updated versions of this patch, one for 8.4 and one
for 8.3. They differ only in line numbers. I've merged your patch with
mine, which mainly contained of more comments. One functionality check
- to make sure the VirtualAllocEx() call returns the same address as
our base one. It should always do this, but my patch adds a check t
make sure this is true.

Dave has built binaries for 8.3.7 and 8.4.0 for this, available at:

http://developer.pgadmin.org/~dpage/postgres_exe_virtualalloc-8_3.zip
http://developer.pgadmin.org/~dpage/postgres_exe_virtualalloc-8_4.zip

We would like as many people as possible to test this both on systems
that currently experience the problem and systems that don't, and let
us know the status. To test, just replace your current postgres.exe
binary with the one in the appropriate ZIP file above. Obviously, take
a backup before you do it! These binaries contain just this one patch
- the rest of what's been applied to the 8.3 and 8.4 branches for the
next minor version is *not* included.

--
Magnus Hagander
Self: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Hello,

Thank you for correcting patch.
However, I think the following block have to use VirualFree*Ex*().

(yes, this should never happen, maybe there is actually no problem.
but for logical correctness)

>+ if (address != UsedShmemSegAddr)
>+ {
>+ /*
>+ * Should never happen - in theory if allocation granularity causes strange
>+ * effects it could, so check just in case.
>+ *
>+ * Don't use FATAL since we're running in the postmaster.
>+ */
>+ elog(LOG, "reserved shared memory region got incorrect address %p, expected %p",
>+ address, UsedShmemSegAddr);
>+ VirtualFree(address, 0, MEM_RELEASE);
VirtualFreeEx(hChild, address, 0, MEM_RELEASE);

>+ return false;
>+ }

Regards,

--
Tsutomu Yamada
SRA OSS, Inc. Japan

On Thu, Jul 23, 2009 at 08:04, Tsutomu Yamada<tsutomu(at)sraoss(dot)co(dot)jp> wrote:
> Hello,
>
> Thank you for correcting patch.
> However, I think the following block have to use VirualFree*Ex*().
>
> (yes, this should never happen, maybe there is actually no problem.
>  but for logical correctness)

That is definitely correct. I have updated the patch in my tree and
will make sure to include that in the eventual commit.

FYI, and others, I have received a couple of off-list reports from
people testing out the patch, and so far only positive results.

--
Magnus Hagander
Self: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Thu, Jul 23, 2009 at 09:04, Magnus Hagander<magnus(at)hagander(dot)net> wrote:
> On Thu, Jul 23, 2009 at 08:04, Tsutomu Yamada<tsutomu(at)sraoss(dot)co(dot)jp> wrote:
>> Hello,
>>
>> Thank you for correcting patch.
>> However, I think the following block have to use VirualFree*Ex*().
>>
>> (yes, this should never happen, maybe there is actually no problem.
>>  but for logical correctness)
>
> That is definitely correct. I have updated the patch in my tree and
> will make sure to include that in the eventual commit.
>
> FYI, and others, I have received a couple of off-list reports from
> people testing out the patch, and so far only positive results.

I have applied this patch to HEAD so we can get buildfarm coverage.
Holding back on the batckpatch for a bit longer.

--
Magnus Hagander
Self: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Wed, Jul 22, 2009 at 17:05, Magnus Hagander<magnus(at)hagander(dot)net> wrote:
> Dave has built binaries for 8.3.7 and 8.4.0 for this, available at:
>
> http://developer.pgadmin.org/~dpage/postgres_exe_virtualalloc-8_3.zip
> http://developer.pgadmin.org/~dpage/postgres_exe_virtualalloc-8_4.zip
>
>
> We would like as many people as possible to test this both on systems
> that currently experience the problem and systems that don't, and let
> us know the status. To test, just replace your current postgres.exe
> binary with the one in the appropriate ZIP file above. Obviously, take
> a backup before you do it! These binaries contain just this one patch
> - the rest of what's been applied to the 8.3 and 8.4 branches for the
> next minor version is *not* included.

It's been a couple of weeks now, and I've had a number of reports both
on-list, on-blog and in private, from people using this. I have not
yet had a single report of a problem caused by this patch (not
counting the case where there was a version mismatch - can't fault the
patch for that).

Given that, I say we apply this for 8.3 and 8.4 now. Comments?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> It's been a couple of weeks now, and I've had a number of reports both
> on-list, on-blog and in private, from people using this. I have not
> yet had a single report of a problem caused by this patch (not
> counting the case where there was a version mismatch - can't fault the
> patch for that).

> Given that, I say we apply this for 8.3 and 8.4 now. Comments?

8.2 as well, no?

regards, tom lane

On Mon, Aug 10, 2009 at 16:10, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>> It's been a couple of weeks now, and I've had a number of reports both
>> on-list, on-blog and in private, from people using this. I have not
>> yet had a single report of a problem caused by this patch (not
>> counting the case where there was a version mismatch - can't fault the
>> patch for that).
>
>> Given that, I say we apply this for 8.3 and 8.4 now. Comments?
>
> 8.2 as well, no?

8.2 has a different shmem implementation - the one that emulates sysv
shmem. The patch will need to be changed around for that, and I
haven't looked at that. It may be worthwhile to do that, but it's a
separate patch, so let's get it out in 8.3 and 8.4 first.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Mon, Aug 10, 2009 at 3:33 PM, Magnus Hagander<magnus(at)hagander(dot)net> wrote:
> On Mon, Aug 10, 2009 at 16:10, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>>> It's been a couple of weeks now, and I've had a number of reports both
>>> on-list, on-blog and in private, from people using this. I have not
>>> yet had a single report of a problem caused by this patch (not
>>> counting the case where there was a version mismatch - can't fault the
>>> patch for that).
>>
>>> Given that, I say we apply this for 8.3 and 8.4 now. Comments?
>>
>> 8.2 as well, no?
>
> 8.2 has a different shmem implementation - the one that emulates sysv
> shmem. The patch will need to be changed around for that, and I
> haven't looked at that. It may be worthwhile to do that, but it's a
> separate patch, so let's get it out in 8.3 and 8.4 first.

Has anyone reported the problem on 8.2?

--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com

On Mon, Aug 10, 2009 at 16:45, Dave Page<dpage(at)pgadmin(dot)org> wrote:
> On Mon, Aug 10, 2009 at 3:33 PM, Magnus Hagander<magnus(at)hagander(dot)net> wrote:
>> On Mon, Aug 10, 2009 at 16:10, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>>>> It's been a couple of weeks now, and I've had a number of reports both
>>>> on-list, on-blog and in private, from people using this. I have not
>>>> yet had a single report of a problem caused by this patch (not
>>>> counting the case where there was a version mismatch - can't fault the
>>>> patch for that).
>>>
>>>> Given that, I say we apply this for 8.3 and 8.4 now. Comments?
>>>
>>> 8.2 as well, no?
>>
>> 8.2 has a different shmem implementation - the one that emulates sysv
>> shmem. The patch will need to be changed around for that, and I
>> haven't looked at that. It may be worthwhile to do that, but it's a
>> separate patch, so let's get it out in 8.3 and 8.4 first.
>
> Has anyone reported the problem on 8.2?

Yes. I've seen reports of it all the way back to 8.0. It does seem to
have increased in frequently with Win2003 and Win2008 as the server
platforms, which means the newer versions have had a higher
percentage, but the issue definitely exists.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Mon, Aug 10, 2009 at 3:49 PM, Magnus Hagander<magnus(at)hagander(dot)net> wrote:
>> Has anyone reported the problem on 8.2?
>
> Yes. I've seen reports of it all the way back to 8.0. It does seem to
> have increased in frequently with Win2003 and Win2008 as the server
> platforms, which means the newer versions have had a higher
> percentage, but the issue definitely exists.

I suppose there's some question of whether this is the kind of issue
we need to bother supporting for back-branches. The whole point of
supporting back branches is so that people who are already using them
can expect to have any known problems they might run into fixed.

If people are still running these old branches then presumably their
setup isn't prone to this problem. If they're going to update to
Win2003 or Win2008 then that's a whole new installation, not an
existing installation which might suddenly run into this problem.

Is the reason we support old branches so that people can install those
old branches in preference to newer ones? Or just so that people who
have already installed them can continue to rely on them?

The flaws in this line of argument are that a) I'm not entirely sure
my premise that someone who has been running fine won't suddenly run
into this problem is true. And b) nor am I entirely clear that you
have to reinstall Postgres or other apps when you upgrade Windows.

--
greg
http://mit.edu/~gsstark/resume.pdf

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> On Mon, Aug 10, 2009 at 16:10, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> 8.2 as well, no?

> 8.2 has a different shmem implementation - the one that emulates sysv
> shmem. The patch will need to be changed around for that, and I
> haven't looked at that. It may be worthwhile to do that, but it's a
> separate patch, so let's get it out in 8.3 and 8.4 first.

If it's at all hard to do, I could see deprecating 8.2 for Windows
instead.

regards, tom lane

On Mon, Aug 10, 2009 at 3:58 PM, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>> On Mon, Aug 10, 2009 at 16:10, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> 8.2 as well, no?
>
>> 8.2 has a different shmem implementation - the one that emulates sysv
>> shmem. The patch will need to be changed around for that, and I
>> haven't looked at that. It may be worthwhile to do that, but it's a -
>> separate patch, so let's get it out in 8.3 and 8.4 first.
>
> If it's at all hard to do, I could see deprecating 8.2 for Windows
> instead.

I could most definitely agree with that on a personal level - no more
Mingw/msys builds to maintain :-)

Alas, it's probably not practical to drop it without inconveniencing a
great many Windows users.

--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com

Dave Page wrote:
>> If it's at all hard to do, I could see deprecating 8.2 for Windows
>> instead.
>>
>
> I could most definitely agree with that on a personal level - no more
> Mingw/msys builds to maintain :-)
>
> Alas, it's probably not practical to drop it without inconveniencing a
> great many Windows users.
>
>

I hope you're not suggesting we drop Mingw/MSys as a build platform,
even if you personally don't want to build with it. I would have found
it much harder to do parallel restore for Windows (which works quite
differently from Unix, and so had to be specifically developed) if I had
been forced to use the MS tool set with which I don't ever otherwise work.

I don't think we should deprecate 8.2 on Windows unless we really can't
backport this fix reasonably.

cheers

andrew

On Mon, Aug 10, 2009 at 4:29 PM, Andrew Dunstan<andrew(at)dunslane(dot)net> wrote:
>
> I hope you're not suggesting we drop Mingw/MSys as a build platform, even if
> you personally don't want to build with it. I would have found it much
> harder to do parallel restore for Windows (which works quite differently
> from Unix, and so had to be specifically developed) if I had been forced to
> use the MS tool set with which I don't ever otherwise work.

Not at all - in fact we need it to maintain some of the other apps
like PostGIS or Slony. I'm just talking about my own use of it for
building PG release builds.

> I don't think we should deprecate 8.2 on Windows unless we really can't
> backport this fix reasonably.

Agreed. There are too many users, and it wouldn't be fair to them.

--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com

On Mon, Aug 10, 2009 at 16:58, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>> On Mon, Aug 10, 2009 at 16:10, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> 8.2 as well, no?
>
>> 8.2 has a different shmem implementation - the one that emulates sysv
>> shmem. The patch will need to be changed around for that, and I
>> haven't looked at that. It may be worthwhile to do that, but it's a
>> separate patch, so let's get it out in 8.3 and 8.4 first.
>
> If it's at all hard to do, I could see deprecating 8.2 for Windows
> instead.

I haven't looked at how much work it would be at all yet. So let's do
that before we decide to deprecate anything. As mentioned downthread,
8.2 is a very widespread release, and we really want to avoid
deprecating it.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Mon, Aug 10, 2009 at 13:41, Magnus Hagander<magnus(at)hagander(dot)net> wrote:
> On Wed, Jul 22, 2009 at 17:05, Magnus Hagander<magnus(at)hagander(dot)net> wrote:
>> Dave has built binaries for 8.3.7 and 8.4.0 for this, available at:
>>
>> http://developer.pgadmin.org/~dpage/postgres_exe_virtualalloc-8_3.zip
>> http://developer.pgadmin.org/~dpage/postgres_exe_virtualalloc-8_4.zip
>>
>>
>> We would like as many people as possible to test this both on systems
>> that currently experience the problem and systems that don't, and let
>> us know the status. To test, just replace your current postgres.exe
>> binary with the one in the appropriate ZIP file above. Obviously, take
>> a backup before you do it! These binaries contain just this one patch
>> - the rest of what's been applied to the 8.3 and 8.4 branches for the
>> next minor version is *not* included.
>
> It's been a couple of weeks now, and I've had a number of reports both
> on-list, on-blog and in private, from people using this. I have not
> yet had a single report of a problem caused by this patch (not
> counting the case where there was a version mismatch - can't fault the
> patch for that).
>
> Given that, I say we apply this for 8.3 and 8.4 now. Comments?

Backpatched to 8.3 and 8.4 for now.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Mon, Aug 10, 2009 at 19:33, Magnus Hagander<magnus(at)hagander(dot)net> wrote:
> On Mon, Aug 10, 2009 at 16:58, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>>> On Mon, Aug 10, 2009 at 16:10, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>>> 8.2 as well, no?
>>
>>> 8.2 has a different shmem implementation - the one that emulates sysv
>>> shmem. The patch will need to be changed around for that, and I
>>> haven't looked at that. It may be worthwhile to do that, but it's a
>>> separate patch, so let's get it out in 8.3 and 8.4 first.
>>
>> If it's at all hard to do, I could see deprecating 8.2 for Windows
>> instead.
>
> I haven't looked at how much work it would be at all yet. So let's do
> that before we decide to deprecate anything. As mentioned downthread,
> 8.2 is a very widespread release, and we really want to avoid
> deprecating it.

Here's an attempt at a backport to 8.2. I haven't examined it in
detail, but it passes "make check" on mingw.

Comments?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Tue, Aug 11, 2009 at 16:30, Magnus Hagander<magnus(at)hagander(dot)net> wrote:
> On Mon, Aug 10, 2009 at 19:33, Magnus Hagander<magnus(at)hagander(dot)net> wrote:
>> On Mon, Aug 10, 2009 at 16:58, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>>>> On Mon, Aug 10, 2009 at 16:10, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>>>> 8.2 as well, no?
>>>
>>>> 8.2 has a different shmem implementation - the one that emulates sysv
>>>> shmem. The patch will need to be changed around for that, and I
>>>> haven't looked at that. It may be worthwhile to do that, but it's a
>>>> separate patch, so let's get it out in 8.3 and 8.4 first.
>>>
>>> If it's at all hard to do, I could see deprecating 8.2 for Windows
>>> instead.
>>
>> I haven't looked at how much work it would be at all yet. So let's do
>> that before we decide to deprecate anything. As mentioned downthread,
>> 8.2 is a very widespread release, and we really want to avoid
>> deprecating it.
>
> Here's an attempt at a backport to 8.2. I haven't examined it  in
> detail, but it passes "make check" on mingw.
>
> Comments?

I've also built a binary that should be copy:able on top of an 8.2.13
installation made from the standard installer, to test this feature.
Anybody on 8.2 on Windows, please give it a shot and let us know how
it works.

http://www.hagander.net/pgsql/postgres_exe_virtualalloc_8_2.zip

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/