Re: [HACKERS] Sync Rep v17
On Sat, Feb 19, 2011 at 9:06 AM, Simon Riggs si...@2ndquadrant.com wrote: Well, good news all round. v17 implements what I believe to be the final set of features for sync rep. This one I'm actually fairly happy with. It can be enjoyed best at DEBUG3. The patch is very lite touch on a few areas of code, plus a chunk of specific code, all on master-side. Pretty straight really. I'm sure problems will be found, its not long since I completed this; thanks to Daniel Farina for your help with patch assembly. Which is just as well, because the other news is that I'm off on holiday for a few days, which is most inconvenient. I won't be committing this for at least a week and absent from the list. OTOH, I think its ready for a final review and commit, so I'm OK if you commit or OK if you leave it for me. Thanks for the patch! Here are the comments: PREPARE TRANSACTION and ROLLBACK PREPARED should wait for replication as well as COMMIT PREPARED? What if fast shutdown is requested while RecordTransactionCommit is waiting in SyncRepWaitForLSN? ISTM fast shutdown cannot complete until replication has been successfully done (i.e., until at least one synchronous standby has connected to the master especially if allow_standalone_primary is disabled). Is this OK? We should emit WARNING when the synchronous standby with wal_receiver_status_interval = 0 connects to the master. Because, in that case, a transaction unexpectedly would wait for replication infinitely. + /* Need a modifiable copy of string */ + rawstring = pstrdup(SyncRepStandbyNames); + + /* Parse string into list of identifiers */ + if (!SplitIdentifierString(rawstring, ',', elemlist)) pfree(rawstring) and list_free(elemlist) should be called also if SplitIdentifierString returns TRUE. Otherwise, memory-leak would happen. + ereport(FATAL, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg(invalid list syntax for parameter \synchronous_standby_names\))); + return false; return false is not required here though that might be harmless. I've read about a tenth of the patch, so I'll submit another comments about the rest later. 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] Sync Rep v17
Well, good news all round. v17 implements what I believe to be the final set of features for sync rep. This one I'm actually fairly happy with. It can be enjoyed best at DEBUG3. The patch is very lite touch on a few areas of code, plus a chunk of specific code, all on master-side. Pretty straight really. I'm sure problems will be found, its not long since I completed this; thanks to Daniel Farina for your help with patch assembly. + primaryvarnamesynchronous_standby_names/ configuration parameter/primary + /indexterm + listitem + para +Specifies a list of standby names that can become the sole +synchronous standby. Other standby servers connect that are also on +the list become potential standbys. If the current synchronous standby +goes away it will be replaced with one of the potential standbys. +Specifying more than one standby name can allow very high availability. + /para Can anybody please enlighten me? I do not quite follow Other standby servers connect that are also on the list become potential standbys part. Can I read this as Other standby servers that are also on the list become potential synchrnous standbys? -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese: http://www.sraoss.co.jp -- 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] PostgreSQL FDW update
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: I needed something to test the FDW API patch with, and didn't want to get involved in the COPY API changes, and also wanted to have something that needs real connection management and can push down quals. So I updated the postgresql_fdw patch to work with the latest FDW patch. Here. It's a bit of a mess, but it works for simple queries.. I'm going to set this CF item back to Waiting on Author, because (a) there doesn't seem to be an actual concrete patch presented at the moment, only a stack of hard-to-follow deltas, and (b) the patch certainly needs updates for the extension mechanism and committed FDW API anyway. It requires a small change to the FDW api (fdw-api-add-serverid-userid.patch). I added server oid and user oid fields to the FdwPlan - that seems like basic information that most FDW's will need, so it seems awkward to require the FDW to wrap them in Const nodes and a List. FYI, I removed those fields from the committed API patch. I don't think it's a good idea to cache potentially-changeable things in a plan node when you can perfectly well fetch them at runtime. Caching user OID is flat wrong anyway, as there's no guarantee that a cached plan will be executed under the same userid it was made with. 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] Snapshot synchronization, again...
A couple more questions: What's the reason for this restriction? if (databaseId != MyDatabaseId) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg(cannot import snapshot from a different database))); Why are we using bytea as the output format instead of text? The output is just plain text anyway, as can be seen by setting bytea_output to 'escape'. Perhaps there would be a value to using bytea if we were to pglz_compress the result, but would there be a point in doing so? Normally exported info should be short enough that it would cause more overhead than it would save anyway. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] FDW API: don't like the EXPLAIN mechanism
On 02/19/2011 11:07 PM, Tom Lane wrote: However, it occurs to me that as long as we're passing the function the ExplainState, it has what it needs to add arbitrary EXPLAIN result fields. Although it could do this the hard way, we could make it a lot easier by exporting the ExplainPropertyType functions. Then it'd be possible to produce output like Foreign Scan on public.agg_csv Foreign File: @abs_srcdir@/data/agg.csv Foreign File Size: 46 which seems a whole lot nicer than the current design. In this scheme the callback function would just be declared to return void. Anybody have an objection to doing it like that? If we allow the invention of new explain states we'll never be able to publish an authoritative schema definition of the data. That's not necessarily an argument against doing it, just something to be aware of. Maybe we don't care about having EXPLAIN XML output validated. cheers andrew -- 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] SQL/MED - file_fdw
Is this right? postgres=# \d+ agg_text Foreign table public.agg_text Column | Type | Modifiers | Storage | Description +--+---+--+- a | smallint | | plain| b | text | | extended | Server: file_server Has OIDs: no It says the agg_text foreign table is using extended storage for the text field. If it's in-file, how can it be classified as potentially TOASTed? -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 -- 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] FDW API: don't like the EXPLAIN mechanism
Andrew Dunstan and...@dunslane.net writes: On 02/19/2011 11:07 PM, Tom Lane wrote: However, it occurs to me that as long as we're passing the function the ExplainState, it has what it needs to add arbitrary EXPLAIN result fields. If we allow the invention of new explain states we'll never be able to publish an authoritative schema definition of the data. That's not necessarily an argument against doing it, just something to be aware of. Maybe we don't care about having EXPLAIN XML output validated. I thought one of the principal arguments for outputting XML/etc formats was exactly that we'd be able to add fields without breaking readers. If that's not the case, why did we bother? 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] FDW API: don't like the EXPLAIN mechanism
On 02/21/2011 11:23 AM, Tom Lane wrote: Andrew Dunstanand...@dunslane.net writes: On 02/19/2011 11:07 PM, Tom Lane wrote: However, it occurs to me that as long as we're passing the function the ExplainState, it has what it needs to add arbitrary EXPLAIN result fields. If we allow the invention of new explain states we'll never be able to publish an authoritative schema definition of the data. That's not necessarily an argument against doing it, just something to be aware of. Maybe we don't care about having EXPLAIN XML output validated. I thought one of the principal arguments for outputting XML/etc formats was exactly that we'd be able to add fields without breaking readers. If that's not the case, why did we bother? Well, I thought the motivation was to allow easy construction of parsers for the data, since creating a parser for those formats is pretty trivial. Anyway, if we don't care about validation that's fine. I just didn't want us to make that decision unconsciously. cheers andrew -- 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] FDW API: don't like the EXPLAIN mechanism
On 02/21/2011 11:38 AM, Andrew Dunstan wrote: On 02/21/2011 11:23 AM, Tom Lane wrote: Andrew Dunstanand...@dunslane.net writes: If we allow the invention of new explain states we'll never be able to publish an authoritative schema definition of the data. That's not necessarily an argument against doing it, just something to be aware of. Maybe we don't care about having EXPLAIN XML output validated. I thought one of the principal arguments for outputting XML/etc formats was exactly that we'd be able to add fields without breaking readers. If that's not the case, why did we bother? Well, I thought the motivation was to allow easy construction of parsers for the data, since creating a parser for those formats is pretty trivial. Anyway, if we don't care about validation that's fine. I just didn't want us to make that decision unconsciously. Parsing XML isn't trivial, not if done correctly... :-) I don't see the benefit of validation beyond test suites, and then the specification can be published with the version of PostgreSQL (as XSD?) if so necessary. Primary benefits include: 1) Open and widely recognized format. 2) Well tested and readily available parsers already exist. 3) Able to easily add content without breaking existing parsers or analyzers, provided the parsers and analyzers are written properly. Any XML parser that does: m[tag(.*?)/tag] ... is not written properly. -- Mark Mielkem...@mielke.cc -- 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] FDW API: don't like the EXPLAIN mechanism
Alvaro Herrera alvhe...@commandprompt.com writes: Excerpts from Andrew Dunstan's message of lun feb 21 13:11:25 -0300 2011: If we allow the invention of new explain states we'll never be able to publish an authoritative schema definition of the data. That's not necessarily an argument against doing it, just something to be aware of. Maybe we don't care about having EXPLAIN XML output validated. The alternative seems to be to let this information proliferate in free form (say as text inside some other node), which may be even less convenient. Yeah; that's exactly what the patch-as-submitted did, and that way was surely not better from the standpoint of machine parsing the output. If you really want to lock down the output format to only what is specified by the core code, then the answer is to forget about giving FDWs an explain hook at all. 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] FDW API: don't like the EXPLAIN mechanism
On 02/21/2011 11:45 AM, Mark Mielke wrote: On 02/21/2011 11:38 AM, Andrew Dunstan wrote: On 02/21/2011 11:23 AM, Tom Lane wrote: Andrew Dunstanand...@dunslane.net writes: If we allow the invention of new explain states we'll never be able to publish an authoritative schema definition of the data. That's not necessarily an argument against doing it, just something to be aware of. Maybe we don't care about having EXPLAIN XML output validated. I thought one of the principal arguments for outputting XML/etc formats was exactly that we'd be able to add fields without breaking readers. If that's not the case, why did we bother? Well, I thought the motivation was to allow easy construction of parsers for the data, since creating a parser for those formats is pretty trivial. Anyway, if we don't care about validation that's fine. I just didn't want us to make that decision unconsciously. Parsing XML isn't trivial, not if done correctly... :-) I don't see the benefit of validation beyond test suites, and then the specification can be published with the version of PostgreSQL (as XSD?) if so necessary. Primary benefits include: 1) Open and widely recognized format. 2) Well tested and readily available parsers already exist. 3) Able to easily add content without breaking existing parsers or analyzers, provided the parsers and analyzers are written properly. Any XML parser that does: m[tag(.*?)/tag] ... is not written properly. Parsing all these formats is trivial enough if you use a library to do the heavy lifting for you. I do *lots* of XML/XSL work and I don't use stuff like m[tag(.*?)/tag]. Here's a fragment from a working program written a week or two back in perl: my $parser= XML::DOM::Parser-new(); my $xp = $parser-parsefile($xmlfile); my ($provider) = $xp-findvalue(//SERVICE_PROVIDER_CODE); my ($invoice_num) = $xp-findvalue(//invoice_num); Not that hard, is it? No regex matching there. :-) Regarding your other suggestion, the whole point I have been making is that if external modules can invent arbitrary nodes then we can't publish an XSD (or RelaxNG or DTD) spec that is worth anything at all. Anyway, it seems like that's what people want. cheers andrew -- 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] FDW API: don't like the EXPLAIN mechanism
Andrew Dunstan and...@dunslane.net writes: Regarding your other suggestion, the whole point I have been making is that if external modules can invent arbitrary nodes then we can't publish an XSD (or RelaxNG or DTD) spec that is worth anything at all. Well, sure we can. But if you're using any external FDW, you'll need to consult its documentation to see what additions it makes. It may be sufficient to say something like ForeignScan can have unspecified additional children. Dunno if we can formalize that in any useful way ... 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] FDW API: don't like the EXPLAIN mechanism
On 02/21/2011 12:28 PM, Tom Lane wrote: Andrew Dunstanand...@dunslane.net writes: Regarding your other suggestion, the whole point I have been making is that if external modules can invent arbitrary nodes then we can't publish an XSD (or RelaxNG or DTD) spec that is worth anything at all. Well, sure we can. But if you're using any external FDW, you'll need to consult its documentation to see what additions it makes. It may be sufficient to say something like ForeignScan can have unspecified additional children. Dunno if we can formalize that in any useful way ... Well, you can override definitions, so the FDW could provide a spec that imported the base spec and then overrode the relevant parts to plug in its extra nodes. But that would get pretty hairy with more than one FDW. Still, we've got by so far with no spec at all so maybe it really doesn't matter. cheers andrew -- 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] Snapshot synchronization, again...
Excerpts from Joachim Wieland's message of dom ene 30 14:36:12 -0300 2011: On Thu, Jan 20, 2011 at 1:37 AM, Noah Misch n...@leadboat.com wrote: Is it valid to scribble directly on snapshots like this? I figured that previously executed code still has references pointing to the snapshots and so we cannot just push a new snapshot on top but really need to change the memory where they are pointing to. Okay. Had no special reason to believe otherwise, just didn't know. This is one part where I'd like someone more experienced than me look into it. Yeah, I don't like this bit very much. I'm inclined to have the import routine fill CurrentSnapshot directly (and perhaps ActiveSnapshot too, not sure about this part yet) instead; I think we need a safety net so that the new serializable isolation code doesn't get upset if we change the base snapshot from under it, but I haven't looked at that yet. Trying to import a snap into a transaction that has committed children should also fail; otherwise, objects touched during those subxacts would be in a spooky zone. Also, I want to have Importing into a read-committed transaction fail with an error instead of the current warning -- just like LOCK TABLE fails if you're not inside a transaction. I'm also inclined to have PREPARE TRANSACTION fail if it has an exported snapshot, instead of just invalidating it. This lets us get rid of the extra locking added during that operation. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] FDW API: don't like the EXPLAIN mechanism
On Feb 21, 2011, at 9:12 AM, Andrew Dunstan wrote: my $parser= XML::DOM::Parser-new(); my $xp = $parser-parsefile($xmlfile); my ($provider) = $xp-findvalue(//SERVICE_PROVIDER_CODE); my ($invoice_num) = $xp-findvalue(//invoice_num); Not that hard, is it? No regex matching there. :-) See also https://github.com/theory/explain-table Best, David -- 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] FDW API: don't like the EXPLAIN mechanism
On Feb 21, 2011, at 10:07 AM, David E. Wheeler wrote: See also https://github.com/theory/explain-table Oops, sorry, make that https://github.com/pgexperts/explain-table Best, David -- 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] Snapshot synchronization, again...
Hi, On Mon, Feb 21, 2011 at 4:56 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: What's the reason for this restriction? if (databaseId != MyDatabaseId) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg(cannot import snapshot from a different database))); I just couldn't think of a use case for it and so didn't want to introduce a feature that we might have to support in the future then. Why are we using bytea as the output format instead of text? The output is just plain text anyway, as can be seen by setting bytea_output to 'escape'. Perhaps there would be a value to using bytea if we were to pglz_compress the result, but would there be a point in doing so? Normally exported info should be short enough that it would cause more overhead than it would save anyway. It is bytea because it should be thought of just some data. It should be regarded more as a token than as text and should not be inspected/interpreted at all. If anybody decides to do so, fine, but then they should not rely on the stability of the message format for the future. Joachim -- 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] FDW API: don't like the EXPLAIN mechanism
On Feb 21, 2011, at 10:11 AM, David E. Wheeler wrote: Oops, sorry, make that https://github.com/pgexperts/explain-table And now I've renamed it (sorry) and released it on PGXN. New links: https://github.com/pgexperts/explanation http://master.pgxn.org/dist/explanation/ Best, David -- 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] Snapshot synchronization, again...
Joachim Wieland j...@mcknight.de writes: On Mon, Feb 21, 2011 at 4:56 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Why are we using bytea as the output format instead of text? It is bytea because it should be thought of just some data. It should be regarded more as a token than as text and should not be inspected/interpreted at all. If anybody decides to do so, fine, but then they should not rely on the stability of the message format for the future. I don't think that passing it as bytea conveys those semantics at all. It just makes it harder to look at the value for debugging purposes. Plus you gotta worry about variable formatting/escaping conventions (bytea_output etc) that could easily be avoided if the value were promised to be text with no funny characters in 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] Snapshot synchronization, again...
Alvaro Herrera wrote: I think we need a safety net so that the new serializable isolation code doesn't get upset if we change the base snapshot from under it, but I haven't looked at that yet. Replacing the snapshot for a serializable transaction after it has acquired its initial snapshot would quietly allow non-serializable behavior, I would think. I don't think that setting a snapshot for a SERIALIZABLE READ ONLY DEFERRABLE transaction makes any sense, since the point of that is that it waits for a snapshot which meets certain criteria to be available; setting a snapshot in that mode should probably just be disallowed. Otherwise, if you set the snapshot before the transaction acquires one through normal means, I can't think of any problems -- just make sure you set FirstSnapshotSet. -Kevin -- 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] Sync Rep v17
On Mon, Feb 21, 2011 at 4:35 AM, Tatsuo Ishii is...@postgresql.org wrote: Well, good news all round. Hello on this thread, I'm taking a look at replication timeout with non-blocking which would be nice but not required for this patch, in my understanding. But before that, we're going to put this patch through some exercise via pgbench to determine two things: * How much performance is impacted * Does it crash? As it will be somewhat hard to prove the durability guarantees of commit without special heroics, unless someone can suggest a mechanism. Expect some results Real Soon. -- fdr -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] validating foreign tables
The API for FDW validators doesn't appear to have any way that the validator function can check that the defined foreign table shape matches the FDW options sanely. Maybe it's a chicken and egg problem, but there seems to be something missing, unless I'm mistaken. We'll have the info when we come to make plan estimates, but that seems like the wrong place to be doing this sort of validation. Can we extend the validator API somehow to make this possible? cheers andrew -- 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] Snapshot synchronization, again...
Excerpts from Kevin Grittner's message of lun feb 21 18:39:26 -0300 2011: Alvaro Herrera wrote: I think we need a safety net so that the new serializable isolation code doesn't get upset if we change the base snapshot from under it, but I haven't looked at that yet. Replacing the snapshot for a serializable transaction after it has acquired its initial snapshot would quietly allow non-serializable behavior, I would think. I don't think that setting a snapshot for a SERIALIZABLE READ ONLY DEFERRABLE transaction makes any sense, since the point of that is that it waits for a snapshot which meets certain criteria to be available; setting a snapshot in that mode should probably just be disallowed. Otherwise, if you set the snapshot before the transaction acquires one through normal means, I can't think of any problems -- just make sure you set FirstSnapshotSet. Actually this seems rather difficult to do, because in order to invoke the function that imports the snapshot, you have to call SELECT, which acquires a snapshot beforehand. So when we actually import the passed-in snapshot, there's already a snapshot set. This is odious but I don't see a way around that -- other than adding special grammar support which seems overkill. I've been thinking in requiring that snapshot to have refcount==1, but I'm not sure if that works. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] validating foreign tables
Andrew Dunstan and...@dunslane.net writes: The API for FDW validators doesn't appear to have any way that the validator function can check that the defined foreign table shape matches the FDW options sanely. Huh? The options ought to be orthogonal to the table column info. If they're not, maybe you need to rethink your option definitions. 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] Snapshot synchronization, again...
Alvaro Herrera alvhe...@commandprompt.com writes: Actually this seems rather difficult to do, because in order to invoke the function that imports the snapshot, you have to call SELECT, which acquires a snapshot beforehand. So when we actually import the passed-in snapshot, there's already a snapshot set. This is odious but I don't see a way around that -- other than adding special grammar support which seems overkill. No, I don't think it is. The alternative is semantics that are at least exceedingly ugly, and very possibly flat-out broken. To take just one point, you have no way at all of preventing the transaction from having done something else using its original snapshot. 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] Snapshot synchronization, again...
Excerpts from Tom Lane's message of lun feb 21 21:00:19 -0300 2011: Alvaro Herrera alvhe...@commandprompt.com writes: Actually this seems rather difficult to do, because in order to invoke the function that imports the snapshot, you have to call SELECT, which acquires a snapshot beforehand. So when we actually import the passed-in snapshot, there's already a snapshot set. This is odious but I don't see a way around that -- other than adding special grammar support which seems overkill. No, I don't think it is. The alternative is semantics that are at least exceedingly ugly, and very possibly flat-out broken. To take just one point, you have no way at all of preventing the transaction from having done something else using its original snapshot. That's true too. So let's discuss the syntax. Maybe START TRANSACTION SNAPSHOT '\xdeadbeef'; This kind of extension seems ugly though; maybe we should consider START TRANSACTION (snapshot='\xdeadbeef'); (like VACUUM, EXPLAIN and COPY) or some such? I don't think we need another top-level command for this. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] Snapshot synchronization, again...
Alvaro Herrera alvhe...@commandprompt.com writes: That's true too. So let's discuss the syntax. Maybe START TRANSACTION SNAPSHOT '\xdeadbeef'; This kind of extension seems ugly though; maybe we should consider START TRANSACTION (snapshot='\xdeadbeef'); (like VACUUM, EXPLAIN and COPY) or some such? +1 for the latter, mainly because (I think) you could avoid making SNAPSHOT into an actual parser keyword that way. I don't think we need another top-level command for this. Mmm ... one possible objection is that if it's hard-wired into the START TRANSACTION syntax, then there is no way to take any locks before establishing the snapshot. Right offhand, though, I can't see a use-case for doing that, given that the snapshot itself would be older than any locks that the secondary transaction could grab. I suppose that a separate top-level command might be nicer just to avoid touching the syntax of START TRANSACTION. If we put the option into parens as above, there seems little chance of colliding with future spec extensions, but it's a tad ugly looking. 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] validating foreign tables
On 02/21/2011 06:56 PM, Tom Lane wrote: Andrew Dunstanand...@dunslane.net writes: The API for FDW validators doesn't appear to have any way that the validator function can check that the defined foreign table shape matches the FDW options sanely. Huh? The options ought to be orthogonal to the table column info. If they're not, maybe you need to rethink your option definitions. Well, let's take a couple of cases. 1. My old favorite, file as a text array. 2. A hypothetical RSS feed, where the options specify which RSS fields we want. Of course, we could just let these break or give odd results when we run a SELECT if the foreign table doesn't match what's expected. file_fdw will presumably break if the input file has rows with the wrong number of columns, just as COPY does. But there will be cases, like the two above, where a sanity check on the table shape could usefully be done at validation time as opposed to run time, and it would be nice to be able to do such a check. cheers andrew -- 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] validating foreign tables
On Tue, Feb 22, 2011 at 10:12, Andrew Dunstan and...@dunslane.net wrote: The API for FDW validators doesn't appear to have any way that the validator function can check that the defined foreign table shape matches the FDW options sanely. Huh? The options ought to be orthogonal to the table column info. If they're not, maybe you need to rethink your option definitions. Well, let's take a couple of cases. 1. My old favorite, file as a text array. 2. A hypothetical RSS feed, where the options specify which RSS fields we want. I think we need to overhaul validators in 9.2 listening to FDW developers' opinions anyway. The text array is an example, but there should be many other requirements. Personally, I'd like to have a method to list available options from SQL. We should also consider column-level options for foreign tables then. -- Itagaki Takahiro -- 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] TODO: You can alter it, but you can't view it
Itagaki Takahiro wrote: On Mon, Sep 27, 2010 at 2:19 PM, Josh Berkus j...@agliodbs.com wrote: While working on some database maintenance, I was just tripped up by the fact that there is no good way to query reloptions for tables. ?By no good way I mean no way which does not involve UNNEST and regexps or procedural code. Can you use pg_options_to_table() for your purpose? =# CREATE TABLE tbl (i integer) with (fillfactor = 70); =# SELECT (pg_options_to_table(reloptions)).* FROM pg_class WHERE oid = 'tbl'::regclass; option_name | option_value -+-- fillfactor | 70 Right now pg_options_to_table() is not documented. Should it be? -- Bruce Momjian br...@momjian.ushttp://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] configure gaps
Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: I propose that we add the following test for this case: AC_CHECK_HEADER(Python.h, [], [AC_MSG_ERROR([header file Python.h is required for Python])]) You'd need to pay attention to python_includespec, but otherwise seems reasonable. Did this get done? If so, I don't see it. -- Bruce Momjian br...@momjian.ushttp://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] configure gaps
On 02/21/2011 09:33 PM, Bruce Momjian wrote: Tom Lane wrote: Andrew Dunstanand...@dunslane.net writes: I propose that we add the following test for this case: AC_CHECK_HEADER(Python.h, [], [AC_MSG_ERROR([header filePython.h is required for Python])]) You'd need to pay attention to python_includespec, but otherwise seems reasonable. Did this get done? If so, I don't see it. Oh, no. It skipped my TODO list. I'll try to get it done in the next day or so. cheers andrew -- 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] SSI bug?
On Mon, Feb 21, 2011 at 11:42:36PM +, YAMAMOTO Takashi wrote: i tested ede45e90dd1992bfd3e1e61ce87bad494b81f54d + ssi-multi-update-1.patch with my application and got the following assertion failure. #4 0x0827977e in CheckTargetForConflictsIn (targettag=0xbfbfce78) at predicate.c:3657 3657Assert(locallock != NULL); It looks like CheckTargetForConflictsIn is making the assumption that the backend-local lock table is accurate, which was probably even true at the time it was written. Unfortunately, it hasn't been for a while, and the new changes for tuple versions make it more likely that this will actually come up. The solution is only slightly more complicated than just removing the assertion. Unless Kevin beats me to it, I'll put together a patch later tonight or tomorrow. (I'm at the airport right now.) Dan -- Dan R. K. Ports MIT CSAILhttp://drkp.net/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] UNLOGGED tables in psql \d
psql \d(+) doesn't show any information about UNLOGGED and TEMP attributes for the table. So, we cannot know the table is unlogged or not unless we directly select from pg_class.relpersistence. Is this a TODO item? The same issue is in TEMP tables, but we can determine them by their schema; they are always created in pg_temp_N schema. -- Itagaki Takahiro -- 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] validating foreign tables
On 02/21/2011 08:59 PM, Itagaki Takahiro wrote: I think we need to overhaul validators in 9.2 listening to FDW developers' opinions anyway. The text array is an example, but there should be many other requirements. Personally, I'd like to have a method to list available options from SQL. We should also consider column-level options for foreign tables then. Ok, I guess. It just seems to me like it will be harder to extend the API later than now, so if we can reasonably foresee a likely need we should try to provide for it. cheers andrew -- 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] validating foreign tables
Andrew Dunstan and...@dunslane.net writes: On 02/21/2011 06:56 PM, Tom Lane wrote: Huh? The options ought to be orthogonal to the table column info. If they're not, maybe you need to rethink your option definitions. Well, let's take a couple of cases. 1. My old favorite, file as a text array. 2. A hypothetical RSS feed, where the options specify which RSS fields we want. As above, I claim that an FDW that has such options is badly designed to begin with. Why can't you generate the RSS command on-the-fly from the table rowtype? Of course, we could just let these break or give odd results when we run a SELECT if the foreign table doesn't match what's expected. file_fdw will presumably break if the input file has rows with the wrong number of columns, just as COPY does. But there will be cases, like the two above, where a sanity check on the table shape could usefully be done at validation time as opposed to run time, and it would be nice to be able to do such a check. I can't get excited about this. For one thing, you'd then need to worry about involving the validator in random ALTER TABLE situations, not just when changing the options it's supposed to be checking. 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] validating foreign tables
Andrew Dunstan and...@dunslane.net writes: On 02/21/2011 08:59 PM, Itagaki Takahiro wrote: I think we need to overhaul validators in 9.2 listening to FDW developers' opinions anyway. Ok, I guess. It just seems to me like it will be harder to extend the API later than now, so if we can reasonably foresee a likely need we should try to provide for it. Perhaps we should put a large friendly EXPERIMENTAL, SUBJECT TO CHANGE notice on all the FDW API stuff? Just tell people up front that we're not prepared to promise any API stability yet. There's stuff we *know* is lacking (it's read-only, the optimization support sucks) in addition to whatever we may later realize is misdesigned. 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] Sync Rep v17
On Tue, Feb 22, 2011 at 7:55 AM, Daniel Farina dan...@heroku.com wrote: I'm taking a look at replication timeout with non-blocking which would be nice but not required for this patch, in my understanding. Why do you think so? You think sync_replication_timeout_client is sufficient for sync rep? 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] Sync Rep v17
On Mon, Feb 21, 2011 at 6:06 PM, Fujii Masao masao.fu...@gmail.com wrote:
I've read about a tenth of the patch, so I'll submit another comments
about the rest later.
Here are another comments:
SyncRepReleaseWaiters should be called when walsender exits. Otherwise,
if the standby crashes while a transaction is waiting for replication,
it waits infinitely.
sync_rep_service and potential_sync_standby are not required to be in the
WalSnd shmem because only walsender accesses them.
+static bool
+SyncRepServiceAvailable(void)
+{
+ bool result = false;
+
+ SpinLockAcquire(WalSndCtl-ctlmutex);
+ result = WalSndCtl-sync_rep_service_available;
+ SpinLockRelease(WalSndCtl-ctlmutex);
+
+ return result;
+}
volatile pointer needs to be used to prevent code rearrangement.
+ slock_t ctlmutex; /* locks shared variables shown
above */
cltmutex should be initialized by calling SpinLockInit.
+ /*
+* Stop providing the sync rep service, even if there
are
+* waiting backends.
+*/
+ {
+ SpinLockAcquire(WalSndCtl-ctlmutex);
+ WalSndCtl-sync_rep_service_available = false;
+ SpinLockRelease(WalSndCtl-ctlmutex);
+ }
sync_rep_service_available should be set to false only when
there is no synchronous walsender.
+ /*
+* When we first start replication the standby will be behind the
primary.
+* For some applications, for example, synchronous replication, it is
+* important to have a clear state for this initial catchup mode, so we
+* can trigger actions when we change streaming state later. We may stay
+* in this state for a long time, which is exactly why we want to be
+* able to monitor whether or not we are still here.
+*/
+ WalSndSetState(WALSNDSTATE_CATCHUP);
+
The above has already been committed. Please remove that from the patch.
I don't like calling SyncRepReleaseWaiters for each feedback because
I guess that it's too frequent. How about receiving all the feedbacks available
from the socket, and then calling SyncRepReleaseWaiters as well as
walreceiver does?
+ boolownLatch; /* do we own the above latch? */
We can just remove the ownLatch flag.
I've read about two-tenths of the patch, so I'll submit another comments
about the rest later. Sorry for the slow reviewing...
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] Sync Rep v17
On Mon, Feb 21, 2011 at 9:35 PM, Tatsuo Ishii is...@postgresql.org wrote: + primaryvarnamesynchronous_standby_names/ configuration parameter/primary + /indexterm + listitem + para + Specifies a list of standby names that can become the sole + synchronous standby. Other standby servers connect that are also on + the list become potential standbys. If the current synchronous standby + goes away it will be replaced with one of the potential standbys. + Specifying more than one standby name can allow very high availability. + /para Can anybody please enlighten me? I do not quite follow Other standby servers connect that are also on the list become potential standbys part. Can I read this as Other standby servers that are also on the list become potential synchrnous standbys? I read that in the same way. 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] validating foreign tables
2011/2/22 Tom Lane t...@sss.pgh.pa.us: Andrew Dunstan and...@dunslane.net writes: On 02/21/2011 08:59 PM, Itagaki Takahiro wrote: I think we need to overhaul validators in 9.2 listening to FDW developers' opinions anyway. Ok, I guess. It just seems to me like it will be harder to extend the API later than now, so if we can reasonably foresee a likely need we should try to provide for it. Perhaps we should put a large friendly EXPERIMENTAL, SUBJECT TO CHANGE notice on all the FDW API stuff? Just tell people up front that we're not prepared to promise any API stability yet. There's stuff we *know* is lacking (it's read-only, the optimization support sucks) in addition to whatever we may later realize is misdesigned. regards, tom lane +1 regards Pavel Stehule -- 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] Snapshot synchronization, again...
On 19.02.2011 02:41, Joachim Wieland wrote: On Fri, Feb 18, 2011 at 8:57 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: 1. why are you using the expansible char array stuff instead of using the StringInfo facility? 2. is md5 the most appropriate digest for this? If you need a cryptographically secure hash, do we need something stronger? If not, why not just use hash_any? We don't need a cryptographically secure hash. Really? The idea of the hash is to prevent you from importing arbitrary snapshots into the system, allowing only copying snapshots that are in use by another backend. The purpose of the hash is to make sure the snapshot the client passes in is identical to one used by another backend. If you don't use a cryptographically secure hash, it's easy to construct a snapshot with the same hash as an existing snapshot, with more or less arbitrary contents. This also makes me worried: + + /* +* Verify that the checksum matches before doing any more work. We will +* later verify again to make sure that the exporting transaction has not +* yet terminated by then. We don't want to optimize this into just one +* verification call at the very end because the instructions that follow +* this comment rely on a sane format of the textual snapshot data. In +* particular they assume that there are not more XactIds than +* advertised... +*/ It sounds like you risk a buffer overflow if the snapshot is bogus, which makes a collision-resistant hash even more important. I know this was discussed already, but I don't much like using a hash like this. We should find a way to just store the whole snapshot in shared memory, or even a temporary file if we want to skimp on shared memory. It's generally better to not rely on cryptography when you don't have to. -- 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] Snapshot synchronization, again...
On 21.02.2011 21:33, Joachim Wieland wrote: Hi, On Mon, Feb 21, 2011 at 4:56 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: What's the reason for this restriction? if (databaseId != MyDatabaseId) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg(cannot import snapshot from a different database))); I just couldn't think of a use case for it and so didn't want to introduce a feature that we might have to support in the future then. Hmm, here's one: getting a consistent snapshot of all databases in pg_dumpall. Not sure how useful that is.. -- 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
