Re: [HACKERS] walsender doesn't send keepalives when writes are pending
On 2014-02-22 09:08:39 +0530, Amit Kapila wrote: The danger is rather that *no* keepalive is sent (with requestReply = true triggering a reply by the client) by the walsender. Try to run pg_receivexlog against a busy server with a low walsender timeout. Since we'll never get to sending a keepalive we'll not trigger a reply by the receiver. Now Looking at code of pg_receivexlog in function HandleCopyStream(), it seems that it can only happen if user has not configured --status-interval in pg_receivexlog. Code referred is as below: The interval interval is configured independently from the primary and pg_receivexlog doesn't tune it automatically to the one configured for the walsender. Even if this is not happening due to some reason, shouldn't this be anyway the responsibility of pg_receivexlog to send the status from time to time rather than sending when server asked for it? It does. At it's own interval. I don't see what's to discuss here, sorry. There's really barely any cost to doing the keepalive correctly, otherwise it'd be problematic in the half dozen cases where *we* do send it. The keepalive mechanism doesn't work in one edgecase. So, let's fix it, and not discuss why we think the entire mechanism might be useless. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: show relation and tuple infos of a lock to acquire
On 2014-02-22 11:53:24 +0530, Amit Kapila wrote: On Fri, Feb 21, 2014 at 4:55 PM, Christian Kruse christ...@2ndquadrant.com wrote: Hi, 1. New context added by this patch is display at wrong place [...] Do this patch expect to print the context at cancel request as well? To be honest, I don't see a problem here. If you cancel the request in most cases it is because it doesn't finish in an acceptable time. So displaying the context seems logical to me. Isn't it a matter of consistency, we might not be able to display it on all long running updates, rather only when it goes for update on tuple on which some other transaction is operating. We'll display it when we are waiting for a lock. Which quite possibly is the reason it's cancelled, so logging the locking information is highly pertinent. Is it possible that we set callback to error_context_stack, only when we go for displaying this message. The way to do could be that rather than forming callback in local variable in fuction XactLockTableWaitWithErrorCallback(), store it in global variable and then use that at appropriate place to set error_context_stack. That seems like unneccessary complexity. In general, why I am suggesting to restrict display of newly added context for the case it is added to ensure that it doesn't get started displaying in other cases where it might make sense or might not make sense. Anything failing while inside an XactLockTableWait() et al. will benefit from the context. In which scenarios could that be unneccessary? (according to Andres we would have to look at rel-rd_node.dbNode) and since other commands dealing with names don't do so, either, I think we should leave out the database name. For this case as I have shown that in similar message, we already display database oid, it should not be a problem to display here as well. It will be more information to user. I don't think we do for any where we actually display the relation name, do we? Currently I simply display the whole tuple with truncating long fields. This is plain easy, no distinction necessary. I did some code reading and it seems somewhat complex to get the PK columns and it seems that we need another lock for this, too - maybe not the best idea when we are already in locking trouble, do you disagree? I don't think you need to take another lock for this, it must already have appropriate lock by that time. There should not be any problem in calling relationHasPrimaryKey except that currently it is static, you need to expose it. Other locations that deal with similar things (notably ExecBuildSlotValueDescription()) doesn't either. I don't think this patch should introduce it then. Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Dump pageinspect to 1.2: page_header with pg_lsn datatype
Hi all, Please find attached a patch to dump pageinspect to 1.2. This simply changes page_header to use the new internal datatype pg_lsn instead of text. Regards, -- Michael diff --git a/contrib/pageinspect/Makefile b/contrib/pageinspect/Makefile index 0e267eb..ee78cb2 100644 --- a/contrib/pageinspect/Makefile +++ b/contrib/pageinspect/Makefile @@ -4,8 +4,8 @@ MODULE_big = pageinspect OBJS = rawpage.o heapfuncs.o btreefuncs.o fsmfuncs.o EXTENSION = pageinspect -DATA = pageinspect--1.1.sql pageinspect--1.0--1.1.sql \ - pageinspect--unpackaged--1.0.sql +DATA = pageinspect--1.2.sql pageinspect--1.0--1.1.sql \ + pageinspect--1.1--1.2.sql pageinspect--unpackaged--1.0.sql ifdef USE_PGXS PG_CONFIG = pg_config diff --git a/contrib/pageinspect/pageinspect--1.1--1.2.sql b/contrib/pageinspect/pageinspect--1.1--1.2.sql new file mode 100644 index 000..5e23ca4 --- /dev/null +++ b/contrib/pageinspect/pageinspect--1.1--1.2.sql @@ -0,0 +1,18 @@ +/* contrib/pageinspect/pageinspect--1.1--1.2.sql */ + +-- complain if script is sourced in psql, rather than via ALTER EXTENSION +\echo Use ALTER EXTENSION pageinspect UPDATE TO 1.2 to load this file. \quit + +DROP FUNCTION page_header(bytea); +CREATE FUNCTION page_header(IN page bytea, +OUT lsn pg_lsn, +OUT checksum smallint, +OUT flags smallint, +OUT lower smallint, +OUT upper smallint, +OUT special smallint, +OUT pagesize smallint, +OUT version smallint, +OUT prune_xid xid) +AS 'MODULE_PATHNAME', 'page_header' +LANGUAGE C STRICT; diff --git a/contrib/pageinspect/pageinspect--1.1.sql b/contrib/pageinspect/pageinspect--1.1.sql deleted file mode 100644 index 22a47d5..000 --- a/contrib/pageinspect/pageinspect--1.1.sql +++ /dev/null @@ -1,107 +0,0 @@ -/* contrib/pageinspect/pageinspect--1.1.sql */ - --- complain if script is sourced in psql, rather than via CREATE EXTENSION -\echo Use CREATE EXTENSION pageinspect to load this file. \quit - --- --- get_raw_page() --- -CREATE FUNCTION get_raw_page(text, int4) -RETURNS bytea -AS 'MODULE_PATHNAME', 'get_raw_page' -LANGUAGE C STRICT; - -CREATE FUNCTION get_raw_page(text, text, int4) -RETURNS bytea -AS 'MODULE_PATHNAME', 'get_raw_page_fork' -LANGUAGE C STRICT; - --- --- page_header() --- -CREATE FUNCTION page_header(IN page bytea, -OUT lsn text, -OUT checksum smallint, -OUT flags smallint, -OUT lower smallint, -OUT upper smallint, -OUT special smallint, -OUT pagesize smallint, -OUT version smallint, -OUT prune_xid xid) -AS 'MODULE_PATHNAME', 'page_header' -LANGUAGE C STRICT; - --- --- heap_page_items() --- -CREATE FUNCTION heap_page_items(IN page bytea, -OUT lp smallint, -OUT lp_off smallint, -OUT lp_flags smallint, -OUT lp_len smallint, -OUT t_xmin xid, -OUT t_xmax xid, -OUT t_field3 int4, -OUT t_ctid tid, -OUT t_infomask2 integer, -OUT t_infomask integer, -OUT t_hoff smallint, -OUT t_bits text, -OUT t_oid oid) -RETURNS SETOF record -AS 'MODULE_PATHNAME', 'heap_page_items' -LANGUAGE C STRICT; - --- --- bt_metap() --- -CREATE FUNCTION bt_metap(IN relname text, -OUT magic int4, -OUT version int4, -OUT root int4, -OUT level int4, -OUT fastroot int4, -OUT fastlevel int4) -AS 'MODULE_PATHNAME', 'bt_metap' -LANGUAGE C STRICT; - --- --- bt_page_stats() --- -CREATE FUNCTION bt_page_stats(IN relname text, IN blkno int4, -OUT blkno int4, -OUT type char, -OUT live_items int4, -OUT dead_items int4, -OUT avg_item_size int4, -OUT page_size int4, -OUT free_size int4, -OUT btpo_prev int4, -OUT btpo_next int4, -OUT btpo int4, -OUT btpo_flags int4) -AS 'MODULE_PATHNAME', 'bt_page_stats' -LANGUAGE C STRICT; - --- --- bt_page_items() --- -CREATE FUNCTION bt_page_items(IN relname text, IN blkno int4, -OUT itemoffset smallint, -OUT ctid tid, -OUT itemlen smallint, -OUT nulls bool, -OUT vars bool, -OUT data text) -RETURNS SETOF record -AS 'MODULE_PATHNAME', 'bt_page_items' -LANGUAGE C STRICT; - --- --- fsm_page_contents() --- -CREATE FUNCTION fsm_page_contents(IN page bytea) -RETURNS text -AS 'MODULE_PATHNAME', 'fsm_page_contents' -LANGUAGE C STRICT; diff --git a/contrib/pageinspect/pageinspect--1.2.sql b/contrib/pageinspect/pageinspect--1.2.sql new file mode 100644 index 000..15e8e1e --- /dev/null +++ b/contrib/pageinspect/pageinspect--1.2.sql @@ -0,0 +1,107 @@ +/* contrib/pageinspect/pageinspect--1.2.sql */ + +-- complain if script is sourced in psql, rather than via CREATE EXTENSION +\echo Use CREATE EXTENSION pageinspect to load this file. \quit + +-- +-- get_raw_page() +-- +CREATE FUNCTION get_raw_page(text, int4) +RETURNS bytea +AS 'MODULE_PATHNAME', 'get_raw_page' +LANGUAGE C STRICT; + +CREATE FUNCTION get_raw_page(text, text, int4) +RETURNS bytea +AS 'MODULE_PATHNAME', 'get_raw_page_fork' +LANGUAGE C STRICT; + +-- +-- page_header() +-- +CREATE FUNCTION page_header(IN page bytea, +OUT lsn pg_lsn, +OUT checksum
[HACKERS] commit fest 2014-01 week 5 report
Last week: Status Summary. Needs Review: 45, Waiting on Author: 13, Ready for Committer: 14, Committed: 38, Returned with Feedback: 4, Rejected: 2. Total: 116. This week: Status Summary. Needs Review: 43, Waiting on Author: 11, Ready for Committer: 12, Committed: 39, Returned with Feedback: 7, Rejected: 4. Total: 116. Everyone was busy with the minor releases, so it's not surprising that little progress was made. Back to the regular programming now. -- 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] [PATCH] Relocation of tablespaces in pg_basebackup
On 2/15/14, 7:05 PM, Peter Eisentraut wrote: I've been working on your patch. Attached is a version I'd be happy to commit. Please check that it's okay with you. Committed after some rebasing. -- 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] Dump pageinspect to 1.2: page_header with pg_lsn datatype
Michael Paquier escribió: Hi all, Please find attached a patch to dump pageinspect to 1.2. This simply changes page_header to use the new internal datatype pg_lsn instead of text. Uhm. Does this crash if you pg_upgrade a server that has 1.1 installed? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] 9.5: UPDATE/DELETE .. ORDER BY .. LIMIT ..
Hello hackers, I know you're busy wrapping up the 9.4 release, so please ignore this patch. ♜ update_delete_order_by_limit_v0.diff 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] 9.5: UPDATE/DELETE .. ORDER BY .. LIMIT ..
On Sat, Feb 22, 2014 at 3:46 PM, Rukh Meski rukh.me...@yahoo.ca wrote: I know you're busy wrapping up the 9.4 release, so please ignore this patch. I think you should describe what the patch does, why you believe the feature is necessary, and perhaps how it compares to other, similar things. You have documentation changes here, but that doesn't really tell us why this is important. -- Peter Geoghegan -- 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] 9.5: UPDATE/DELETE .. ORDER BY .. LIMIT ..
On Saturday, February 22, 2014 11:57:06 PM, Peter Geoghegan p...@heroku.com wrote: I think you should describe what the patch does, why you believe the feature is necessary, and perhaps how it compares to other, similar things. You have documentation changes here, but that doesn't really tell us why this is important. Sorry, I wanted to minimize the attention my message attracts. I mostly posted it to let people know I plan on working on this for 9.5 to avoid duplicated effort. I will post more documentation and my reasons for wanting this feature in postgre later, if that's all right. ♜ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review: tests for client programs
On 2/9/14, 1:01 AM, Pavel Stehule wrote: b) Prepared tests fails when PostgreSQL server was up - should be checked and should to raise a valuable error message The original patch used a hard-coded port number, which was a mistake. I have changed this now to use a nonstandard port number that is different from the compiled-in one, similar to how pg_regress used to do it. It's still not bullet-proof. Do we need to do more? you can check before starting test if some Postgres on this port is living or not. We have pg_isready. I'm having trouble reproducing this scenario. The tests use a Unix-domain socket in a private directory, so I don't see how that can conflict. Can you show me exactly how you invoked the tests and which tests and which tests failed? And what platform are you on? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review: tests for client programs
On 2/17/14, 9:19 AM, Alvaro Herrera wrote: there can be option --with-client-tests and this option should to require IRC::Run A configure option seems a workable idea. In the future we might want to use the Perl test framework for other things, so perhaps --enable-perl-testing or something like that. I don't think I want to add another configure option. That just reduces flexibility during development and will make it less likely that people can or will run the tests. Skipping tests because of missing dependencies is a standard behavior in TAP test suites. The alternative is that we make it a hard requirement. I'd be for that, but maybe next release. -- 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] SSL: better default ciphersuite
On 2/2/14, 7:16 AM, Marko Kreen wrote: On Thu, Dec 12, 2013 at 04:32:07PM +0200, Marko Kreen wrote: Attached patch changes default ciphersuite to HIGH:MEDIUM:+3DES:!aNULL and also adds documentation about reasoning for it. This is the last pending SSL cleanup related patch: https://commitfest.postgresql.org/action/patch_view?id=1310 Peter, you have claimed it as committer, do you see any remaining issues with it? I'm OK with this change on the principle of clarifying and refining the existing default. But after inspecting the expanded cipher list with the openssl cipher tool, I noticed that the new default re-enabled MD5 ciphers. Was that intentional? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] typemode for variable types
Hello. I have a new type similar to varchar. I want to fine how did I can to calculate typemod and where must I calculate typemod for this type. thanks.
Re: [HACKERS] Patch: show relation and tuple infos of a lock to acquire
On Sat, Feb 22, 2014 at 3:17 PM, Andres Freund and...@2ndquadrant.com wrote: On 2014-02-22 11:53:24 +0530, Amit Kapila wrote: In general, why I am suggesting to restrict display of newly added context for the case it is added to ensure that it doesn't get started displaying in other cases where it might make sense or might not make sense. Anything failing while inside an XactLockTableWait() et al. will benefit from the context. In which scenarios could that be unneccessary? This path is quite deep, so I have not verified that whether this new context can make sense for all error cases. For example, in below path (error message), it doesn't seem to make any sense (I have tried to generate it through debugger, actual message will vary). XactLockTableWait-SubTransGetParent-SimpleLruReadPage_ReadOnly-SimpleLruReadPage-SlruReportIOError ERROR: could not access status of transaction 927 DETAIL: Could not open file pg_subtrans/: No error. CONTEXT: relation public.t1 tuple ctid (0,2) (according to Andres we would have to look at rel-rd_node.dbNode) and since other commands dealing with names don't do so, either, I think we should leave out the database name. For this case as I have shown that in similar message, we already display database oid, it should not be a problem to display here as well. It will be more information to user. I don't think we do for any where we actually display the relation name, do we? I have not checked that, but the reason I mentioned about database name is due to display of database oid in similar message, please see the message below. I think in below context we get it from lock tag and I think for the patch case also, it might be better to display, but not a mandatory thing. Consider it as a suggestion which if you also feel good, then do it, else ignore it. LOG: process 4656 still waiting for ExclusiveLock on tuple (0,1) of relation 57 513 of database 12045 after 1046.000 ms Currently I simply display the whole tuple with truncating long fields. This is plain easy, no distinction necessary. I did some code reading and it seems somewhat complex to get the PK columns and it seems that we need another lock for this, too - maybe not the best idea when we are already in locking trouble, do you disagree? I don't think you need to take another lock for this, it must already have appropriate lock by that time. There should not be any problem in calling relationHasPrimaryKey except that currently it is static, you need to expose it. Other locations that deal with similar things (notably ExecBuildSlotValueDescription()) doesn't either. I don't think this patch should introduce it then. Displaying whole tuple doesn't seem to be the most right way to get debug information and especially in this case we are already displaying tuple offset(ctid) which is unique identity to identify a tuple. It seems to me that it is sufficient to display unique value of tuple if present. I understand that there is no clear issue here, so may be if others also share their opinion then it will be quite easy to take a call. PS - I will be out for work assignment for next week, so might not be able to complete the review, so others are welcome to takeup and complete it in the meantime. With Regards, Amit Kapila. 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
