Re: [HACKERS] testing HS/SR - 1 vs 2 performance
On Thu, 2010-04-22 at 08:57 +0300, Heikki Linnakangas wrote: > > > > I think the assert is a good idea. If there's no real problem here, > > the assert won't trip. It's just a safety precaution. > > Right. And assertions also act as documentation, they are a precise and > compact way to document invariants we assume to hold. A comment > explaining why the cyclic nature of XIDs is not a problem would be nice > too, in addition or instead of the assertions. Agreed. I was going to reply just that earlier but have been distracted on other things. -- Simon Riggs www.2ndQuadrant.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] should I post the patch as committed?
On Tue, Apr 20, 2010 at 10:41 PM, Alvaro Herrera wrote: > > > > I think committing a patch from a non-regular is a special case and > attaching the modified patch is reasonable in that case. > > My 8.8 Richter ... > > Or may be just mention the commit id for easy look up in the git log. Thanks, Pavan -- Pavan Deolasee EnterpriseDB http://www.enterprisedb.com
Re: [HACKERS] testing HS/SR - 1 vs 2 performance
Robert Haas wrote: > On Wed, Apr 21, 2010 at 9:37 AM, Simon Riggs wrote: >> On Wed, 2010-04-21 at 15:27 +0300, Heikki Linnakangas wrote: >> >>> Given the discussion about the cyclic nature of XIDs, it would be good >>> to add an assertion that when a new XID is added to the array, it is >>> >>> a) larger than the biggest value already in the array >>> (TransactionIdFollows(new, head)), and >>> b) not too far from the smallest value in the array to confuse binary >>> search (TransactionIdFollows(new, tail)). >> We discussed this in November. You convinced me it isn't possible for >> older xids to stay in the standby because anti-wraparound vacuums would >> conflict and kick them out. The primary can't run with wrapped xids and >> neither can the standby. I think that is correct. >> >> Adding an assertion isn't going to do much because it's unlikely anybody >> is going to be running for 2^31 transactions with asserts enabled. >> >> Worrying about things like this seems strange when real and negative >> behaviours are right in our faces elsewhere. Performance and scalability >> are real world concerns. > > I think the assert is a good idea. If there's no real problem here, > the assert won't trip. It's just a safety precaution. Right. And assertions also act as documentation, they are a precise and compact way to document invariants we assume to hold. A comment explaining why the cyclic nature of XIDs is not a problem would be nice too, in addition or instead of the assertions. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.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] libpq connectoin redirect
feng tian wrote: > Hi, > > I want to load balance a postgres server on 4 physical machines, say > 127.0.0.11-14. I can set up a pgbouncer on 127.0.0.10 and connection pooling > to my four boxes. However, the traffic from/to clients will go through an > extra hop. Another way to do this, is to send the client an "redirect" > message. When client connect to 127.0.0.10, instead of accepting the > connection, it can reply to client telling it to reconnect to one of the > server on 127.0.0.11-14. One common way to do that is to set up one DNS entry for those 4 IP addresses. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.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] extended operator classes vs. type interfaces
On Fri, 2010-04-16 at 23:18 -0700, Scott Bailey wrote: > Well I've been doing a lot of work with range abstract data types in > Oracle lately. And I've got to say that the OO features in Oracle make > it really nice. Of course its Oracle, so its like a half baked OO in > cobol syntax, lol. But I for one think it would be great if Postgres had > object data types that had methods and could be subclassed. That's interesting. I think the most critical piece of that is "subclassing" in the sense of a type interface. There have already been a couple threads about type interfaces: http://archives.postgresql.org/pgsql-hackers/2009-10/msg01403.php http://archives.postgresql.org/pgsql-hackers/2010-04/msg00443.php Regards, Jeff Davis -- 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] don't allow walsender to consume superuser_reserved_connection slots, or during shutdown
On Thu, Apr 22, 2010 at 11:01 AM, Tom Lane wrote:
> Robert Haas writes:
>> Here's the fine patch. The actual code changes are simple and seem to
>> work as expected, but I struggled a bit with the phrasing of the
>> messages. Feel free to suggest improvements.
>
> Stick with the original wording? I don't really see a need to change it.
How about?:
if ((!am_superuser || am_walsender) &&
ReservedBackends > 0 &&
!HaveNFreeProcs(ReservedBackends))
{
if (am_walsender)
ereport(FATAL,
(errcode(ERRCODE_TOO_MANY_CONNECTIONS),
errmsg("remaining connection slots are reserved for
non-replication superuser connections")));
else
ereport(FATAL,
(errcode(ERRCODE_TOO_MANY_CONNECTIONS),
errmsg("connection limit exceeded for
non-superusers")));
}
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
--
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] don't allow walsender to consume superuser_reserved_connection slots, or during shutdown
Robert Haas writes: > Here's the fine patch. The actual code changes are simple and seem to > work as expected, but I struggled a bit with the phrasing of the > messages. Feel free to suggest improvements. Stick with the original wording? I don't really see a need to change it. 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] [GENERAL] trouble with to_char('L')
Takahiro Itagaki wrote: > Revised patch attached. Please test it. I applied this version of the patch. Please check wheter the bug is fixed and any buildfarm failures. Regards, --- Takahiro Itagaki NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] don't allow walsender to consume superuser_reserved_connection slots, or during shutdown
On Wed, Apr 21, 2010 at 1:56 PM, Tom Lane wrote: > Robert Haas writes: >> ...shouldn't we move the "tests", plural, rather than just the one? >> It seems right to reject new SR connections during shutdown. > > Yeah; you'd also need to adjust both of them to consider am_walsender. > (IOW, we want to treat SR connections as non-superuser for both tests.) [ subject changed, recipient list trimmed ] Here's the fine patch. The actual code changes are simple and seem to work as expected, but I struggled a bit with the phrasing of the messages. Feel free to suggest improvements. Also, I wasn't sure if there was somewhere in the documentation where we discussed the restriction that only superusers can connect during shutdown. If there is such a place, we should update that, too. ...Robert superuser_is_not_enuf.patch Description: Binary data -- 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] libpq connectoin redirect
On Apr 20, 2010, at 10:03 PM, feng tian wrote: > Another way to do this, is to send the client an "redirect" message. When > client connect to 127.0.0.10, instead of accepting the connection, it can > reply to client telling it to reconnect to one of the server on > 127.0.0.11-14. ISTM that this would be better handled at a higher-level. That is, given a server (127.0.0.10) that holds 127.0.0.11-14. Connect to that server and query for the correct target host. -- 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] libpq connectoin redirect
feng tian wrote: Hi, I want to load balance a postgres server on 4 physical machines, say 127.0.0.11-14. I can set up a pgbouncer on 127.0.0.10 and connection pooling to my four boxes. However, the traffic from/to clients will go through an extra hop. Another way to do this, is to send the client an "redirect" message. When client connect to 127.0.0.10, instead of accepting the connection, it can reply to client telling it to reconnect to one of the server on 127.0.0.11-14. I am planning to write/submit a patch to do that. I wonder if there is similar effort in extending libpq protocol, or, if you have better ideas on how to achieve this. how do you plan on maintaining consistency, transactional integrity and atomicity of updates across these 4 machines? -- 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] plpgsql GUC variable: custom or built-in?
I wrote: > Well, if there are no other comments, I'll push forward with the fix > proposed here: > http://archives.postgresql.org/pgsql-hackers/2009-11/msg00531.php Done. I did not make the change I speculated about of allowing completely unknown variables (those that don't even match custom_variable_classes) to be set by superusers. It would be a very minor tweak to the committed code to allow that, but I'm not convinced that making a corner case in dump/restore slightly easier is worth the loss of error checking. In practice, if you have ALTER ... SETs for custom variables, you'd better list their modules in custom_variable_classes, or it won't work nicely. I see no really strong reason not to fix that parameter before you restore instead of after. 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] libpq connectoin redirect
Hi, I want to load balance a postgres server on 4 physical machines, say 127.0.0.11-14. I can set up a pgbouncer on 127.0.0.10 and connection pooling to my four boxes. However, the traffic from/to clients will go through an extra hop. Another way to do this, is to send the client an "redirect" message. When client connect to 127.0.0.10, instead of accepting the connection, it can reply to client telling it to reconnect to one of the server on 127.0.0.11-14. I am planning to write/submit a patch to do that. I wonder if there is similar effort in extending libpq protocol, or, if you have better ideas on how to achieve this. Thank you, Feng _ The New Busy think 9 to 5 is a cute idea. Combine multiple calendars with Hotmail. http://www.windowslive.com/campaign/thenewbusy?tile=multicalendar&ocid=PID28326::T:WLMTAGL:ON:WL:en-US:WM_HMP:042010_5
Re: [HACKERS] Should database = all in pg_hba.conf match a replication connection?
On Tue, 2010-04-20 at 19:06 -0400, Tom Lane wrote: > Should we change this? It seems to me to be a good thing on security > grounds if replication connections can't be made through a generic > pg_hba entry. That's a good change. -- Simon Riggs www.2ndQuadrant.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] [RFC] nodeToString format and exporting the SQL parser
2010/4/21 Jehan-Guillaume (ioguix) de Rorthais :
> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
>
> On 04/04/2010 18:10, David Fetter wrote:
>> On Sat, Apr 03, 2010 at 03:17:30PM +0200, Markus Schiltknecht wrote:
>>> Hi,
>>>
>>> Michael Tharp wrote:
I have been spending a little time making the internal SQL parser
available to clients via a C-language SQL function.
>>>
>>> This sounds very much like one of the Cluster Features:
>>> http://wiki.postgresql.org/wiki/ClusterFeatures#API_into_the_Parser_.2F_Parser_as_an_independent_module
>>>
>>> Is this what you (or David) have in mind?
>>
>> I'm not a fan of statement-based replication of any description. The
>> use cases I have in mind involve things like known-correct syntax
>> highlighting in text editors.
>
> The point here is not to expose the internal data structure, but to
> deliver a tokenized version of the given SQL script.
>
> There's actually many different use cases for external projects :
> - syntax highlighting
> - rewrite query with proper indentation
> - replication
> - properly splitting queries from a script
> - define type of the query (SELECT ? UPDATE/DELETE ? DDL ?)
> - checking validity of a query before sending it
> - ...
>
> In addition of PgPool needs, I can see 3 or 4 direct use cases for
> pgAdmin and phpPgAdmin.
>
> So it seems to me having the parser code in a shared library would be
> very useful for external C projects which can link to it. However it
> would be useless for other non-C projects which can't use it directly
> but are connected to a PostgreSQL backend anyway (phpPgAdmin as instance).
>
> What about having a new SQL command like TOKENIZE ? it would kinda act
> like EXPLAIN but giving a tokenized version of the given SQL script. As
> EXPLAIN, it could speak XML, YAML, JSON, you name it...
>
> Each token could have :
> - a type ('identifier', 'string', 'sql command', 'sql keyword',
> 'variable'...)
> - the start position in the string
> - the value
> - the line number
> - ...
>
> A simple example of a tokenizer is the php one:
> http://fr.php.net/token_get_all
>
> And here is a basic example which return pseudo rows here :
>
> => TOKENIZE $script$
> SELECT 1;
> UPDATE test SET "a"=2;
> $script$;
>
you don't need special command for this task .. function is enough
new SQL command is useless
http://www.pgsql.cz/index.php/Oracle_functionality_%28en%29#PLVlex
it can be very simple with new changes in parser.
Regards
Pavel Stehule
> type | pos | value | line
> - -+-+--+--
> SQL_COMMAND | 1 | 'SELECT' | 1
> CONSTANT | 8 | '1' | 1
> DELIMITER | 9 | ';' | 1
> SQL_COMMAND | 11 | 'UPDATE' | 2
> IDENTIFIER | 18 | 'test' | 2
> SQL_KEYWORD | 23 | 'SET' | 2
> IDENTIFIER | 27 | '"a"' | 2
> OPERATOR | 30 | '=' | 2
> CONSTANT | 31 | '1' | 2
>
>>
>> Cheers,
>> David.
>
> As a phpPgAdmin dev, I am thinking about this subject since a long time.
> I am interested about trying to create such a patch after discussing it
> and if you think it is doable.
>
> - --
> JGuillaume (ioguix) de Rorthais
> http://www.dalibo.com
> -BEGIN PGP SIGNATURE-
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkvPOJMACgkQxWGfaAgowiLrUACfa7qMVr3oiOVS7JfhTa1S9EqY
> pYkAn3Sj6cezC/EdWPu2+kzrgjaDygGE
> =oY1c
> -END PGP SIGNATURE-
>
> --
> 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] [DOCS] Streaming replication document improvements
Robert Haas writes: > ...shouldn't we move the "tests", plural, rather than just the one? > It seems right to reject new SR connections during shutdown. Yeah; you'd also need to adjust both of them to consider am_walsender. (IOW, we want to treat SR connections as non-superuser for both tests.) 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] [RFC] nodeToString format and exporting the SQL parser
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 04/04/2010 18:10, David Fetter wrote:
> On Sat, Apr 03, 2010 at 03:17:30PM +0200, Markus Schiltknecht wrote:
>> Hi,
>>
>> Michael Tharp wrote:
>>> I have been spending a little time making the internal SQL parser
>>> available to clients via a C-language SQL function.
>>
>> This sounds very much like one of the Cluster Features:
>> http://wiki.postgresql.org/wiki/ClusterFeatures#API_into_the_Parser_.2F_Parser_as_an_independent_module
>>
>> Is this what you (or David) have in mind?
>
> I'm not a fan of statement-based replication of any description. The
> use cases I have in mind involve things like known-correct syntax
> highlighting in text editors.
The point here is not to expose the internal data structure, but to
deliver a tokenized version of the given SQL script.
There's actually many different use cases for external projects :
- syntax highlighting
- rewrite query with proper indentation
- replication
- properly splitting queries from a script
- define type of the query (SELECT ? UPDATE/DELETE ? DDL ?)
- checking validity of a query before sending it
- ...
In addition of PgPool needs, I can see 3 or 4 direct use cases for
pgAdmin and phpPgAdmin.
So it seems to me having the parser code in a shared library would be
very useful for external C projects which can link to it. However it
would be useless for other non-C projects which can't use it directly
but are connected to a PostgreSQL backend anyway (phpPgAdmin as instance).
What about having a new SQL command like TOKENIZE ? it would kinda act
like EXPLAIN but giving a tokenized version of the given SQL script. As
EXPLAIN, it could speak XML, YAML, JSON, you name it...
Each token could have :
- a type ('identifier', 'string', 'sql command', 'sql keyword',
'variable'...)
- the start position in the string
- the value
- the line number
- ...
A simple example of a tokenizer is the php one:
http://fr.php.net/token_get_all
And here is a basic example which return pseudo rows here :
=> TOKENIZE $script$
SELECT 1;
UPDATE test SET "a"=2;
$script$;
type | pos | value | line
- -+-+--+--
SQL_COMMAND | 1 | 'SELECT' | 1
CONSTANT| 8 | '1' | 1
DELIMITER | 9 | ';' | 1
SQL_COMMAND | 11 | 'UPDATE' | 2
IDENTIFIER | 18 | 'test' | 2
SQL_KEYWORD | 23 | 'SET'| 2
IDENTIFIER | 27 | '"a"'| 2
OPERATOR| 30 | '=' | 2
CONSTANT| 31 | '1' | 2
>
> Cheers,
> David.
As a phpPgAdmin dev, I am thinking about this subject since a long time.
I am interested about trying to create such a patch after discussing it
and if you think it is doable.
- --
JGuillaume (ioguix) de Rorthais
http://www.dalibo.com
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAkvPOJMACgkQxWGfaAgowiLrUACfa7qMVr3oiOVS7JfhTa1S9EqY
pYkAn3Sj6cezC/EdWPu2+kzrgjaDygGE
=oY1c
-END PGP SIGNATURE-
--
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] [DOCS] Streaming replication document improvements
On Wed, Apr 21, 2010 at 12:20 PM, Tom Lane wrote: > Robert Haas writes: >> Thanks for the heads up. It doesn't look hard to put a similar test >> in the walsender code path, but is there any reason to duplicate the >> code? Seems like we might be able to just put this test (with the >> necessary modification) right before this comment: > > Hm, actually I think you're right: we could move both of those > connection-rejecting tests up to before the walsender exit. The I am guessing that by "both of those connection-rejecting tests", you mean the one that can throw "must be superuser to connect during database shutdown" as well as the one we were discussing, which can throw "connection limit exceeded for non-superusers", in which case... > only extra state we need is ReservedBackends, which should be valid > at that point (in particular, it can't be affected by any process-local > GUC settings). > > +1 for just moving the test. ...shouldn't we move the "tests", plural, rather than just the one? It seems right to reject new SR connections during shutdown. ...Robert -- 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] Move tablespace
On Wed, 2010-04-21 at 11:53 -0400, Tom Lane wrote: > Simon Riggs writes: > > On Wed, 2010-04-21 at 14:37 +0300, Heikki Linnakangas wrote: > >> I also think we shouldn't be fiddling with this at this stage in the > >> release cycle. > > > OK, but not because I see a problem with the technique. > > You made that plain already, but you have not convinced anyone else. Not pouting, just recording the fact that its an ongoing discussion. > More to the point, ALTER SET TABLESPACE is not an operation that > happens so much that we ought to take any risks to optimize it. I think its the opposite: people don't run it because they can't. But I'm happy not to press further at this stage of 9.0. I am always optimistic about convincing you ;-) -- Simon Riggs www.2ndQuadrant.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] [DOCS] Streaming replication document improvements
Robert Haas writes: > Thanks for the heads up. It doesn't look hard to put a similar test > in the walsender code path, but is there any reason to duplicate the > code? Seems like we might be able to just put this test (with the > necessary modification) right before this comment: Hm, actually I think you're right: we could move both of those connection-rejecting tests up to before the walsender exit. The only extra state we need is ReservedBackends, which should be valid at that point (in particular, it can't be affected by any process-local GUC settings). +1 for just moving the test. 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] [DOCS] Streaming replication document improvements
On Tue, Apr 20, 2010 at 7:53 PM, Tom Lane wrote: > Robert Haas writes: >> Current logic says we hit the connection limit if: > >> if (!am_superuser && >> ReservedBackends > 0 && >> !HaveNFreeProcs(ReservedBackends)) > >> Couldn't we just change this to: > >> if ((!am_superuser || am_walsender) && >> ReservedBackends > 0 && >> !HaveNFreeProcs(ReservedBackends)) > > As of the patch I just committed, that code is not reached anymore by a > walsender process. However, it shouldn't be hard to put a similar test > into the walsender code path. Thanks for the heads up. It doesn't look hard to put a similar test in the walsender code path, but is there any reason to duplicate the code? Seems like we might be able to just put this test (with the necessary modification) right before this comment: /* * If walsender, we're done here --- we don't want to connect to any * particular database. */ In fact, in some ways, it seems better to put it up there. If the database is really being flooded with connection attempts, we want to ephemerally consume a backend slot for as little time as possible... ...Robert -- 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] GSoC - proposal - Materialized Views in PostgreSQL
2010/4/20 Pavel : > For now I know it is not commitable in actual state, but for my thesis it is > enough and I know it will not be commitable with this design at all. In case > of GSoC it will depends on the time I will be able to spend on it, if I will > consider some other design. I am not sure about this, but I would think we would not want to accept the project unless you intend to try to make it committable. I haven't looked at your actual code to see how much work I think that would take. ...Robert -- 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] Move tablespace
Simon Riggs writes: > On Wed, 2010-04-21 at 14:37 +0300, Heikki Linnakangas wrote: >> I also think we shouldn't be fiddling with this at this stage in the >> release cycle. > OK, but not because I see a problem with the technique. You made that plain already, but you have not convinced anyone else. More to the point, ALTER SET TABLESPACE is not an operation that happens so much that we ought to take any risks to optimize it. 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] testing HS/SR - 1 vs 2 performance
On Apr 21, 2010, at 16:49 , Simon Riggs wrote: > On Wed, 2010-04-21 at 16:22 +0200, marcin mank wrote: > >> Is that not a good idea that (at least for dev-builds, like with >> enable-cassert) the xid counter start at like 2^31 - 1000 ? It could >> help catch some bugs. > > It is a good idea, I'm sure that would help catch bugs. > > It wouldn't help here because the case in doubt is whether it's possible > to have an xid still showing in memory arrays from the last time the > cycle wrapped. It isn't. These things aren't random. These numbers are > extracted directly from activity that was occurring on the primary and > regularly checked and cleaned as the standby runs. > > So you'll need to do 2^31 transactions to prove this isn't true, which > isn't ever going to happen in testing with an assert build and nobody > with that many transactions would run an assert build anyway. ISTM that there's no need to actually execute 2^31 transactions to trigger this bug (if it actually exists), it'd be sufficient to increment the xid counter by more than one each time a xid is assigned, no? Or would that trip snapshot creation on the standby? best regards, Florian Pflug smime.p7s Description: S/MIME cryptographic signature
Re: [HACKERS] testing HS/SR - 1 vs 2 performance
On Wed, Apr 21, 2010 at 10:12 AM, Simon Riggs wrote: > On Wed, 2010-04-21 at 09:51 -0400, Robert Haas wrote: >> > >> > Adding an assertion isn't going to do much because it's unlikely anybody >> > is going to be running for 2^31 transactions with asserts enabled. >> > > >> I think the assert is a good idea. If there's no real problem here, >> the assert won't trip. It's just a safety precaution. > > If you believe that, then I think you should add this to all the other > places in the current server where that assumption is made without > assertion being added. As a safety precaution. I feel like this conversation is getting a little heated. We are just trying to solve a technical problem here. Perhaps I am misreading - tone doesn't come through very well in email. I think the assumptions that are being made in this particular case are different from the ones made elsewhere in the server. Generally, we don't assume transaction IDs are arriving in ascending order - in fact, we usually explicitly have to deal with the fact that they might not be. So if we have a situation where we ARE relying on them arriving in order, because we have extrinsic reasons why we know it has to happen that way, adding an assertion to make sure that things are happening the way we expect doesn't seem out of line. This code is fairly complex. There is arguably less value in asserting that the newly added xid follows the tail as well as the head, but I still like the idea. Not sure whether that's rational or not. ...Robert -- 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] Thoughts on pg_hba.conf rejection
Robert Haas writes: > On Tue, Apr 20, 2010 at 7:13 PM, Tom Lane wrote: >> (You might want to look back at the archived discussions about how to >> avoid storing entries for temp tables in these catalogs; that poses >> many of the same issues.) > Do you happen to know what a good search term might be? I tried > searching for things like "pg_class temp tables" and "pg_class > temporary tables" and didn't come up with much. I found this thread: http://archives.postgresql.org/pgsql-hackers/2008-07/msg00593.php I claimed in that message that there were previous discussions but I did not come across them right away. 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] testing HS/SR - 1 vs 2 performance
On Wed, 2010-04-21 at 16:22 +0200, marcin mank wrote: > Is that not a good idea that (at least for dev-builds, like with > enable-cassert) the xid counter start at like 2^31 - 1000 ? It could > help catch some bugs. It is a good idea, I'm sure that would help catch bugs. It wouldn't help here because the case in doubt is whether it's possible to have an xid still showing in memory arrays from the last time the cycle wrapped. It isn't. These things aren't random. These numbers are extracted directly from activity that was occurring on the primary and regularly checked and cleaned as the standby runs. So you'll need to do 2^31 transactions to prove this isn't true, which isn't ever going to happen in testing with an assert build and nobody with that many transactions would run an assert build anyway. -- Simon Riggs www.2ndQuadrant.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] testing HS/SR - 1 vs 2 performance
On Wed, Apr 21, 2010 at 4:12 PM, Simon Riggs wrote: > On Wed, 2010-04-21 at 09:51 -0400, Robert Haas wrote: >> > >> > Adding an assertion isn't going to do much because it's unlikely anybody >> > is going to be running for 2^31 transactions with asserts enabled. >> > > >> I think the assert is a good idea. If there's no real problem here, >> the assert won't trip. It's just a safety precaution. > > If you believe that, then I think you should add this to all the other > places in the current server where that assumption is made without > assertion being added. As a safety precaution. > Is that not a good idea that (at least for dev-builds, like with enable-cassert) the xid counter start at like 2^31 - 1000 ? It could help catch some bugs. Greetings Marcin Mańk -- 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] testing HS/SR - 1 vs 2 performance
On Wed, 2010-04-21 at 09:51 -0400, Robert Haas wrote: > > > > Adding an assertion isn't going to do much because it's unlikely anybody > > is going to be running for 2^31 transactions with asserts enabled. > > > I think the assert is a good idea. If there's no real problem here, > the assert won't trip. It's just a safety precaution. If you believe that, then I think you should add this to all the other places in the current server where that assumption is made without assertion being added. As a safety precaution. -- Simon Riggs www.2ndQuadrant.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] testing HS/SR - 1 vs 2 performance
On Wed, Apr 21, 2010 at 8:20 AM, Heikki Linnakangas wrote: > The locking seems overly complex to me. I tend to agree. ! /* !* Callers must hold either ProcArrayLock in Exclusive mode or !* ProcArrayLock in Shared mode *and* known_assigned_xids_lck !* to update these values. !*/ I'm not convinced that this is either (a) correct or (b) performant. ...Robert -- 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] testing HS/SR - 1 vs 2 performance
On Wed, Apr 21, 2010 at 9:37 AM, Simon Riggs wrote: > On Wed, 2010-04-21 at 15:27 +0300, Heikki Linnakangas wrote: > >> Given the discussion about the cyclic nature of XIDs, it would be good >> to add an assertion that when a new XID is added to the array, it is >> >> a) larger than the biggest value already in the array >> (TransactionIdFollows(new, head)), and >> b) not too far from the smallest value in the array to confuse binary >> search (TransactionIdFollows(new, tail)). > > We discussed this in November. You convinced me it isn't possible for > older xids to stay in the standby because anti-wraparound vacuums would > conflict and kick them out. The primary can't run with wrapped xids and > neither can the standby. I think that is correct. > > Adding an assertion isn't going to do much because it's unlikely anybody > is going to be running for 2^31 transactions with asserts enabled. > > Worrying about things like this seems strange when real and negative > behaviours are right in our faces elsewhere. Performance and scalability > are real world concerns. I think the assert is a good idea. If there's no real problem here, the assert won't trip. It's just a safety precaution. ...Robert -- 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] BETA
On Wed, 21 Apr 2010, Robert Haas wrote: Well, never mind that then. How about a beta next week? I'm good for that ... Marc G. FournierHub.Org Hosting Solutions S.A. scra...@hub.org http://www.hub.org Yahoo:yscrappySkype: hub.orgICQ:7615664MSN:scra...@hub.org -- 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] testing HS/SR - 1 vs 2 performance
On Wed, 2010-04-21 at 15:27 +0300, Heikki Linnakangas wrote: > Given the discussion about the cyclic nature of XIDs, it would be good > to add an assertion that when a new XID is added to the array, it is > > a) larger than the biggest value already in the array > (TransactionIdFollows(new, head)), and > b) not too far from the smallest value in the array to confuse binary > search (TransactionIdFollows(new, tail)). We discussed this in November. You convinced me it isn't possible for older xids to stay in the standby because anti-wraparound vacuums would conflict and kick them out. The primary can't run with wrapped xids and neither can the standby. I think that is correct. Adding an assertion isn't going to do much because it's unlikely anybody is going to be running for 2^31 transactions with asserts enabled. Worrying about things like this seems strange when real and negative behaviours are right in our faces elsewhere. Performance and scalability are real world concerns. -- Simon Riggs www.2ndQuadrant.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] testing HS/SR - 1 vs 2 performance
On Wed, 2010-04-21 at 15:20 +0300, Heikki Linnakangas wrote: > Simon Riggs wrote: > > On Sun, 2010-04-18 at 08:24 +0100, Simon Riggs wrote: > >> On Sat, 2010-04-17 at 18:52 -0400, Tom Lane wrote: > >>> Simon Riggs writes: > What I'm not clear on is why you've used a spinlock everywhere when only > weak-memory thang CPUs are a problem. Why not have a weak-memory-protect > macro that does does nada when the hardware already protects us? (i.e. a > spinlock only for the hardware that needs it). > >>> Well, we could certainly consider that, if we had enough places where > >>> there was a demonstrable benefit from it. I couldn't measure any real > >>> slowdown from adding a spinlock in that sinval code, so I didn't propose > >>> doing so at the time --- and I'm pretty dubious that this code is > >>> sufficiently performance-critical to justify the work, either. > >> OK, I'll put a spinlock around access to the head of the array. > > > > v2 patch attached > > The locking seems overly complex to me. > Looking at > KnownAssignedXidsAdd(), isn't it possible for two processes to call it > concurrently in exclusive_lock==false mode and get the same 'head' > value, and step on each others toes? I guess KnownAssignedXidsAdd() is > only called by the startup process, but it seems like an accident > waiting to happen. Not at all. That assumption is also used elsewhere, so it is safe to use here. > Spinlocks are fast, if you have to add an if-branch to decide whether to > acquire it, I suspect you've lost any performance gain to be had > already. I think you're misreading the code. One caller already holds exclusive lock, one does not. The if test is to determine whether to acquire the lock or not. > Let's keep it simple. And acquiring ProcArrayLock in exclusive > mode while adding entries to the array seems OK to me as well. It only > needs to be held very briefly, and getting this correct and keeping it > simple is much more important at this point than squeezing out every > possible CPU cycle from the system. I don't understand what you're saying: you say I'm wasting a few cycles on an if test and should change that, but at the same time you say I shouldn't worry about a few cycles. > Just require that the caller holds ProcArrayLock in exclusive mode when > calling an operation that modifies the array, and in shared mode when > reading it. The point of the code you're discussing is to remove the exclusive lock requests, not to save a few cycles. Spinlocks are fast, as you say. Exclusive lock requests block under a heavy load of shared lock holders, we know that already. It is worth removing the contention that can occur by minimising the number of exclusive locks required. This patch shows how and I don't see any reason from what you have said to avoid committing it. I'm willing to hear some sound reasons, if any exist. -- Simon Riggs www.2ndQuadrant.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] testing HS/SR - 1 vs 2 performance
Simon Riggs wrote: > On Sun, 2010-04-18 at 08:24 +0100, Simon Riggs wrote: >> On Sat, 2010-04-17 at 18:52 -0400, Tom Lane wrote: >>> Simon Riggs writes: What I'm not clear on is why you've used a spinlock everywhere when only weak-memory thang CPUs are a problem. Why not have a weak-memory-protect macro that does does nada when the hardware already protects us? (i.e. a spinlock only for the hardware that needs it). >>> Well, we could certainly consider that, if we had enough places where >>> there was a demonstrable benefit from it. I couldn't measure any real >>> slowdown from adding a spinlock in that sinval code, so I didn't propose >>> doing so at the time --- and I'm pretty dubious that this code is >>> sufficiently performance-critical to justify the work, either. >> OK, I'll put a spinlock around access to the head of the array. > > v2 patch attached Given the discussion about the cyclic nature of XIDs, it would be good to add an assertion that when a new XID is added to the array, it is a) larger than the biggest value already in the array (TransactionIdFollows(new, head)), and b) not too far from the smallest value in the array to confuse binary search (TransactionIdFollows(new, tail)). -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.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] testing HS/SR - 1 vs 2 performance
Simon Riggs wrote: > On Sun, 2010-04-18 at 08:24 +0100, Simon Riggs wrote: >> On Sat, 2010-04-17 at 18:52 -0400, Tom Lane wrote: >>> Simon Riggs writes: What I'm not clear on is why you've used a spinlock everywhere when only weak-memory thang CPUs are a problem. Why not have a weak-memory-protect macro that does does nada when the hardware already protects us? (i.e. a spinlock only for the hardware that needs it). >>> Well, we could certainly consider that, if we had enough places where >>> there was a demonstrable benefit from it. I couldn't measure any real >>> slowdown from adding a spinlock in that sinval code, so I didn't propose >>> doing so at the time --- and I'm pretty dubious that this code is >>> sufficiently performance-critical to justify the work, either. >> OK, I'll put a spinlock around access to the head of the array. > > v2 patch attached The locking seems overly complex to me. Looking at KnownAssignedXidsAdd(), isn't it possible for two processes to call it concurrently in exclusive_lock==false mode and get the same 'head' value, and step on each others toes? I guess KnownAssignedXidsAdd() is only called by the startup process, but it seems like an accident waiting to happen. Spinlocks are fast, if you have to add an if-branch to decide whether to acquire it, I suspect you've lost any performance gain to be had already. Let's keep it simple. And acquiring ProcArrayLock in exclusive mode while adding entries to the array seems OK to me as well. It only needs to be held very briefly, and getting this correct and keeping it simple is much more important at this point than squeezing out every possible CPU cycle from the system. Just require that the caller holds ProcArrayLock in exclusive mode when calling an operation that modifies the array, and in shared mode when reading it. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.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] BETA
On Tue, Apr 20, 2010 at 1:53 PM, Marc G. Fournier wrote: > On Tue, 20 Apr 2010, Robert Haas wrote: > >> /me pushes luck >> >> And how about a set of back-branch releases while we're at it? > > We tend to try and avoid overlapping a "release" with a "beta" to avoid > confusion ... but didn't we just do a fresh back branch release anyway? Eh, so we did. How did I miss that? 8.4.3 was released 2010-03-05. I was thinking 8.4.2 was still current. Well, never mind that then. How about a beta next week? ...Robert -- 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] Move tablespace
On Wed, 2010-04-21 at 14:37 +0300, Heikki Linnakangas wrote: > Simon Riggs wrote: > > On Tue, 2010-04-20 at 21:03 -0400, Tom Lane wrote: > >> Simon Riggs writes: > >>> Following patch writes a new WAL record that just says "copy foo to > >>> newts" and during replay we flush buffers and then re-execute the copy > >>> (but only when InArchiveRecovery). So the copy happens locally on the > >>> standby, not copying from primary to standby. We do this just with a > >>> little refactoring and a simple new WAL message. > >> And what happens to crash-recovery replay? You can't have it both ways, > >> either the data is in WAL or it's missing. > > > > The patch changes nothing in the case of crash recovery. > > What happens if the record is replayed twice in archive recovery? For > example if you stop and restart a standby server after it has replayed > that record. What does the 2nd redo attempt do if the source file was > already deleted by the 1st recovery. If the source is absent then we know that replay had successfully copied the file and synced it, so we can just skip the record. > I also think we shouldn't be fiddling with this at this stage in the > release cycle. OK, but not because I see a problem with the technique. -- Simon Riggs www.2ndQuadrant.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] Move tablespace
Simon Riggs wrote: > On Tue, 2010-04-20 at 21:03 -0400, Tom Lane wrote: >> Simon Riggs writes: >>> Following patch writes a new WAL record that just says "copy foo to >>> newts" and during replay we flush buffers and then re-execute the copy >>> (but only when InArchiveRecovery). So the copy happens locally on the >>> standby, not copying from primary to standby. We do this just with a >>> little refactoring and a simple new WAL message. >> And what happens to crash-recovery replay? You can't have it both ways, >> either the data is in WAL or it's missing. > > The patch changes nothing in the case of crash recovery. What happens if the record is replayed twice in archive recovery? For example if you stop and restart a standby server after it has replayed that record. What does the 2nd redo attempt do if the source file was already deleted by the 1st recovery. I also think we shouldn't be fiddling with this at this stage in the release cycle. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
