Re: [HACKERS] pg_cancel_backend by non-superuser
2011/10/1 Tom Lane : > Daniel Farina writes: >> This patch would appear(?) to have languished: >> https://commitfest.postgresql.org/action/patch_view?id=541 > >> I'd really like to see it included. In the last comments of the >> review, there seem to be problems in *terminate* backend, but even >> just pg_cancel_backend as non-superuser would be just a huge >> improvement. What are the things blocking non-superuser >> pg_cancel_backend from being accepted? > > I think the reason the patch stalled is that we have not got consensus > on how far to extend the conditions under which these operations should > be allowed. For instance, in the last comment attached to that > commitfest entry, Noah alleges that a non-superuser database owner > should be allowed to kill a superuser's session, if it's connected > to his database. My reaction to that is somewhere between "no" and > "hell no"; IMO superusers can mess up non-superusers, never vice versa. > If I recall the discussion correctly, there were other points of > contention too. > Hi, the original patch allow only for the DB Owner to kill sessions owner by other users. This because in real world I have some production database where I'm not the DBA, but only the DB owner. I think that is not a good idea that a normal users is able to kill session from the same user because, unfortunally, in some real environment there are a lots of application that need to access to the same database and the same user is used. I know that is not a good practise but it is on the field For this reason I suppose that allow only to DB onwer to kill other sessions it is a good compromize between functionality and security, but is my personal opinion ... > I don't think we need more coding right now ... we need somebody to > write a spec that everyone can agree to. > > ISTM it would be reasonably non-controversial to allow users to issue > pg_cancel_backend against other sessions logged in as the same userID. > The question is whether to go further than that, and if so how much. > > 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 > -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_cancel_backend by non-superuser
Daniel Farina writes: > This patch would appear(?) to have languished: > https://commitfest.postgresql.org/action/patch_view?id=541 > I'd really like to see it included. In the last comments of the > review, there seem to be problems in *terminate* backend, but even > just pg_cancel_backend as non-superuser would be just a huge > improvement. What are the things blocking non-superuser > pg_cancel_backend from being accepted? I think the reason the patch stalled is that we have not got consensus on how far to extend the conditions under which these operations should be allowed. For instance, in the last comment attached to that commitfest entry, Noah alleges that a non-superuser database owner should be allowed to kill a superuser's session, if it's connected to his database. My reaction to that is somewhere between "no" and "hell no"; IMO superusers can mess up non-superusers, never vice versa. If I recall the discussion correctly, there were other points of contention too. I don't think we need more coding right now ... we need somebody to write a spec that everyone can agree to. ISTM it would be reasonably non-controversial to allow users to issue pg_cancel_backend against other sessions logged in as the same userID. The question is whether to go further than that, and if so how much. 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
Re: [HACKERS] Re: Optimizing pg_trgm makesign() (was Re: WIP: Fast GiST index build)
I wrote: > So what I'm thinking we ought to do is redefine things so that > initGISTstate sets fn_mcxt to a context that has the same lifespan as > the GISTSTATE itself does. We could possibly eliminate a few retail > pfree's in the process, eg by keeping the GISTSTATE itself in that same > context. > Having done that, what gtrgm_consistent is doing would be an officially > supported (dare I suggest even documented?) thing instead of a kluge, > and we could then adopt the same methodology in gtrgm_penalty. I've committed patches along this line. On my machine, the time needed to do the test case Heikki proposed (build a gist_trgm_ops index on the contents of /usr/share/dict/words) drops from around 29.9 seconds to about 17.3. makesign is now down in the noise according to oprofile, so I see no further reason to pursue the patch that started this thread. What's not in the noise is the memcmp() required to check if the cached makesign result is still good --- as best I can tell, that's near 50% of the remaining runtime. I don't see any non-invasive way to avoid that. If we were willing to redo the GiST support function API, we could fix it, but I'm not sure it's worth that. 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
[HACKERS] pg_cancel_backend by non-superuser
This patch would appear(?) to have languished: https://commitfest.postgresql.org/action/patch_view?id=541 I'd really like to see it included. In the last comments of the review, there seem to be problems in *terminate* backend, but even just pg_cancel_backend as non-superuser would be just a huge improvement. What are the things blocking non-superuser pg_cancel_backend from being accepted? -- fdr -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [REVIEW] pg_last_xact_insert_timestamp
On Fri, Sep 30, 2011 at 8:57 PM, Robert Haas wrote: > On Fri, Sep 30, 2011 at 3:22 PM, Simon Riggs wrote: >> If the feature could not be done another way, easily, I might agree. > > I don't see that you've offered a reasonable alternative. The > alternative proposals that you proposed don't appear to me to be > solving the same problem. AIUI, the requested feature is to be able > to get, on the master, the timestamp of the last commit or abort, just > as we can already get the timestamp of the last commit or abort > replayed on the standby. Nothing you WAL log and no messages you send > to the standby are going to accomplish that goal. The goal of the OP was to find out the replication delay. This function was suggested, but its not the only way. My alternative proposals solve the original problem in a better way. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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
Re: [HACKERS] [REVIEW] pg_last_xact_insert_timestamp
On Fri, Sep 30, 2011 at 3:22 PM, Simon Riggs wrote: > If the feature could not be done another way, easily, I might agree. I don't see that you've offered a reasonable alternative. The alternative proposals that you proposed don't appear to me to be solving the same problem. AIUI, the requested feature is to be able to get, on the master, the timestamp of the last commit or abort, just as we can already get the timestamp of the last commit or abort replayed on the standby. Nothing you WAL log and no messages you send to the standby are going to accomplish that goal. -- 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
Re: [HACKERS] [REVIEW] pg_last_xact_insert_timestamp
On Fri, Sep 30, 2011 at 8:11 PM, Robert Haas wrote: > On Fri, Sep 30, 2011 at 3:18 AM, Simon Riggs wrote: >> On Fri, Sep 30, 2011 at 3:24 AM, Kyotaro HORIGUCHI >> wrote: >> >>> Ok, I send this patch to comitters. >> >> I repeat my objection to this patch. I'm very sorry I haven't been >> around much in last few weeks to keep up a dialogue about this and to >> make it clear how wrong I think this is. >> >> Adding something onto the main path of transaction commit is not good, >> especially when the only purpose of it is to run an occasional >> monitoring query for those people that use replication. Not everybody >> uses replication and even people that do use it don't need to run it >> so frequently that every single commit needs this. This is just bloat >> that we do not need and can also easily avoid. > > I think the overhead of this is so completely trivial that we > shouldn't be concerned about it. It's writing 12 bytes to shared > memory for each commit, without taking a lock, in a cache line that > should be minimally contended. We write plenty of other data to > shared memory on each commit, and that's nowhere close to being the > expensive part of committing a transaction. What's expensive is > fighting over WALInsertLock and ProcArrayLock and CLogControlLock, and > possibly waiting for the commit to be durably flushed to disk, and > maybe (at the margin) wrenching the cache lines in our PGPROC away > from whatever processor last stole them to do GetSnapshotData(). I > don't think that a couple of stores to uncontended shared memory are > going to make any difference. If the feature could not be done another way, easily, I might agree. The point is it isn't necessary, useful or elegant to do it this way and *any* cycles added to mainline transactions need to have careful justification. And there really isn't any justification for doing things this way other than its the first way somebody thought of. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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
Re: [HACKERS] [REVIEW] pg_last_xact_insert_timestamp
On Fri, Sep 30, 2011 at 3:18 AM, Simon Riggs wrote: > On Fri, Sep 30, 2011 at 3:24 AM, Kyotaro HORIGUCHI > wrote: > >> Ok, I send this patch to comitters. > > I repeat my objection to this patch. I'm very sorry I haven't been > around much in last few weeks to keep up a dialogue about this and to > make it clear how wrong I think this is. > > Adding something onto the main path of transaction commit is not good, > especially when the only purpose of it is to run an occasional > monitoring query for those people that use replication. Not everybody > uses replication and even people that do use it don't need to run it > so frequently that every single commit needs this. This is just bloat > that we do not need and can also easily avoid. I think the overhead of this is so completely trivial that we shouldn't be concerned about it. It's writing 12 bytes to shared memory for each commit, without taking a lock, in a cache line that should be minimally contended. We write plenty of other data to shared memory on each commit, and that's nowhere close to being the expensive part of committing a transaction. What's expensive is fighting over WALInsertLock and ProcArrayLock and CLogControlLock, and possibly waiting for the commit to be durably flushed to disk, and maybe (at the margin) wrenching the cache lines in our PGPROC away from whatever processor last stole them to do GetSnapshotData(). I don't think that a couple of stores to uncontended shared memory are going to make any difference. > The calculation itself would be problematic since it differs from the > way standby_delay is calculated on the standby, which will cause much > confusion. Some thought or comment should be made about that also. The string standby_delay doesn't appear in our source tree anywhere, so I'm not sure what this is referring to. In any case, I'm in favor of this feature. Currently, the only way to measure the lag on the standby is to measure it in WAL bytes - and you get to write your own script to parse the WAL positions. This will allow people to measure it in minutes. That seems like a significant usability improvement. -- 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
Re: [HACKERS] Mismatch of relation names: pg_toast.pg_toast_nnn during pg_upgrade from 8.4 to 9.1
I regret that as a part-timer recently brought back on here I didn't get an opportunity to test this earlier. The upgrade with the patch worked fine on my first attempt. Thanks again, Jamie On Wed, Sep 28, 2011 at 7:32 PM, Bruce Momjian wrote: > Jamie Fox wrote: >> Thanks, I'm following the thread "pg_upgrade automatic testing" and >> will try the patch just detailed there. > > I have applied the patch to head and 9.1.X. We still have a win32 bug > to fix. It is a shame I was not able to fix these before 9.1.1 was > released. :-( > > --- > >> >> >> On Wed, Sep 28, 2011 at 12:50 AM, Peter Eisentraut wrote: >> > On tis, 2011-09-27 at 16:19 -0700, Jamie Fox wrote: >> >> >> >> It fails at this stage: >> >> >> >> ? ? Restoring user relation files >> >> ? ? linking /data/pgsql/prod-84/base/11564/2613 to >> >> /data/pgsql/prod-91/base/12698/12570 >> >> ? ? linking /data/pgsql/prod-84/base/11564/2683 to >> >> /data/pgsql/prod-91/base/12698/12572 >> >> ? ? Mismatch of relation names: database "prod1", old rel >> >> pg_toast.pg_toast_54542379, new rel pg_toast.pg_toast_16735 >> >> ? ? Failure, exiting >> > >> > This issue is known and a fix is currently being discussed. >> > >> > >> >> -- >> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) >> To make changes to your subscription: >> http://www.postgresql.org/mailpref/pgsql-hackers > > -- > Bruce Momjian http://momjian.us > EnterpriseDB http://enterprisedb.com > > + It's impossible for everything to be true. + > -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Optimizing pg_trgm makesign() (was Re: WIP: Fast GiST index build)
I wrote: > Alexander Korotkov writes: >> Isn't it possible to cache signature of newitem in gtrgm_penalty >> like gtrgm_consistent do this for query? > [ studies that code for awhile ... ] Ick, what a kluge. > The main problem with that code is that the cache data gets leaked at > the conclusion of a scan. Having just seen the consequences of leaking > the "giststate", I think this is something we need to fix not emulate. > I wonder whether it's worth having the GIST code create a special > scan-lifespan (or insert-lifespan) memory context that could be used > for cached data such as this? It's already creating a couple of > contexts for its own purposes, so one more might not be a big problem. > We'd have to figure out a way to make that context available to GIST > support functions, though, as well as something cleaner than fn_extra > for them to keep pointers in. I've been chewing on this for awhile and am now thinking that maybe what gtrgm_consistent is doing isn't that unreasonable. It's certainly legitimate for it to use fn_extra to maintain state between calls. The problem fundamentally is that when a function uses fn_extra to reference data it keeps in the fn_mcxt context, there's an implicit contract that fn_extra will survive for the same length of time that the fn_mcxt context does. (Otherwise there's no way for the function to avoid leaking that data after it's been called for the last time using that FmgrInfo.) And GIST is violating that assumption: it resets fn_extra when it does initGISTstate, but fn_mcxt gets set to CurrentMemoryContext, which in the problematic cases is a query-lifespan context that will still be around after the GIST indexscan is concluded. So what I'm thinking we ought to do is redefine things so that initGISTstate sets fn_mcxt to a context that has the same lifespan as the GISTSTATE itself does. We could possibly eliminate a few retail pfree's in the process, eg by keeping the GISTSTATE itself in that same context. Having done that, what gtrgm_consistent is doing would be an officially supported (dare I suggest even documented?) thing instead of a kluge, and we could then adopt the same methodology in gtrgm_penalty. Comments? 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
Re: [HACKERS] [v9.2] Fix Leaky View Problem
On Sun, Sep 25, 2011 at 11:22:56PM -0400, Robert Haas wrote: > On Sun, Sep 25, 2011 at 10:38 PM, Noah Misch wrote: > > On Sun, Sep 25, 2011 at 11:22:03AM -0500, Kevin Grittner wrote: > >> Robert Haas ?09/25/11 10:58 AM >>> > >> > >> > I'm not sure we've been 100% consistent about that, since we > >> > previously made CREATE OR REPLACE LANGUAGE not replace the owner > >> > with the current user. > >> > >> I think we've been consistent in *not* changing security on an > >> object when it is replaced. > > > >> [CREATE OR REPLACE FUNCTION does not change proowner or proacl] > > > > Good point. ?C-O-R VIEW also preserves column default values. ?I believe we > > are > > consistent to the extent that everything possible to specify in each C-O-R > > statement gets replaced outright. ?The preserved characteristics *require* > > commands like GRANT, COMMENT and ALTER VIEW to set in the first place. > > > > The analogue I had in mind is SECURITY DEFINER, which C-O-R FUNCTION > > reverts to > > SECURITY INVOKER if it's not specified each time. ?That default is safe, > > though, > > while the proposed default of security_barrier=false is unsafe. > > Even though I normally take the opposite position, I still like the > idea of dedicated syntax for this feature. Not knowing what view > options we might end up with in the future, I hate having to decide on > what the general behavior ought to be. But it would be easy to decide > that CREATE SECURITY VIEW followed by CREATE OR REPLACE VIEW leaves > the security flag set; it would be consistent with what we're doing > with owner and acl information and wouldn't back us into any > unpleasant decisions down the road. I prefer the previous UI (WITH (security_barrier=...)) to this proposal, albeit for diffuse reasons. Both kinds of views can have the consequence of granting new access to data. One kind leaks tuples to untrustworthy code whenever it's convenient for performance, and the other does not. A "non-security view" would not mimic either of these objects; it would be a mere subquery macro. Using WITH (...) syntax attached to the CREATE VIEW command better evokes the similarity between the alternatives we're actually offering. I also find it mildly odd letting CREATE OR REPLACE VIEW update an object originating with CREATE SECURITY VIEW. Unqualified CREATE VIEW will retain no redeeming value apart from backward compatibility; new applications with any concern for database-level security should use only security_barrier=true and mark functions LEAKPROOF as needed. nm -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] fix for pg_upgrade
Great, thanks! -- View this message in context: http://postgresql.1045698.n5.nabble.com/fix-for-pg-upgrade-tp3411128p4856336.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [REVIEW] pg_last_xact_insert_timestamp
On Fri, Sep 30, 2011 at 3:24 AM, Kyotaro HORIGUCHI wrote: > Ok, I send this patch to comitters. I repeat my objection to this patch. I'm very sorry I haven't been around much in last few weeks to keep up a dialogue about this and to make it clear how wrong I think this is. Adding something onto the main path of transaction commit is not good, especially when the only purpose of it is to run an occasional monitoring query for those people that use replication. Not everybody uses replication and even people that do use it don't need to run it so frequently that every single commit needs this. This is just bloat that we do not need and can also easily avoid. The calculation itself would be problematic since it differs from the way standby_delay is calculated on the standby, which will cause much confusion. Some thought or comment should be made about that also. If we want to measure times, we can easily send regular messages into WAL to provide this function. Using checkpoint records would seem frequent enough to me. We don't always send checkpoint records but we can send an info message instead if we are streaming. If archive_timeout is not set and max_wal_senders > 0 then we can send an info WAL message with timestamp on a regular cycle. Alternatively, we can send regular special messages from WALSender to WALReceiver that do not form part of the WAL stream, so we don't bulk up WAL archives. (i.e. don't use "w" messages). That seems like the most viable approach to this problem. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, 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
