PostmasterPid not marked with PGDLLIMPORT
Hi all,
While hacking a background worker for Windows/Linux that is sending
signals to the Postmaster depending on the state of the server where
Postgres is running (particularly after a certain size threshold is
reached on the partition of PGDATA SIGINT is sent to PostmasterPid to
have it stop cleanly), I have noticed that PostmasterPid is not marked
as PGDLLIMPORT in miscadmin.h, so I cannot use the field directly in
this background worker for Windows... Bypassing this problem can be
done by parsing postmaster.pid but adding this extra logic is really
annoying as this requires a one-line change upstream...
Could it be possible to mark PostmasterPid with PGDLLIMPORT on HEAD
and back-branches?
Regards,
--
Michael
Attachments:
mark-postpid-pgdllimport.patchtext/x-diff; charset=US-ASCII; name=mark-postpid-pgdllimport.patchDownload
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 356fcc6..78545da 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -143,7 +143,7 @@ do { \
/*
* from utils/init/globals.c
*/
-extern pid_t PostmasterPid;
+extern PGDLLIMPORT pid_t PostmasterPid;
extern bool IsPostmasterEnvironment;
extern PGDLLIMPORT bool IsUnderPostmaster;
extern bool IsBackgroundWorker;
On 1 June 2016 at 11:48, Michael Paquier <michael.paquier@gmail.com> wrote:
Could it be possible to mark PostmasterPid with PGDLLIMPORT on HEAD
and back-branches?
Sounds sensible to me.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
On Wed, Jun 1, 2016 at 12:06 AM, Craig Ringer <craig@2ndquadrant.com> wrote:
On 1 June 2016 at 11:48, Michael Paquier <michael.paquier@gmail.com> wrote:
Could it be possible to mark PostmasterPid with PGDLLIMPORT on HEAD
and back-branches?Sounds sensible to me.
I don't really want to set a precedent that we'll back-patch
PGDLLIMPORT markings every time somebody needs a new symbol for some
extension they are writing, but I don't mind changing this in master.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Jun 1, 2016 at 9:00 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Jun 1, 2016 at 12:06 AM, Craig Ringer <craig@2ndquadrant.com>
wrote:On 1 June 2016 at 11:48, Michael Paquier <michael.paquier@gmail.com>
wrote:
Could it be possible to mark PostmasterPid with PGDLLIMPORT on HEAD
and back-branches?Sounds sensible to me.
+1
I don't really want to set a precedent that we'll back-patch
PGDLLIMPORT markings every time somebody needs a new symbol for some
extension they are writing
[...]
-1
David J.
Robert Haas wrote:
On Wed, Jun 1, 2016 at 12:06 AM, Craig Ringer <craig@2ndquadrant.com> wrote:
On 1 June 2016 at 11:48, Michael Paquier <michael.paquier@gmail.com> wrote:
Could it be possible to mark PostmasterPid with PGDLLIMPORT on HEAD
and back-branches?Sounds sensible to me.
I don't really want to set a precedent that we'll back-patch
PGDLLIMPORT markings every time somebody needs a new symbol for some
extension they are writing, but I don't mind changing this in master.
I wonder why is that -- just to reduce the commit load? I don't think
this kind of change is likely to break anything, is it?
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Jun 1, 2016 at 12:24 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
Robert Haas wrote:
On Wed, Jun 1, 2016 at 12:06 AM, Craig Ringer <craig@2ndquadrant.com> wrote:
On 1 June 2016 at 11:48, Michael Paquier <michael.paquier@gmail.com> wrote:
Could it be possible to mark PostmasterPid with PGDLLIMPORT on HEAD
and back-branches?Sounds sensible to me.
I don't really want to set a precedent that we'll back-patch
PGDLLIMPORT markings every time somebody needs a new symbol for some
extension they are writing, but I don't mind changing this in master.I wonder why is that -- just to reduce the commit load? I don't think
this kind of change is likely to break anything, is it?
Probably not, but yes, I do want to reduce the commit load. I also
think that we essentially have a contract with our users to limit what
we back-patch to critical bug fixes and security fixes. When we don't
do that, people start asking to have individual fixes cherry-picked
instead of just upgrading, and that's not good. We may know that such
changes are low-risk, but that doesn't mean everyone else does.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Jun 1, 2016 at 5:04 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Jun 1, 2016 at 12:24 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:Robert Haas wrote:
On Wed, Jun 1, 2016 at 12:06 AM, Craig Ringer <craig@2ndquadrant.com>
wrote:
On 1 June 2016 at 11:48, Michael Paquier <michael.paquier@gmail.com>
wrote:
Could it be possible to mark PostmasterPid with PGDLLIMPORT on HEAD
and back-branches?Sounds sensible to me.
I don't really want to set a precedent that we'll back-patch
PGDLLIMPORT markings every time somebody needs a new symbol for some
extension they are writing, but I don't mind changing this in master.I wonder why is that -- just to reduce the commit load? I don't think
this kind of change is likely to break anything, is it?Probably not, but yes, I do want to reduce the commit load. I also
think that we essentially have a contract with our users to limit what
we back-patch to critical bug fixes and security fixes. When we don't
do that, people start asking to have individual fixes cherry-picked
instead of just upgrading, and that's not good. We may know that such
changes are low-risk, but that doesn't mean everyone else does.
Are there two concerns here? One, that people will think we are
back-patching stuff and destabilizing back-branches, and two, that people
will see increase back-patching and therefore make unreasonable requests of
us to which we dislike saying "no". The later doesn't seem likely, and I'd
say you can't stop people from having badly formed opinions and that our
track record on back-patching decisions is excellent.
We want third-party tools
to support our prior releases and if miss making one of our features
available to Windows because of a missing PGDLLIMPORT that's on our heads
and should be fixed. If a user equates that to "please batch-patch jsonb
to 9.3 because I don't want to upgrade" I'm not going to feel much guilt
saying "that's different, ain't gonna happen". Informed people will
understand the purpose of the back-patch and until I start hearing a vocal
uninformed person start griping I'd rather give the community the benefit
of the doubt.
David J.
On Wed, Jun 1, 2016 at 5:04 PM, Robert Haas <robertmhaas@gmail.com> wrote:
Probably not, but yes, I do want to reduce the commit load. I also
think that we essentially have a contract with our users to limit what
we back-patch to critical bug fixes and security fixes. When we don't
do that, people start asking to have individual fixes cherry-picked
instead of just upgrading, and that's not good. We may know that such
changes are low-risk, but that doesn't mean everyone else does.
I suggest that there's a more principled reason for refusing a back-patch
here, which is that we don't back-patch new features, only bug fixes.
This request is certainly not a bug fix. It's in support of a new feature
--- and one that's not even ours, but a third-party extension.Considering that said extension isn't finished yet, it's hard to guess
whether this will be the only blocking factor preventing it from working
with older versions, but it might well be that there are other problems.
Also, even if it would work, the author would be reduced to saying things
like "it will work in 9.4, if it's 9.4.9 or later". Keeping track of that
level of detail is no fun either for the author or for users. It'd be
a lot simpler all around to just say "my spiffy new extension requires 9.6
or later".
In short, I'd vote for putting this change in HEAD, but I see no need to
back-patch.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Jun 1, 2016 at 5:40 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
On Wed, Jun 1, 2016 at 5:04 PM, Robert Haas <robertmhaas@gmail.com>
wrote:
Probably not, but yes, I do want to reduce the commit load. I also
think that we essentially have a contract with our users to limit what
we back-patch to critical bug fixes and security fixes. When we don't
do that, people start asking to have individual fixes cherry-picked
instead of just upgrading, and that's not good. We may know that such
changes are low-risk, but that doesn't mean everyone else does.I suggest that there's a more principled reason for refusing a back-patch here, which is that we don't back-patch new features, only bug fixes. This request is certainly not a bug fix. It's in support of a new feature --- and one that's not even ours, but a third-party extension.
Maybe I don't understand PGDLLEXPORT...
The PostgreSQL function/feature in question is already in place and can be
accessed by someone using Linux or other unix-like variant. But it cannot
be access by our Window's users because we failed to add a PGDLLEXPORT
somewhere. If it is our goal to treat Windows and Linux/Unix equally then
that discrepancy is on its face a bug. The fact we don't catch these until
some third-party points it out doesn't make it any less a bug.
Considering that said extension isn't finished yet, it's hard to guess
whether this will be the only blocking factor preventing it from working
with older versions, but it might well be that there are other problems.
From my prior point the reason someone wants to use the unexposed but
documented public API shouldn't matter.
Also, even if it would work, the author would be reduced to saying things
like "it will work in 9.4, if it's 9.4.9 or later". Keeping track of that
level of detail is no fun either for the author or for users.
This seems hollow. "It works on the latest version of all officially
supported PostgreSQL releases" is a reasonable policy for a third-party
application to take. That it might work on older releases would be a
bonus for some new users.
It'd be
a lot simpler all around to just say "my spiffy new extension requires 9.6
or later".
And it makes the available user base considerably smaller. A small change
to get the other 80% of the users is something to take seriously.
In short, I'd vote for putting this change in HEAD, but I see no need to
back-patch.
I see a need for back-patching and no technical reason why it cannot be
done - easily.
David J.
On Thu, Jun 2, 2016 at 6:40 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I suggest that there's a more principled reason for refusing a back-patch here, which is that we don't back-patch new features, only bug fixes. This request is certainly not a bug fix. It's in support of a new feature --- and one that's not even ours, but a third-party extension.
Yes, that's not a bug fix. I agree on that.
Considering that said extension isn't finished yet, it's hard to guess
whether this will be the only blocking factor preventing it from working
with older versions, but it might well be that there are other problems.
Also, even if it would work, the author would be reduced to saying things
like "it will work in 9.4, if it's 9.4.9 or later". Keeping track of that
level of detail is no fun either for the author or for users.
The extension in this case is for 9.4, for a product yet to be
released that is now on 9.4.8, so I don't care much about the support
grid here to be honest.
It'd be a lot simpler all around to just say "my spiffy new extension requires 9.6
or later".
Well, that's the only factor as far as I saw that prevented me to use
this extension on Windows. But I won't fight your might regarding a
backpatch, wearing the burden of a custom patch applied to miscadmin.h
for REL9_4_STABLE is not huge: this code never changes.
In short, I'd vote for putting this change in HEAD, but I see no need to
back-patch.
OK, fine for me.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Jun 1, 2016 at 5:59 PM, David G. Johnston
<david.g.johnston@gmail.com> wrote:
Maybe I don't understand PGDLLEXPORT...
We're talking about PGDLLIMPORT.
The PostgreSQL function/feature in question is already in place and can be
accessed by someone using Linux or other unix-like variant. But it cannot
be access by our Window's users because we failed to add a PGDLLEXPORT
somewhere. If it is our goal to treat Windows and Linux/Unix equally then
that discrepancy is on its face a bug. The fact we don't catch these until
some third-party points it out doesn't make it any less a bug.
If we had a policy of putting PGDLLIMPORT on everything, I'd agree
with you, but we clearly don't. Something's only a bug if we intended
A but accidentally got B. If we intended and got A and somebody
doesn't like that, that's not a bug; that's a difference of opinion.
I personally feel that we should sprinkle PGDLLIMPORT markings onto a
lot more things, but Tom Lane has opposed that at every turn. I hope
we'll change our policy about that someday, but that's a different
question from whether such changes should be back-patched.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Jun 1, 2016 at 7:39 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
In short, I'd vote for putting this change in HEAD, but I see no need to
back-patch.OK, fine for me.
Done.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Jun 3, 2016 at 1:55 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Jun 1, 2016 at 5:59 PM, David G. Johnston
<david.g.johnston@gmail.com> wrote:Maybe I don't understand PGDLLEXPORT...
We're talking about PGDLLIMPORT.
Typo, was thinking "we export this for others to consume"...
The PostgreSQL function/feature in question is already in place and can
be
accessed by someone using Linux or other unix-like variant. But it
cannot
be access by our Window's users because we failed to add a PGDLLEXPORT
somewhere. If it is our goal to treat Windows and Linux/Unix equallythen
that discrepancy is on its face a bug. The fact we don't catch these
until
some third-party points it out doesn't make it any less a bug.
If we had a policy of putting PGDLLIMPORT on everything, I'd agree
with you, but we clearly don't. Something's only a bug if we intended
A but accidentally got B. If we intended and got A and somebody
doesn't like that, that's not a bug; that's a difference of opinion.
I find it a stretch that there is intent involved here. Your comments
below further that belief.
I personally feel that we should sprinkle PGDLLIMPORT markings onto a
lot more things, but Tom Lane has opposed that at every turn. I hope
we'll change our policy about that someday, but that's a different
question from whether such changes should be back-patched.
Different but related.
I'm sure the opinion I've expressed has supporters but their isn't enough
bashing of us by the community at large that this is likely to get the
decision makers to switch their feelings.
David J.
On Sat, Jun 4, 2016 at 2:56 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Jun 1, 2016 at 7:39 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:In short, I'd vote for putting this change in HEAD, but I see no need to
back-patch.OK, fine for me.
Done.
Thanks you.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers