Support for QNX6, POSIX IPC and PTHREAD-style locking
Here is the patch which adds following things to 7.2:
1. Support for QNX6 (builds cleanly on stock installation and passes all
regression tests).
2. HAVE_POSIX_IPC feature, which if enabled switches implementation of
IpcSemaphoreXXX() (ipc.[ch]) to POSIX semaphores and mmap(). Enabled on QNX6
but should be useful for lot of platforms. Since IpcSemaphoreCreate() really
assumed SysV semaphores, I had to change its prototype *when* this feature
is enabled. That function is called from proc.c and spin.c, which were
patched accordingly for POSIX case (with #ifdef guards).
3. USE_PTHREAD_MUTEXES feature, which if enabled implements S_LOCK stuff
with PTHREAD mutexes. It is useful (better than spinlocks) on non-SMP
systems when overhead of kernel call is small compared to overhead of
scheduling. Enabled on QNX6.
4. USE_PTHREAD_SPINLOCKS feature which if enabled implements S_LOCK stuff
with PTHREAD spinlocks (may not be available on all platforms). It might be
better on SMP systems when hardware does not support TAS (in which case SysV
semaphores would be used as of now and it is hard to be worse than that).
MIPS systems come to mind (and QNX6 runs on them, so it will be enabled on
QNX6 in such cases).
I haven't put checks for (2), (3) or (4) into configure to not break
supported platforms in unexpected ways. Benefits of (3) and (4) are really
OS and hardware dependent, so if you think they could be useful for your
platform, they have to be enabled in appropriate OS-specific header.
Same for HAVE_POSIX_IPC, but that one in fact can be useful on lot of
platforms. I've seen benchmark suggesting POSIX semaphores are 4 times
faster on Linux than SysV ones. It certainly applies to QNX4 as well and
makes emulation of SysV stuff unnecessary. I believe it could be extended to
support platforms like Darwin and BeOS which currently also rely on SysV
emulation. If there's interest from involved people, we could come up with a
better unified abstraction model and implementations for all supported
platforms...
I've been warned it might be considered too 'major' for 7.2, but please look
at the patch before judging. I tried my best to not break existing stuff,
all changes are only activated when explicitly enabled, QNX6-specific or
obviously compatible (the only 'unguarded' changes are typecasts for things
like SemId = -1, since its type is pointer in case of POSIX and fix for
broken QNX qsort() which I believe is already commited into CVS). I've spent
considerable time doing this and would really appreciate if it went into
7.2. Code builds and runs clean with or without any of above features
enabled.
The patch generated as recursive GNU-diff between original 7.2b2 and patched
(+ make distclean) top level directories, like 'diff -crP pgsql-original
pgsql-patched'.
It is big mostly because it contains whole new files for QNX6 (-P treats
missing files as empty).
regards,
- igor
Attachments:
pgsql.diffapplication/octet-stream; name=pgsql.diffDownload+2222-71
Here is the patch which adds following things to 7.2:
1. Support for QNX6 (builds cleanly on stock installation and passes all
regression tests).
There is no way we can put this in 7.2. We will keep it for 7.3.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026
I want to comment on this patch. The author is not happy to have all
his QNX6 work wait for 7.3 so I suggested he split the patch in two:
one part that is seen only by QNX and regression stuff, and a second
patch that adds Posix semaphore support. I think the first part can be
put into 7.2 if we can all be sure it is seen only by QNX.
FYI, we support QNX4, but not QNX6 at this point.
---------------------------------------------------------------------------
Here is the patch which adds following things to 7.2:
1. Support for QNX6 (builds cleanly on stock installation and passes all
regression tests).2. HAVE_POSIX_IPC feature, which if enabled switches implementation of
IpcSemaphoreXXX() (ipc.[ch]) to POSIX semaphores and mmap(). Enabled on QNX6
but should be useful for lot of platforms. Since IpcSemaphoreCreate() really
assumed SysV semaphores, I had to change its prototype *when* this feature
is enabled. That function is called from proc.c and spin.c, which were
patched accordingly for POSIX case (with #ifdef guards).3. USE_PTHREAD_MUTEXES feature, which if enabled implements S_LOCK stuff
with PTHREAD mutexes. It is useful (better than spinlocks) on non-SMP
systems when overhead of kernel call is small compared to overhead of
scheduling. Enabled on QNX6.4. USE_PTHREAD_SPINLOCKS feature which if enabled implements S_LOCK stuff
with PTHREAD spinlocks (may not be available on all platforms). It might be
better on SMP systems when hardware does not support TAS (in which case SysV
semaphores would be used as of now and it is hard to be worse than that).
MIPS systems come to mind (and QNX6 runs on them, so it will be enabled on
QNX6 in such cases).I haven't put checks for (2), (3) or (4) into configure to not break
supported platforms in unexpected ways. Benefits of (3) and (4) are really
OS and hardware dependent, so if you think they could be useful for your
platform, they have to be enabled in appropriate OS-specific header.Same for HAVE_POSIX_IPC, but that one in fact can be useful on lot of
platforms. I've seen benchmark suggesting POSIX semaphores are 4 times
faster on Linux than SysV ones. It certainly applies to QNX4 as well and
makes emulation of SysV stuff unnecessary. I believe it could be extended to
support platforms like Darwin and BeOS which currently also rely on SysV
emulation. If there's interest from involved people, we could come up with a
better unified abstraction model and implementations for all supported
platforms...I've been warned it might be considered too 'major' for 7.2, but please look
at the patch before judging. I tried my best to not break existing stuff,
all changes are only activated when explicitly enabled, QNX6-specific or
obviously compatible (the only 'unguarded' changes are typecasts for things
like SemId = -1, since its type is pointer in case of POSIX and fix for
broken QNX qsort() which I believe is already commited into CVS). I've spent
considerable time doing this and would really appreciate if it went into
7.2. Code builds and runs clean with or without any of above features
enabled.The patch generated as recursive GNU-diff between original 7.2b2 and patched
(+ make distclean) top level directories, like 'diff -crP pgsql-original
pgsql-patched'.
It is big mostly because it contains whole new files for QNX6 (-P treats
missing files as empty).regards,
- igor
[ Attachment, skipping... ]
---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026
----- Original Message -----
From: "Bruce Momjian" <pgman@candle.pha.pa.us>
To: "Igor Kovalenko" <Igor.Kovalenko@motorola.com>
Cc: <pgsql-patches@postgresql.org>
Sent: Friday, November 23, 2001 2:14 PM
Subject: Re: [PATCHES] Support for QNX6, POSIX IPC and PTHREAD-style locking
I want to comment on this patch. The author is not happy to have all
his QNX6 work wait for 7.3 so I suggested he split the patch in two:
one part that is seen only by QNX and regression stuff, and a second
patch that adds Posix semaphore support. I think the first part can be
put into 7.2 if we can all be sure it is seen only by QNX.
We had some more discussion with Bruce on the nature of changes.
Essentially, my point is that all the changes will only be seen on QNX6,
with or without Posix semaphore stuff, unless other platforms will
explicitly enable new code. I could simply use #ifdef __QNX__ everywhere and
not have this discussion, but since Posix semaphore code could be usable for
other platforms, I decided to use a 'feature key' rather than 'platform
key'. So the code is guarded by #ifdef HAVE_POSIX_IPC and can be enabled on
platform-specific basis. The patch will only enable it on QNX6 so it is as
'tight' as '#ifdef __QNX__' would be. Eventually maintainers of other
platforms can try it and enable if they like.
To prove my point I tried to build patched version on Linux 2.2 (Suse 6.4)
and Solaris (2.5/Sparc and 8/Intel). It compiled well and passed all
regression tests except geometry (which fails due to usual numeric precision
mismatches).
That of course still does not cover *all* platforms. I could probably try to
check on AIX and HP/UX but not all supported platforms anyway. I believe
however that if there were problems with patch, either Linux or Solaris
would expose them.
I also tried to use Posix semaphores on those platforms and it worked
perfectly well on Solaris 8 (it needed proper flags to compiler though). On
Linux Posix semaphores can not be shared between processes, so it is of no
use for postgres (unless new Linux versions fix that issue).
Guess it comes down to vote. I can also do what Bruce suggested (separate
patches) if I still did not convince everyone that patch won't hurt as is.
Please vote which way you like it (none, separate QNX6 & Posix sem,
combined).
regards,
- igor
FYI, we support QNX4, but not QNX6 at this point.
--------------------------------------------------------------------------
-
Here is the patch which adds following things to 7.2:
1. Support for QNX6 (builds cleanly on stock installation and passes all
regression tests).2. HAVE_POSIX_IPC feature, which if enabled switches implementation of
IpcSemaphoreXXX() (ipc.[ch]) to POSIX semaphores and mmap(). Enabled on
QNX6
but should be useful for lot of platforms. Since IpcSemaphoreCreate()
really
assumed SysV semaphores, I had to change its prototype *when* this
feature
is enabled. That function is called from proc.c and spin.c, which were
patched accordingly for POSIX case (with #ifdef guards).3. USE_PTHREAD_MUTEXES feature, which if enabled implements S_LOCK stuff
with PTHREAD mutexes. It is useful (better than spinlocks) on non-SMP
systems when overhead of kernel call is small compared to overhead of
scheduling. Enabled on QNX6.4. USE_PTHREAD_SPINLOCKS feature which if enabled implements S_LOCK
stuff
with PTHREAD spinlocks (may not be available on all platforms). It might
be
better on SMP systems when hardware does not support TAS (in which case
SysV
semaphores would be used as of now and it is hard to be worse than
that).
MIPS systems come to mind (and QNX6 runs on them, so it will be enabled
on
QNX6 in such cases).
I haven't put checks for (2), (3) or (4) into configure to not break
supported platforms in unexpected ways. Benefits of (3) and (4) are
really
OS and hardware dependent, so if you think they could be useful for your
platform, they have to be enabled in appropriate OS-specific header.Same for HAVE_POSIX_IPC, but that one in fact can be useful on lot of
platforms. I've seen benchmark suggesting POSIX semaphores are 4 times
faster on Linux than SysV ones. It certainly applies to QNX4 as well and
makes emulation of SysV stuff unnecessary. I believe it could be
extended to
support platforms like Darwin and BeOS which currently also rely on SysV
emulation. If there's interest from involved people, we could come up
with a
better unified abstraction model and implementations for all supported
platforms...I've been warned it might be considered too 'major' for 7.2, but please
look
at the patch before judging. I tried my best to not break existing
stuff,
all changes are only activated when explicitly enabled, QNX6-specific or
obviously compatible (the only 'unguarded' changes are typecasts for
things
like SemId = -1, since its type is pointer in case of POSIX and fix for
broken QNX qsort() which I believe is already commited into CVS). I've
spent
considerable time doing this and would really appreciate if it went into
7.2. Code builds and runs clean with or without any of above features
enabled.The patch generated as recursive GNU-diff between original 7.2b2 and
patched
Show quoted text
(+ make distclean) top level directories, like 'diff -crP pgsql-original
pgsql-patched'.
It is big mostly because it contains whole new files for QNX6 (-P treats
missing files as empty).regards,
- igor[ Attachment, skipping... ]
---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org-- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
"Igor Kovalenko" <Igor.Kovalenko@motorola.com> writes:
We had some more discussion with Bruce on the nature of changes.
Essentially, my point is that all the changes will only be seen on QNX6,
with or without Posix semaphore stuff, unless other platforms will
explicitly enable new code.
No, the point is that the Posix semaphore stuff is a major change to a
critical and delicate part of Postgres. It's too late in the 7.2 beta
cycle for such a change to receive the review and testing it needs.
We'll gladly consider it as a non-QNX-specific improvement for 7.3.
We won't ship it in 7.2, QNX-only or otherwise, because we don't have
any confidence in it yet (and no, we don't want to delay 7.2 release
for it either).
While I agree that a SysV emulation layer is ugly, it's by now a
well-tested technology: we've got several other platforms that do it
that way and have been known to work for a few releases. I have some
confidence that that same approach can be transposed into QNX6 and be
trustworthy enough to release with limited testing. I don't yet have
any confidence in changes at higher levels.
Also, I think that your patch as it stands is really only a halfway
measure; it makes the code uglier rather than cleaner. We probably
ought to revamp the higher-level APIs in such a way that either Posix
or SysV semaphores can be used to implement the APIs reasonably cleanly.
I'd prefer to design those changes and revise semaphore handling once,
not tweak it a little in this release and some more in the next.
Perhaps what this really means is that it's too late to think of
supporting QNX6 in PG 7.2 at all, and that we should target it for 7.3
instead. Considering that we're hoping to put out a final candidate
tarball next week, this train has pretty much left the station already.
regards, tom lane
Perhaps what this really means is that it's too late to think of
supporting QNX6 in PG 7.2 at all, and that we should target it for 7.3
instead. Considering that we're hoping to put out a final candidate
tarball next week, this train has pretty much left the station already.
Agreed. I think a good solution would be to put a patch on an ftp
server and have that available for QNX6 users to apply to 7.2.X.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026
----- Original Message -----
From: "Tom Lane" <tgl@sss.pgh.pa.us>
To: "Igor Kovalenko" <Igor.Kovalenko@motorola.com>
Cc: <pgsql-patches@postgresql.org>
Sent: Saturday, November 24, 2001 11:47 AM
Subject: Re: [PATCHES] Support for QNX6, POSIX IPC and PTHREAD-style locking
"Igor Kovalenko" <Igor.Kovalenko@motorola.com> writes:
We had some more discussion with Bruce on the nature of changes.
Essentially, my point is that all the changes will only be seen on QNX6,
with or without Posix semaphore stuff, unless other platforms will
explicitly enable new code.No, the point is that the Posix semaphore stuff is a major change to a
critical and delicate part of Postgres. It's too late in the 7.2 beta
cycle for such a change to receive the review and testing it needs.
I have feeling we're talking different languages. Can someone explain me how
does it need lot of review and testing if all changes are #ifdef-ed and
invisible to all currently supported platforms? I just want to understand
that point...
While I agree that a SysV emulation layer is ugly, it's by now a
well-tested technology: we've got several other platforms that do it
that way and have been known to work for a few releases. I have some
confidence that that same approach can be transposed into QNX6 and be
trustworthy enough to release with limited testing. I don't yet have
any confidence in changes at higher levels.
Does that mean you'd be ok with patch if it did not have Posix semaphores
and worked through sysV emulation? I could easily take that part out.
We already had similar discussion with Bruce and he was also objecting in
similar way until I explained the patch piece by piece and demonstrated why
I think it is harmless. Consider following pseudocode:
#ifdef HAVE_POSIX_IPC
// new code
#else
// unmodified old code
#endif
How does that shatter your confidence? Please educate me.
Also, I think that your patch as it stands is really only a halfway
measure; it makes the code uglier rather than cleaner. We probably
ought to revamp the higher-level APIs in such a way that either Posix
or SysV semaphores can be used to implement the APIs reasonably cleanly.
I'd prefer to design those changes and revise semaphore handling once,
not tweak it a little in this release and some more in the next.
Theoretically, yes and so far this is the only valid argument that I see.
Indeed it would be better to revamp IpcSemaphoreXXX() API to cover SysV,
Posix and BeOS semaphores. I wanted to do that, but I did not because I
anticipated concerns about changing existing code. So my goal was to change
as little of it as possible. I hoped it would help me to get it into 7.2,
but now you're pointing to it as to a mistake ;)
In practice you have to start with something and it is not always bad idea
to start with smaller steps. You may keep thinking about higher goals for a
year and nothing will happen until someone willing to spend his time on that
comes in and does it. I already spent mine and even went out of my way to
test it on various platforms. What I get for that is people refusing to
accept the patch without even reading it. That's really encouraging ...
Perhaps what this really means is that it's too late to think of
supporting QNX6 in PG 7.2 at all, and that we should target it for 7.3
instead. Considering that we're hoping to put out a final candidate
tarball next week, this train has pretty much left the station already.
I think I asked for formal vote. Delaying it for 7.3 means somebody will
have to essentially redo it and I did not see any volunteers popping up yet.
regards,
- igor
"Igor Kovalenko" <Igor.Kovalenko@motorola.com> writes:
No, the point is that the Posix semaphore stuff is a major change to a
critical and delicate part of Postgres. It's too late in the 7.2 beta
cycle for such a change to receive the review and testing it needs.
I have feeling we're talking different languages. Can someone explain me how
does it need lot of review and testing if all changes are #ifdef-ed and
invisible to all currently supported platforms?
It needs review and testing because we're not convinced that it's right.
We're not interested in shipping a possibly-unreliable QNX6 port just to
have a QNX6 port.
Also, patches that introduce a bunch of #ifdefs into what had been
non-system-specific code are disliked on general principles around this
project: they make the code harder to read and less maintainable over
the long run. In that sense the patch is going in the wrong direction.
There needs to be some work done on restructuring the existing code to
preserve readability.
I do believe that it's a good idea to support Posix semaphores; that's
been in the wind for awhile, and it's clear that QNX6 is not the only
platform that would benefit. We will take up this code, in some form,
in 7.3. But I don't think it's a wise idea to cram it into 7.2.
7.2 is already two months behind schedule, and I don't want to risk
any more delays in this release cycle.
... What I get for that is people refusing to
accept the patch without even reading it. That's really encouraging ...
I *have* read it. More than once. I'm not saying you've done bad work.
It's a great starting point, in fact. But I don't want to apply it
as-is, and I don't want to hold up 7.2 anymore in order to get QNX6
support into 7.2.
regards, tom lane
----- Original Message -----
From: "Tom Lane" <tgl@sss.pgh.pa.us>
To: "Igor Kovalenko" <Igor.Kovalenko@motorola.com>
Cc: <pgsql-patches@postgresql.org>
Sent: Sunday, November 25, 2001 12:08 PM
Subject: Re: [PATCHES] Support for QNX6, POSIX IPC and PTHREAD-style locking
"Igor Kovalenko" <Igor.Kovalenko@motorola.com> writes:
No, the point is that the Posix semaphore stuff is a major change to a
critical and delicate part of Postgres. It's too late in the 7.2 beta
cycle for such a change to receive the review and testing it needs.I have feeling we're talking different languages. Can someone explain me
how
does it need lot of review and testing if all changes are #ifdef-ed and
invisible to all currently supported platforms?It needs review and testing because we're not convinced that it's right.
We're not interested in shipping a possibly-unreliable QNX6 port just to
have a QNX6 port.
You're mixing issues related to QNX6 port per say and to Posix semaphores in
that statement. There is nothing more unreliable (even potentially) in QNX6
port than in some other platforms. Implementation of Posix semaphores could
be considered non-proven, but that's separate issue. I already sent separate
patch which deals only with QNX6 support (no Posix semaphores, no Pthread
locking). It works through sysV emulation and should not be considered any
less reliable than QNX4 port (emulation code was essentially inherited from
there). Apparently it is still awaiting approval to be distributed. Bruce
has copy though.
Also, patches that introduce a bunch of #ifdefs into what had been
non-system-specific code are disliked on general principles around this
project: they make the code harder to read and less maintainable over
the long run. In that sense the patch is going in the wrong direction.
There needs to be some work done on restructuring the existing code to
preserve readability.
You can not avoid #ifdefs completely. At some point somewhere you're bound
to have them. Of course it is better to have them encapsulated on lower
level. I believe that's what I did, except for those in proc.c & spin.c but
I already explained why I did that. So I don't think the patch goes in wrong
direction, it just goes half-way due to [intentional] compromise.
It could be structured better, but only at the cost of making more changes.
I do believe that it's a good idea to support Posix semaphores; that's
been in the wind for awhile, and it's clear that QNX6 is not the only
platform that would benefit. We will take up this code, in some form,
in 7.3. But I don't think it's a wise idea to cram it into 7.2.
7.2 is already two months behind schedule, and I don't want to risk
any more delays in this release cycle.... What I get for that is people refusing to
accept the patch without even reading it. That's really encouraging ...I *have* read it. More than once. I'm not saying you've done bad work.
It's a great starting point, in fact. But I don't want to apply it
as-is, and I don't want to hold up 7.2 anymore in order to get QNX6
support into 7.2.
Bruce said he'd accept patch without Posix semaphores. And I asked to vote
3-way (whole patch, qnx6-support-only or none). What you're saying appears
to be 'none' but you're not making it clear if that's 'no to whole patch',
'no to posix semaphores', 'no to qnx6-support-only patch' or 'no to
anything'.
My understanding at this point is, there should not be problem with applying
QNX6-support-only patch, as long as it works within existing framework and
does not introduce non-proven code anywhere. That's what new version of
patch is.
regards,
- igor
Bruce said he'd accept patch without Posix semaphores. And I asked to vote
3-way (whole patch, qnx6-support-only or none). What you're saying appears
to be 'none' but you're not making it clear if that's 'no to whole patch',
'no to posix semaphores', 'no to qnx6-support-only patch' or 'no to
anything'.My understanding at this point is, there should not be problem with applying
QNX6-support-only patch, as long as it works within existing framework and
does not introduce non-proven code anywhere. That's what new version of
patch is.
I will vote for the QNX-only version. However, I will not apply any
patch unless there is agreement from the group.
It is up the group to decide the issue.
However,even though I will vote for the QNX-only version, I am doing
this only because you feel so strongly something should be in 7.2. My
personal opinion is that this entire patch should wait for 7.3. It is
very late in 7.2 development and even a reviewed patch of this
complexity causes more risk than benefit.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026
----- Original Message -----
From: "Bruce Momjian" <pgman@candle.pha.pa.us>
To: "Igor Kovalenko" <Igor.Kovalenko@motorola.com>
Cc: <pgsql-patches@postgresql.org>
Sent: Sunday, November 25, 2001 6:34 PM
Subject: Re: [PATCHES] Support for QNX6, POSIX IPC and PTHREAD-style locking
Bruce said he'd accept patch without Posix semaphores. And I asked to
vote
3-way (whole patch, qnx6-support-only or none). What you're saying
appears
to be 'none' but you're not making it clear if that's 'no to whole
patch',
'no to posix semaphores', 'no to qnx6-support-only patch' or 'no to
anything'.My understanding at this point is, there should not be problem with
applying
QNX6-support-only patch, as long as it works within existing framework
and
does not introduce non-proven code anywhere. That's what new version of
patch is.I will vote for the QNX-only version. However, I will not apply any
patch unless there is agreement from the group.It is up the group to decide the issue.
However,even though I will vote for the QNX-only version, I am doing
this only because you feel so strongly something should be in 7.2. My
personal opinion is that this entire patch should wait for 7.3. It is
very late in 7.2 development and even a reviewed patch of this
complexity causes more risk than benefit.
Fair enough. I am asking you to get something into 7.2 for practical
reasons. Sooner people will get something, sooner someone will uncover
problems is there are any. That would allow to have reasonable confidence by
the time 7.3 rolled out.
Of course there are betas for such purpose, but people prefer to work with
releases, so I simply want it to have larger exposure. I also want clearer
test case - forcing people to use [potentially unstable] alphas/betas on
QNX6 would distort the picture - if there will be problems it won't be very
clear whether they are related to port or to the base code.
You can mark QNX6 support as 'experimental' if that will help anything.
thanks,
- igor
Igor Kovalenko writes:
Fair enough. I am asking you to get something into 7.2 for practical
reasons. Sooner people will get something, sooner someone will uncover
problems is there are any. That would allow to have reasonable confidence by
the time 7.3 rolled out.
The patch looks mostly harmless in the sense that it doesn't break
anything else, although some parts are clearly bogus, such as the patches
to 'configure' and 'resultmap' and some "comment out if you want xyz"
comments where there's nothing to comment out nearby. (Plus, commenting
stuff in and out in makefiles is not an acceptable practice to spread.)
CFLAGS in template/qnx6 should probably be -O2 unless you have reasons to
do otherwise, which should be documented. LIBS= has no business in the
template file. Overriding CC as done in port/qnx6/Makefile is not valid.
The SHLIB_LINK line in Makefile.shlib is not possibly correct. (The same
goes for most of the other SHLIB_LINK lines there, btw.) These issues are
"mostly harmless", but they would need to be fixed.
My mind on this is that we hope to put out the first *release candidate*
this week, which means, "if there are no more serious bugs, this is the
final release". This would mean that this patch would receive virtually
*no* testing before release. Surely I trust your word that says that this
patch makes PostgreSQL run correctly on your system, and it doesn't look
like it'll break anything else. But this kind of reasoning is not
responsible. PostgreSQL is, for better or worse, not developed by proving
that the code is theoretically correct; we allocate for extensive beta
testing because we know we need extensive beta testing. Exceptions are
always made, but a new feature has never qualified for such an exception.
"There will always be another release."
--
Peter Eisentraut peter_e@gmx.net
My reply went only to Peter. Can't get used to the fact that this list sends
replies to author rather than list ;)
----- Original Message -----
From: "Peter Eisentraut" <peter_e@gmx.net>
To: "Igor Kovalenko" <Igor.Kovalenko@motorola.com>
Cc: "Bruce Momjian" <pgman@candle.pha.pa.us>; <pgsql-patches@postgresql.org>
Sent: Monday, November 26, 2001 1:15 PM
Subject: Re: [PATCHES] Support for QNX6, POSIX IPC and PTHREAD-style locking
Igor Kovalenko writes:
Fair enough. I am asking you to get something into 7.2 for practical
reasons. Sooner people will get something, sooner someone will uncover
problems is there are any. That would allow to have reasonable
confidence by
the time 7.3 rolled out.
The patch looks mostly harmless in the sense that it doesn't break
anything else, although some parts are clearly bogus,
I am all for making it 'proper' if you elaborate a bit.
such as the patches
to 'configure' and 'resultmap'
How those are bogus? Configure needs to recognize system, otherwise it will
say Postgres has not been ported to your OS ....
Patch to resultmap is not as bogus as it may seem to you. Trouble is, the
QNX4 port plays against QNX6 there, by 'fake' matches which are not
appropriate for QNX6. That happens because host ID string (as produced by
config.guess) contains common part on both systems. Since I did not want to
touch other ports, I had little choice but to force matching with proper
files on QNX6, even if they are default files (otherwise QNX4-specific ones
were matched, producing bogus 'failures' of tests). You don't think I would
put things in there for no reason, would you?
and some "comment out if you want xyz"
comments where there's nothing to comment out nearby. (Plus, commenting
stuff in and out in makefiles is not an acceptable practice to spread.)
Those comments are leftover from Posix semaphores stuff. Those makefiles
would not need to be patched if Posix semaphores were used, but patches are
needed otherwise. If you can tell me how to handle that more elegantly, I am
all ears. Since now there's no posix semaphores in the patch, those comments
can be wiped out, I just did not think it would be such a bad sin to have
them there. Assuming there will be Posix semaphores eventually, some
comments would need to be there again...
CFLAGS in template/qnx6 should probably be -O2 unless you have reasons to
do otherwise, which should be documented.
It is somewhat problematic on QNX. Takes a lot of memory to compile and is
prone to 'internal compiler errors'. The default is -O anyway.
LIBS= has no business in the
template file.
I did not invent it. Template was copied from QNX4, how I was supposed to
know what has business there and what not. Univel and Windows have LIBS in
template too.
Overriding CC as done in port/qnx6/Makefile is not valid.
This one is my bad...
The SHLIB_LINK line in Makefile.shlib is not possibly correct. (The same
goes for most of the other SHLIB_LINK lines there, btw.) These issues are
SHLIB_LINK was copied from other platforms, most of which have -lc there.
Following the logic in comments, I added -lm -lsocket since they are
certainly needed on QNX6. If that's not possibly correct, you should educate
us all.
"mostly harmless", but they would need to be fixed.
My mind on this is that we hope to put out the first *release candidate*
this week, which means, "if there are no more serious bugs, this is the
final release". This would mean that this patch would receive virtually
*no* testing before release. Surely I trust your word that says that this
patch makes PostgreSQL run correctly on your system, and it doesn't look
like it'll break anything else. But this kind of reasoning is not
responsible. PostgreSQL is, for better or worse, not developed by proving
that the code is theoretically correct; we allocate for extensive beta
testing because we know we need extensive beta testing.
The patch was being tested for last couple of weeks by several people. The
fact that is passes regression tests should give some confidence too,
otherwise what is the point to have them at all.
Exceptions are
always made, but a new feature has never qualified for such an exception."There will always be another release."
Sure. I think I made it clear enough why I want it in 7.2. I also took away
the most interesting part of patch since there were some valid logical
objections against including it as is. I thought that would clear the way,
but now I am presented with unbeatable 'this is untested' reason. Who do you
think would be testing it if it was included in 7.3 or 8.0 for that matter?
Not you, I suspect. That would be same old me probably and why do you think
few weeks of testing by me and my fellows then will be somehow better than
same testing we've done now?
regards,
- igor
On Mon, Nov 26, 2001 at 09:39:42PM -0600, Igor Kovalenko wrote:
Exceptions are
always made, but a new feature has never qualified for such an exception."There will always be another release."
Sure. I think I made it clear enough why I want it in 7.2. I also took away
the most interesting part of patch since there were some valid logical
objections against including it as is. I thought that would clear the way,
but now I am presented with unbeatable 'this is untested' reason. Who do you
think would be testing it if it was included in 7.3 or 8.0 for that matter?
Not you, I suspect. That would be same old me probably and why do you think
few weeks of testing by me and my fellows then will be somehow better than
same testing we've done now?
Because if it _does_ break other peoples builds, it'll happen in an
unstable devlopment tree, which constitutes testing, and everyone just
says 'hmm, fix that'. If it gets in now, there's a chance it'll break
someone's build _no matter how much you've tested it_. If that happens
in a _stable release_, now we have a problem. PostgreSQL has a strong
track record of no 'brown paper bag' bugs in stable releases (well,
almost none). You seldom see a minor version number over 3 or 4, for
that reason. Logically 'proving' that your patch _couldn't possibly_
cause any problems has no impact on the matter: there are stranger build
environments out there than you've dreamed of, and someone has ported
PostgreSQL to most of them.
At this point, I'd suggest you yield gracefully: the core developers
don't want it right now, but have read your patch and given you valuable
feedback, to improve you submission for 7.3 (in about two weeks).
Ross
--
Ross Reedstrom, Ph.D. reedstrm@rice.edu
Executive Director phone: 713-348-6166
Gulf Coast Consortium for Bioinformatics fax: 713-348-6182
Rice University MS-39
Houston, TX 77005
"Ross J. Reedstrom" <reedstrm@rice.edu> writes:
... submission for 7.3 (in about two weeks).
That's probably overly optimistic; if things don't slip any more,
it's still unlikely that we'll open the 7.3 branch for development
before the first of the year. Traditionally we don't make the branch
immediately after release, because there are always a few bugs
justifying a dot-1 release that somehow sneak through beta testing.
It's easier to deal with these if we don't have to double-patch both
current and stable branches.
However, the fact that we always end up having to make a dot-1 release
is not for lack of trying. The dot-0 release is *supposed* to be
bulletproof.
regards, tom lane
On Tue, Nov 27, 2001 at 10:49:31AM -0500, Tom Lane wrote:
"Ross J. Reedstrom" <reedstrm@rice.edu> writes:
... submission for 7.3 (in about two weeks).
That's probably overly optimistic; if things don't slip any more,
it's still unlikely that we'll open the 7.3 branch for development
before the first of the year. Traditionally we don't make the branch
immediately after release, because there are always a few bugs
justifying a dot-1 release that somehow sneak through beta testing.
It's easier to deal with these if we don't have to double-patch both
current and stable branches.
I guess I was repeating something from earlier up the thread. However,
given how long this beta has been, we _might_ find very few things
to fix, so the 'double-patch' burden might be small. <finger crossed>
I know there are a lot of people holding patches for 7.3, as well. It
might be good to try to get it opened before the inter-holiday week,
since people often manage to find some free time then.
However, the fact that we always end up having to make a dot-1 release
is not for lack of trying. The dot-0 release is *supposed* to be
bulletproof.
And you've never got up to .16, that's for sure. ;-)
Ross
"Ross J. Reedstrom" <reedstrm@rice.edu> writes:
... submission for 7.3 (in about two weeks).
That's probably overly optimistic; if things don't slip any more,
it's still unlikely that we'll open the 7.3 branch for development
before the first of the year. Traditionally we don't make the branch
immediately after release, because there are always a few bugs
justifying a dot-1 release that somehow sneak through beta testing.
It's easier to deal with these if we don't have to double-patch both
current and stable branches.However, the fact that we always end up having to make a dot-1 release
is not for lack of trying. The dot-0 release is *supposed* to be
bulletproof.
Let me add that the number of patches that go into .1 and later releases
is decreasing, so we may be more aggresive this time in branching the
development tree earlier this time.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026
On Tue, Nov 27, 2001 at 10:49:31AM -0500, Tom Lane wrote:
"Ross J. Reedstrom" <reedstrm@rice.edu> writes:
... submission for 7.3 (in about two weeks).
That's probably overly optimistic; if things don't slip any more,
it's still unlikely that we'll open the 7.3 branch for development
before the first of the year. Traditionally we don't make the branch
immediately after release, because there are always a few bugs
justifying a dot-1 release that somehow sneak through beta testing.
It's easier to deal with these if we don't have to double-patch both
current and stable branches.I guess I was repeating something from earlier up the thread. However,
given how long this beta has been, we _might_ find very few things
to fix, so the 'double-patch' burden might be small. <finger crossed>
I know there are a lot of people holding patches for 7.3, as well. It
might be good to try to get it opened before the inter-holiday week,
since people often manage to find some free time then.
Yes, exactly what I suspect, and also my personal target date for a
branch.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026
Bruce Momjian <pgman@candle.pha.pa.us> writes:
Let me add that the number of patches that go into .1 and later releases
is decreasing, so we may be more aggresive this time in branching the
development tree earlier this time.
Maybe. Beta has been so quiet that I figure either (a) we've got a real
solid release here, or (b) hardly anyone's beta testing. If it's (a)
then the post-release period will be quiet too. If it's (b) ...
If things are still quiet two weeks after release then I'd be ready
to make the branch then. If not, we'll know we need more time.
regards, tom lane
Bruce Momjian <pgman@candle.pha.pa.us> writes:
Let me add that the number of patches that go into .1 and later releases
is decreasing, so we may be more aggresive this time in branching the
development tree earlier this time.Maybe. Beta has been so quiet that I figure either (a) we've got a real
solid release here, or (b) hardly anyone's beta testing. If it's (a)
then the post-release period will be quiet too. If it's (b) ...If things are still quiet two weeks after release then I'd be ready
to make the branch then. If not, we'll know we need more time.
I can't believe people aren't testing the beta. They have jumped all
over betas in the past. The only reason people wouldn't be testing is
because we are getting less popular, and I _know_ that isn't true.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026