Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 22 June 2013 05:17, Amit kapila amit.kap...@huawei.com wrote: On Friday, June 21, 2013 11:48 PM Robert Haas wrote: On Thu, Jun 20, 2013 at 12:18 AM, Amit Kapila amit.kap...@huawei.com wrote: Auto.conf- 1 Vote (Josh) System.auto.conf - 1 Vote (Josh) Postgresql.auto.conf - 2 Votes (Zoltan, Amit) Persistent.auto.conf - 0 Vote generated_by_server.conf - 1 Vote (Peter E) System.conf - 1 Vote (Magnus) alter_system.conf- 1 Vote (Alvaro) I had consolidated the list, so that it would be helpful for committer to decide among proposed names for this file. I'll also vote for postgresql.auto.conf. Thanks to all of you for suggesting meaningful names. I will change the name of file to postgresql.auto.conf. Kindly let me know if there is any objection to it. postgresql.auto.conf is similar enough to postgresql.conf that it will prevent tab completion from working as it does now. As a result it will cause people to bring that file up for editing when we wish to avoid that. So for me the name is not arbitrary because of that point and we should avoid the current choice. Can we go for auto.postgresql.conf instead almost identical, just significantly visually different and not interfering with tab completion? -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services
Re: [HACKERS] New regression test time
On 2 July 2013 18:43, Noah Misch n...@leadboat.com wrote: On Tue, Jul 02, 2013 at 10:17:08AM -0400, Robert Haas wrote: So I think the first question we need to answer is: Should we just reject Robins' patches en masse? If we do that, then the rest of this is moot. If we don't do that, then the second question is whether we should try to introduce a new schedule, and if so, whether we should split out that new schedule before or after committing these patches as they stand. It's sad to simply reject meaningful automated tests on the basis of doubt that they're important enough to belong in every human-in-the-loop test run. I share the broader vision for automated testing represented by these patches. +1 We should be encouraging people to submit automated tests. I share the annoyance of increasing the length of the automated test runs, and have watched them get longer and longer over time. But I run the tests because I want to see whether I broke anything and I stopped sitting and watching them run long ago. Automated testing is about x10-100 faster than manual testing, so I see new tests as saving me time not wasting it. Here are my opinions, for what they are worth. First, I think that rejecting these new tests is a bad idea, although I've looked them over a bit and I think there might be individual things we might want to take out. Second, I think that creating a new schedule is likely to cost developers more time than it saves them. Sure, you'll be able to run the tests slightly faster, but when you commit something, break the buildfarm (which does run those tests), and then have to go back and fix the tests (or your patch), you'll waste more time doing that than you saved by avoiding those few extra seconds of runtime. Third, I think if we do want to create a new schedule, it makes more sense to commit the tests first and then split out the new schedule according to some criteria that we devise. There should be a principled reason for putting tests in one schedule or the other; all the tests that Robins Tharakan wrote is not a good filter criteria. +1 for that plan. I don't know whether placing certain tests outside the main test sequence would indeed cost more time than it saves. I definitely agree that if these new tests should appear elsewhere, some of our existing tests should also move there. Let's have a new schedule called minute-check with the objective to run the common tests in 60 secs. We can continue to expand the normal schedules from here. Anybody that wants short tests can run that, everyone else can run the full test suite. We should be encouraging people to run the full test suite, not the fast one. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services
Re: [HACKERS] Improving avg performance for numeric
Hello 2013/3/20 Hadi Moshayedi h...@moshayedi.net: Hi Tom, Tom Lane t...@sss.pgh.pa.us wrote: After thinking about that for awhile: if we pursue this type of optimization, what would probably be appropriate is to add an aggregate property (stored in pg_aggregate) that allows direct specification of the size that the planner should assume for the aggregate's transition value. We were getting away with a hardwired assumption of 8K for internal because the existing aggregates that used that transtype all had similar properties, but it was always really a band-aid not a proper solution. A per-aggregate override could be useful in other cases too. Cool. I created a patch which adds an aggregate property to pg_aggregate, so the transition space is can be overridden. This patch doesn't contain the numeric optimizations. It uses 0 (meaning not-set) for all existing aggregates. I manual-tested it a bit, by changing this value for aggregates and observing the changes in plan. I also updated some docs and pg_dump. Does this look like something along the lines of what you meant? please, can you subscribe your patch to next commitfest? I tested this patch, and it increase performance about 20% what is interesting. More - it allows more comfortable custom aggregates for custom types with better hash agg support. Regards Pavel Thanks, -- Hadi -- 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 unused buffers to freelist
On 28 June 2013 05:52, Amit Kapila amit.kap...@huawei.com wrote: As per my understanding Summarization of points raised by you and Andres which this patch should address to have a bigger win: 1. Bgwriter needs to be improved so that it can help in reducing usage count and finding next victim buffer (run the clock sweep and add buffers to the free list). 2. SetLatch for bgwriter (wakeup bgwriter) when elements in freelist are less. 3. Split the workdone globallock (Buffreelist) in StrategyGetBuffer (a spinlock for the freelist, and an lwlock for the clock sweep). 4. Separate processes for writing dirty buffers and moving buffers to freelist 5. Bgwriter needs to be more aggressive, logic based on which it calculates how many buffers it needs to process needs to be improved. 6. There can be contention around buffer mapping locks, but we can focus on it later 7. cacheline bouncing around the buffer header spinlocks, is there anything we can do to reduce this? My perspectives here would be * BufFreelistLock is a huge issue. Finding a next victim block needs to be an O(1) operation, yet it is currently much worse than that. Measuring contention on that lock hides that problem, since having shared buffers lock up for 100ms or more but only occasionally is a huge problem, even if it doesn't occur frequently enough for the averaged contention to show as an issue. * I'm more interested in reducing response time spikes than in increasing throughput. It's easy to overload a benchmark so we get better throughput numbers, but that's not helpful if we make the system more bursty. * bgwriter's effectiveness is not guaranteed. We have many clear cases where it is useless. So the question should be to continually answer the question: do we need a bgwriter and if so, what should it do? The fact we have one already doesn't mean it should be given things to do. It is a possible option that things may be better if it did nothing. (Not saying that is true, just that we must consider that optione ach time). -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Wednesday, July 03, 2013 11:42 AM Simon Riggs wrote: On 22 June 2013 05:17, Amit kapila amit.kap...@huawei.com wrote: On Friday, June 21, 2013 11:48 PM Robert Haas wrote: On Thu, Jun 20, 2013 at 12:18 AM, Amit Kapila amit.kap...@huawei.com wrote: Auto.conf - 1 Vote (Josh) System.auto.conf - 1 Vote (Josh) Postgresql.auto.conf - 2 Votes (Zoltan, Amit) Persistent.auto.conf - 0 Vote generated_by_server.conf - 1 Vote (Peter E) System.conf - 1 Vote (Magnus) alter_system.conf - 1 Vote (Alvaro) I had consolidated the list, so that it would be helpful for committer to decide among proposed names for this file. I'll also vote for postgresql.auto.conf. Thanks to all of you for suggesting meaningful names. I will change the name of file to postgresql.auto.conf. Kindly let me know if there is any objection to it. postgresql.auto.conf is similar enough to postgresql.conf that it will prevent tab completion from working as it does now. As a result it will cause people to bring that file up for editing when we wish to avoid that. This new file postgresql.auto.conf will be created inside a new directory config, so both will be in different directories. Will there be any confusion with tab completion for different directories? So for me the name is not arbitrary because of that point and we should avoid the current choice. Can we go for auto.postgresql.conf instead almost identical, just significantly visually different and not interfering with tab completion? With Regards, Amit Kapila. -- 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: clean up addRangeTableEntryForFunction
Robert Haas wrote: On Tue, Jan 22, 2013 at 11:45 AM, David Fetter da...@fetter.org wrote: I've been working with Andrew Gierth (well, mostly he's been doing the work, as usual) to add WITH ORDINALITY as an option for set-returning functions. In the process, he found a minor opportunity to clean up the interface for $SUBJECT, reducing the call to a Single Point of Truth for lateral-ness, very likely improving the efficiency of calls to that function. Please find attached the patch. I think this patch is utterly pointless. I recommend we reject it. If this were part of some larger refactoring that was going in some direction we could agree on, it might be worth it. But as it is, I think it's just a shot in the dark whether this change will end up being better or worse, and my personal bet is that it won't make any difference whatsoever. To be frank, I agree with Robert. Sorry for the delay in my review. Best regards, Etsuro Fujita -- 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] Reduce maximum error in tuples estimation after vacuum.
Hello, I could see the same output with your latest script, also I could reproduce the test if I run the test with individual sql statements. One of the main point for reproducing individual test was to keep autovacuum = off. I see. Autovacuum's nap time is 60 sconds for the default settings. Your operation might help it to snipe the window between the last massive delete and the next explict vacuum in store_result().. Anyway setting autovacuum to off should aid to make clean environment fot this issue. Now I can look into it further, I have still not gone through in detail about your new approach to calculate the reltuples, but I am wondering whether there can be anyway with which estimates can be improved with different calculation in vac_estimate_reltuples(). I'll explain this in other words alghough It might be repetitious. It is tough to decide how to modify there. Currently I decided to preserve vac_estimate_reltuples as possible as it is. For that objective, I picked up old_rel_tuples as intermediate variable for the aid to 'deceive' the function. This can be different form deciding to separate this estimation function from that for analyze. As I described before, vac_estimates_reltuples has a presumption that the tuple density in skipped pages is not so different from that in whole table before vacuuming. Since the density is calculated without using any hint about the skipped pages, and it cannot tell how much tuples aganst pg_class.reltuples is already dead, the value can be far different from the true one and cannot be verified. Given that we canot use pg_stat_user_tables.n_dead_tup, reading all pages can fix it but the penalty should be intolerable. Using FSM to know the used bytes in skipped pages (which is all visible by the definition) seems to give good estimations of the tuples in the skipped pages to some extent assuming the uniformity of tuple length. Of course strong deviation in length can deceive the algorithm. Does it make sense for you? I might could show the numerical explanation but I'm afraind I can't do it for now. I'll be able to take time sooner... (also for reviewing..) One thing I have observed that 2nd parameter is_analyze of vac_estimate_reltuples() is currently not used. Mmm, it seems to have been useless from the beginning of the function... I cannot work on it till early next week, so others are welcome to join regards, -- Kyotaro Horiguchi 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] A better way than tweaking NTUP_PER_BUCKET
Hi all, I have been working on a patch for the above discussed functionalities. I made an array of int32s, one for each bucket in a hash table(the array is per hash table). I am using the hash value directly to set the corresponding bit in the bit field.Specifically: bitfield_orvalue = 1hashvalue; hashtable-bitFields[bucketNumber] = (hashtable-bitFields[bucketNumber]) |bitfield_orvalue; But,the hash values are way beyond this, and I admit that my choice of int32 as bitfield isn't correct here. The hash values are like: 1359910425 1359910425 -1662820951 -1662820951 What should I be using for the bit map? Regards, Atri -- Regards, Atri l'apprenant -- 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] Add an ldapoption to disable chasing LDAP referrals
On Wed, Jul 3, 2013 at 3:04 AM, James Sewell james.sew...@lisasoft.comwrote:
Hey Peter,
You are correct, it is the same as the referrals option in pam_ldap. It's
also the -C (sometimes -R - it seems ldapsearch options are pretty
non-standard) option in ldapsearch.
As far as I'm aware you can't pass this in an LDAP URL, primarily because
this never gets sent to the LDAP server. The server always returns an LDIF
with inline references, this just determines if you chase them client side
or just list them as is.
I could be missing something here, but using:
ldapreferrals={0|1}
Would require a three state type, as we need a way of not interfering with
the library defaults? To 'enable' the new behavior here using a boolean you
would need to set ldapreferrals=false - which with the normal way of
dealing with config booleans would alter the default behavior if the option
was not specified.
How do you feel about:
ldapdisablereferrals=(0|1)
I agree with Peter that the negative thing is bad. l don't see the problem,
really. If you don't specify it, you rely on library defaults. If you do
specify it, we lock it to that setting. I don't see the need to
specifically have a setting to rely on library defaults - just remove it
from the line and you get that.
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
Re: [HACKERS] Improvement of checkpoint IO scheduler for stable transaction responses
Hi,
I tested and changed segsize=0.25GB which is max partitioned table file size and
default setting is 1GB in configure option (./configure --with-segsize=0.25).
Because I thought that small segsize is good for fsync phase and background disk
write in OS in checkpoint. I got significant improvements in DBT-2 result!
* Performance result in DBT-2 (WH340)
| NOTPM90%tileAverage Maximum
-+---
original_0.7 (baseline) | 3474.62 18.348328 5.73936.977713
fsync + write| 3586.85 14.459486 4.96027.266958
fsync + write + segsize=0.25 | 3661.17 8.288164.11717.23191
Changing segsize with my checkpoint patches improved original over 50% at
90%tile
and maximum response time.
However, this tests ware not same condition... I also changed SESSION parameter
100 to 300 in DBT-2 driver. In general, I heard good SESSION parameter is 100.
Andt I didn't understand optimized DBT-2 parameters a lot. So I will retry to
test my patches and baseline with optimized parameters in DBT-2. Please wait for
a while.
Best regards,
--
Mitsumasa KONDO
NTT Open Source Software Center
diff --git a/configure b/configure
index 7c662c3..6269cb9 100755
--- a/configure
+++ b/configure
@@ -2879,7 +2879,7 @@ $as_echo $as_me: error: Invalid block size. Allowed values are 1,2,4,8,16,32.
esac
{ $as_echo $as_me:$LINENO: result: ${blocksize}kB 5
$as_echo ${blocksize}kB 6; }
-
+echo ${blocksize}
cat confdefs.h _ACEOF
#define BLCKSZ ${BLCKSZ}
@@ -2917,14 +2917,15 @@ else
segsize=1
fi
-
# this expression is set up to avoid unnecessary integer overflow
# blocksize is already guaranteed to be a factor of 1024
-RELSEG_SIZE=`expr '(' 1024 / ${blocksize} ')' '*' ${segsize} '*' 1024`
-test $? -eq 0 || exit 1
+#RELSEG_SIZE=`expr '(' 1024 / ${blocksize} ')' '*' ${segsize} '*' 1024`
+RELSEG_SIZE=`echo 1024/$blocksize*$segsize*1024 | bc`
+#test $? -eq} 0 || exit 1
{ $as_echo $as_me:$LINENO: result: ${segsize}GB 5
$as_echo ${segsize}GB 6; }
-
+echo ${segsize}
+echo ${RELSEG_SIZE}
cat confdefs.h _ACEOF
#define RELSEG_SIZE ${RELSEG_SIZE}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] possible/feasible to specify field and value in error msg?
Hi, I have some complicated query that truncates and fills a table and i get this message: ERROR: smallint out of range STATEMENT: my huge query This is in postgres 8.4 I don't know where the error is, and the query takes rather long. So it is going to be a bit cumbersome for me to debug this. Would it be possible/feasible to specify, in future versions of postgres: * what value * which field (of which table) * the offending tuple? (possibly truncated to some threshold nr of characters) I ask because i can imagine that, inside the code that handles this, you might not have access to that information and adding access to it might be inefficient. I do get the whole query of course, and that is very handy for automated things. But in this case, it doesn't help me. Cheers, WBL -- Quality comes from focus and clarity of purpose -- Mark Shuttleworth
Re: [HACKERS] proposal: simple date constructor from numeric values
2013/7/2 Pavel Stehule pavel.steh...@gmail.com: 2013/7/1 Peter Eisentraut pete...@gmx.net: On 7/1/13 3:47 AM, Pavel Stehule wrote: and it is a part of our ToDo: Add function to allow the creation of timestamps using parameters so we can have a functions with signatures I would just name them date(...), time(...), etc. I tested this names, and I got a syntax error for function time we doesn't support real type constructors, and parser doesn't respect syntax. so we can use a different names, or we can try to implement type constructor functions. Comments Regards Pavel +1 CREATE OR REPLACE FUNCTION construct_date(year int, month int DEFAULT 1, day int DEFAULT 1) RETURNS date; I would not use default values for this one. I have no problem with it CREATE OR REPLACE FUNCTION construct_time(hour int DEFAULT 0, mi int DEFAULT 0, sec int DEFAULT 0, ms float DEFAULT 0.0); If we are using integer datetime storage, we shouldn't use floats to construct them. so possible signature signature should be CREATE FUNCTION time(hour int, mi int, sec int, used int) ?? and CREATE FUNCTION timetz(hour int, mi int, sec int, isec int, tz int) ?? Regards Pavel -- 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] proposal: simple date constructor from numeric values
2013/7/3 Pavel Stehule pavel.steh...@gmail.com: 2013/7/2 Pavel Stehule pavel.steh...@gmail.com: 2013/7/1 Peter Eisentraut pete...@gmx.net: On 7/1/13 3:47 AM, Pavel Stehule wrote: and it is a part of our ToDo: Add function to allow the creation of timestamps using parameters so we can have a functions with signatures I would just name them date(...), time(...), etc. I tested this names, and I got a syntax error for function time we doesn't support real type constructors, and parser doesn't respect syntax. so we can use a different names, or we can try to implement type constructor functions. constructor function - : A niladic SQL-invoked function of which exactly one is implicitly specified for every structured type. An invocation of the constructor function for data type returns a value of the most specific type such that is not null ... as minimum for Postgres - a possibility to implement function with same name as type name. Regards Pavel . Comments Regards Pavel +1 CREATE OR REPLACE FUNCTION construct_date(year int, month int DEFAULT 1, day int DEFAULT 1) RETURNS date; I would not use default values for this one. I have no problem with it CREATE OR REPLACE FUNCTION construct_time(hour int DEFAULT 0, mi int DEFAULT 0, sec int DEFAULT 0, ms float DEFAULT 0.0); If we are using integer datetime storage, we shouldn't use floats to construct them. so possible signature signature should be CREATE FUNCTION time(hour int, mi int, sec int, used int) ?? and CREATE FUNCTION timetz(hour int, mi int, sec int, isec int, tz int) ?? Regards Pavel -- 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] proposal: simple date constructor from numeric values
On 1 July 2013 17:47, Pavel Stehule pavel.steh...@gmail.com wrote: 2013/6/29 Pavel Stehule pavel.steh...@gmail.com: long time I am thinking about simple function for creating date or timestamp values based on numeric types without necessity to create format string. What do you think about this idea? I found so same idea was discussed three years ago http://www.postgresql.org/message-id/14107.1276443...@sss.pgh.pa.us I suggested something similar also: http://www.postgresql.org/message-id/AANLkTi=W1wtcL7qR4PuQaQ=uoabmjsusz6qgjtucx...@mail.gmail.com The thread I linked died off without reaching a consensus about what the functions ought to be named, although Josh and Robert were generally supportive of the idea. The function signatures I have been using in my own C functions are: * date(year int, month int, day int) returns date * datetime(year int, month int, day int, hour int, minute int, second int) returns timestamp Cheers, BJ -- 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.3 bug fix] ECPG does not escape backslashes
Hello, I happened to find a trivial bug of ECPG while experimenting with 9.3 beta 2. Please find attached the patch to fix this. This is not specific to 9.3. Could you commit and backport this? [Bug description] Running ecpg c:\command\a.pgc produces the following line in a.c: #line 1 c:\command\a.pgc Then, compiling the resulting a.c with Visual Studio (cl.exe) issues the warning: a.c(8) : warning C4129: 'c' : unrecognized character escape sequence This is because ecpg doesn't escape \ in the #line string. [How to fix] Escape \ in the input file name like this: #line 1 c:\\command\\a.pgc This is necessary not only on Windows but also on UNIX/Linux. For your information, running gcc -E di\\r/a.c escapes \ and outputs the line: # 1 di\\r/a.c Regards MauMau ecpg_line.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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Wednesday, July 03, 2013 2:58 AM Alvaro Herrera wrote: Amit Kapila escribió: I have changed the file name to postgresql.auto.conf and I have changed the name of SetPersistentLock to AutoFileLock. Zoltan, Could you please once check the attached Patch if you have time, as now all the things are resolved except for small doubt in syntax extensibility which can be changed based on final decision. There are a bunch of whitespace problems, as you would notice with git diff --check or git show --color. Could you please clean that up? Also, some of the indentation in various places is not right; perhaps you could fix that by running pgindent over the source files you modified (this is easily visible by eyeballing the git diff output; stuff is misaligned in pretty obvious ways.) Random minor other comments: + use of xref linkend=SQL-ALTER SYSTEM this SGML link cannot possibly work. Please run make check in the doc/src/sgml directory. Examples in alter_system.sgml have a typo SYTEM. Same file has or or. I will fix above issues. Also in that file, The name of an configuration parameter that exist in filenamepostgresql.conf/filename. the parameter needn't exist in that file to be settable, right? refnamediv refnameALTER SYSTEM/refname refpurposechange a server configuration parameter/refpurpose /refnamediv Yes you are right. How about below line, same parameter description as SET command Name of a settable run-time parameter. Available parameters are documented in Chapter 18 Perhaps permanently change ..? I am not sure, if adding permanently is appropriate here, as one could also interpret it, that after parameter is changed with this command, it can never be changed again. Also, some parameters require a restart, not a reload; maybe we should direct the user somewhere else to learn how to freshen up the configuration after calling the command. Below link contains useful information in this regard: http://www.postgresql.org/docs/devel/static/config-setting.html Particularly refer below para in above link (the below text is for your reference to see if above link is useful): The configuration file is reread whenever the main server process receives a SIGHUP signal; this is most easily done by running pg_ctl reload from the command-line or by calling the SQL function pg_reload_conf(). The main server process also propagates this signal to all currently running server processes so that existing sessions also get the new value. Alternatively, you can send the signal to a single server process directly. Some parameters can only be set at server start; any changes to their entries in the configuration file will be ignored until the server is restarted. Invalid parameter settings in the configuration file are likewise ignored (but logged) during SIGHUP processing. + /* skip auto conf temporary file */ + if (strncmp(de-d_name, + PG_AUTOCONF_FILENAME ., + sizeof(PG_AUTOCONF_FILENAME)) == 0) + continue; What's the dot doing there? This was from previous version of patch, now it is not required, I will remove it. + if ((stat(AutoConfFileName, st) == -1)) + ereport(elevel, + (errmsg(configuration parameters changed with ALTER SYSTEM command before restart of server + will not be effective as \%s\ file is not accessible., PG_AUTOCONF_FILENAME))); + else + ereport(elevel, + (errmsg(configuration parameters changed with ALTER SYSTEM command before restart of server + will not be effective as default include directive for \%s\ folder is not present in postgresql.conf, PG_AUTOCONF_DIR))); These messages should be split. Perhaps have the will not be effective in the errmsg() and the reason as part of errdetail()? How about changing to below messages ereport(elevel, (errmsg(configuration parameters changed with ALTER SYSTEM command will not be effective) errdetail(file \%s\ containing configuration parameters is not accessible., PG_AUTOCONF_FILENAME))); ereport(elevel, (errmsg(configuration parameters changed with ALTER SYSTEM command will not be effective) errdetail(default include directive for \%s\ folder is not present in postgresql.conf, PG_AUTOCONF_DIR))); Also, I'm not really sure about doing another stat() on the file after parsing failed; I think the parse routine should fill some failure context information, instead of having this code trying to reproduce the failure to know what to report. Maybe something like the SlruErrorCause enum, not sure. It might not be feasible with current implementation as currently it just note down whether it has parsed file with name postgresql.auto.conf and then in outer function takes decision based on that parameter.
Re: [HACKERS] Custom gucs visibility
If we haven't loaded the .so yet, where would we get the list of custom GUCs from? This has come up before. We could show the string value of the GUC, if it's been set in postgresql.conf, but we do not have correct values for any of the other columns in pg_settings; nor are we even sure that the module will think the value is valid once it does get loaded. So the consensus has been that allowing the GUC to be printed would be more misleading than helpful. How about printing them with something along the lines of, Please load extension foobar for details or (less informative, but possibly easier to code) libfoobar.so not loaded. ? Well, we have done the CREATE EXTENSION successfully earlier. Also, the GUC becomes automagically visible after the backend has executed a function from that extension ( in which case the .so gets loaded as part of the function handling). Also note that SET foo.custom_guc works ok by setting up a placeholder guc if the .so has not been loaded yet. I wonder if we should dare to try to load the .so if a 'SHOW extension_name.custom_guc' is encountered via internal_load_library or something? Obviously we should check if the extension was created before as well. Regards, Nikhils Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
Re: [HACKERS] proposal: simple date constructor from numeric values
Hello 2013/7/3 Brendan Jurd dire...@gmail.com: On 1 July 2013 17:47, Pavel Stehule pavel.steh...@gmail.com wrote: 2013/6/29 Pavel Stehule pavel.steh...@gmail.com: long time I am thinking about simple function for creating date or timestamp values based on numeric types without necessity to create format string. What do you think about this idea? I found so same idea was discussed three years ago http://www.postgresql.org/message-id/14107.1276443...@sss.pgh.pa.us I suggested something similar also: http://www.postgresql.org/message-id/AANLkTi=W1wtcL7qR4PuQaQ=uoabmjsusz6qgjtucx...@mail.gmail.com The thread I linked died off without reaching a consensus about what the functions ought to be named, although Josh and Robert were generally supportive of the idea. The function signatures I have been using in my own C functions are: * date(year int, month int, day int) returns date * datetime(year int, month int, day int, hour int, minute int, second int) returns timestamp I am thinking so for these functions exists some consensus - minimally for function date(year, month, int) - I dream about this function ten years :) I am not sure about datetime: a) we use timestamp name for same thing in pg b) we can simply construct timestamp as sum of date + time, what is little bit more practical (for me), because it doesn't use too wide parameter list. so my proposal is two new function date and time but, because we doesn't support type constructor function, I don't think so name date is good (in this moment) MSSQL has function DATEFROMPARTS, TIMEFROMPARTS and DATETIMEFROMPARTS MySQL has little bit obscure function MAKEDATE(year, dayinyear) and MAKETIME(hour, min, sec) Oracle and db2 has nothing similar what do you think about names? make_date make_time I don't would to use to_date, to_time functions, a) because these functions use formatted input, b) we hold some compatibility with Oracle. Regards Pavel Stehule Cheers, BJ -- 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] proposal: enable new error fields in plpgsql (9.4)
On Wed, Jul 03, 2013 at 06:17:18AM +0200, Pavel Stehule wrote: 2013/7/2 Noah Misch n...@leadboat.com: Here's a revision bearing the discussed name changes and protocol documentation tweaks, along with some cosmetic adjustments. If this seems good to you, I will commit it. +1 Done. Rushabh, I neglected to credit you as a reviewer and realized it too late. Sorry about that. -- Noah Misch 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] proposal: enable new error fields in plpgsql (9.4)
2013/7/3 Noah Misch n...@leadboat.com: On Wed, Jul 03, 2013 at 06:17:18AM +0200, Pavel Stehule wrote: 2013/7/2 Noah Misch n...@leadboat.com: Here's a revision bearing the discussed name changes and protocol documentation tweaks, along with some cosmetic adjustments. If this seems good to you, I will commit it. +1 Done. Thank you, very much Regards Pavel Rushabh, I neglected to credit you as a reviewer and realized it too late. Sorry about that. -- Noah Misch 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] Move unused buffers to freelist
On Wednesday, July 03, 2013 12:27 PM Simon Riggs wrote: On 28 June 2013 05:52, Amit Kapila amit.kap...@huawei.com wrote: As per my understanding Summarization of points raised by you and Andres which this patch should address to have a bigger win: 1. Bgwriter needs to be improved so that it can help in reducing usage count and finding next victim buffer (run the clock sweep and add buffers to the free list). 2. SetLatch for bgwriter (wakeup bgwriter) when elements in freelist are less. 3. Split the workdone globallock (Buffreelist) in StrategyGetBuffer (a spinlock for the freelist, and an lwlock for the clock sweep). 4. Separate processes for writing dirty buffers and moving buffers to freelist 5. Bgwriter needs to be more aggressive, logic based on which it calculates how many buffers it needs to process needs to be improved. 6. There can be contention around buffer mapping locks, but we can focus on it later 7. cacheline bouncing around the buffer header spinlocks, is there anything we can do to reduce this? My perspectives here would be * BufFreelistLock is a huge issue. Finding a next victim block needs to be an O(1) operation, yet it is currently much worse than that. Measuring contention on that lock hides that problem, since having shared buffers lock up for 100ms or more but only occasionally is a huge problem, even if it doesn't occur frequently enough for the averaged contention to show as an issue. To optimize finding next victim buffer, I am planning to run the clock sweep in background. Apart from that do you have any idea to make it closer to O(1)? With Regards, Amit Kapila. -- 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 unused buffers to freelist
On 3 July 2013 12:56, Amit Kapila amit.kap...@huawei.com wrote: My perspectives here would be * BufFreelistLock is a huge issue. Finding a next victim block needs to be an O(1) operation, yet it is currently much worse than that. Measuring contention on that lock hides that problem, since having shared buffers lock up for 100ms or more but only occasionally is a huge problem, even if it doesn't occur frequently enough for the averaged contention to show as an issue. To optimize finding next victim buffer, I am planning to run the clock sweep in background. Apart from that do you have any idea to make it closer to O(1)? Yes, I already posted patches to attentuate the search time. Please check back last few CFs of 9.3 -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services
Re: [HACKERS] Review: Patch to compute Max LSN of Data Pages
On Wednesday, July 03, 2013 1:26 AM Robert Haas wrote: On Tue, Jun 25, 2013 at 1:42 PM, Andres Freund and...@2ndquadrant.com wrote: I think the usecase for this utility isn't big enough to be included in postgres since it really can only help in a very limited circumstances. And I think it's too likely to be misused for stuff it's not useable for (e.g. remastering). The only scenario I see is that somebody deleted/corrupted pg_controldata. In that scenario the tool is supposed to be used to find the biggest lsn used so far so the user then can use pg_resetxlog to set that as the wal starting point. But that can be way much easier solved by just setting the LSN to something very, very high. The database cannot be used for anything reliable afterwards anyway. I guess this is true, but I think I'm mildly in favor of including this anyway. I think I would have used it once or twice, if it had been there - maybe not even to feed into pg_resetxlog, but just to check for future LSNs. We don't have anything like a suite of diagnostic tools in bin or contrib today, for use by database professionals in fixing things that fall strictly in the realm of don't try this at home, and I think we should have such a thing. Unfortunately this covers about 0.1% of the space I'd like to see covered, which might be a reason to reject it or at least insist that it be enhanced first. At any rate, I don't think this is anywhere near committable as it stands today. Some random review comments: remove_parent_refernces() is spelled wrong. It was corrected in last posted version. http://www.postgresql.org/message-id/6C0B27F7206C9E4CA54AE035729E9C383BEB928 8@szxeml509-mbx Why does this patch need all of this fancy path-handling logic - specifically, remove_parent_refernces() and make_absolute_path()? Surely its needs are not that different from pg_ctl or pg_resetxlog or pg_controldata. If they all need it and it's duplicated, we should fix that. Otherwise, why the special logic here? I don't think we need getLinkPath() either. The server has no trouble finding its files by just using a pathname that follows the symlink. Why do we need anything different here? The whole point of symlinks is that you can traverse them transparently, without knowing where they point. It is to handle negative scenario's like if there is any recursion in path. However if you feel this is not important, it can be removed. Duplicating PageHeaderIsValid doesn't seem acceptable. Moreover, since the point of this is to be able to use it on a damaged cluster, why is that logic even desirable? I think we'd be better off assuming the pages to be valid. The calling convention for this utility seems quite baroque. There's no real need for all of this -p/-P stuff. I think the syntax should just be: pg_computemaxlsn file_or_directory... For each argument, we determine whether it's a file or a directory. If it's a file, we assume it's a PostgreSQL data file and scan it. If it's a directory, we check whether it looks like a data directory. If it does, we recurse through the whole tree structure and find the data files, and process them. If it doesn't look like a data directory, we scan each plain file in that directory whose name looks like a PostgreSQL data file name. With this approach, there's no need to limit ourselves to a single input argument and no need to specify what kind of argument it is; the tool just figures it out. I think it would be a good idea to have a mode that prints out the max LSN found in *each data file* scanned, and then prints out the overall max found at the end. In fact, I think that should perhaps be the default mode, with -q, --quiet to disable it. Printing too many LSN for each file might fill user's screen and he might be needing only overall LSN. Should we keep -p --printall as option to print all LSN's and keep default as overall max LSN? With Regards, Amit Kapila. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 3 July 2013 07:45, Amit Kapila amit.kap...@huawei.com wrote: postgresql.auto.conf is similar enough to postgresql.conf that it will prevent tab completion from working as it does now. As a result it will cause people to bring that file up for editing when we wish to avoid that. This new file postgresql.auto.conf will be created inside a new directory config, so both will be in different directories. Will there be any confusion with tab completion for different directories? Tab completion will not be confused, no. But I think everything else will be. How will I know that some settings have been set by ALTER SYSTEM and some by other means? Sounds to me like a recipe for chaos. I hope nobody is calling for a commit on this anytime soon. I think the whole thing needs some careful thought and review before we commit this. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services
Re: [HACKERS] Review: Patch to compute Max LSN of Data Pages
On Wed, Jul 3, 2013 at 8:44 AM, Amit Kapila amit.kap...@huawei.com wrote: Why does this patch need all of this fancy path-handling logic - specifically, remove_parent_refernces() and make_absolute_path()? Surely its needs are not that different from pg_ctl or pg_resetxlog or pg_controldata. If they all need it and it's duplicated, we should fix that. Otherwise, why the special logic here? I don't think we need getLinkPath() either. The server has no trouble finding its files by just using a pathname that follows the symlink. Why do we need anything different here? The whole point of symlinks is that you can traverse them transparently, without knowing where they point. It is to handle negative scenario's like if there is any recursion in path. However if you feel this is not important, it can be removed. I'm having a hard time imagining a situation where that would be a problem. If the symlink points to itself somehow, the OS will throw an error. If your filesystem is so badly hosed that the directory structure is more fundamentally broken than the OS's circular-symlink detection code can handle, whether or not this utility works is a second-order consideration. What kind of scenario are you imagining? I think it would be a good idea to have a mode that prints out the max LSN found in *each data file* scanned, and then prints out the overall max found at the end. In fact, I think that should perhaps be the default mode, with -q, --quiet to disable it. Printing too many LSN for each file might fill user's screen and he might be needing only overall LSN. Should we keep -p --printall as option to print all LSN's and keep default as overall max LSN? Honestly, I have a hard time imagining the use case for that mode. This isn't a tool that people should be running regularly, and some output that lends a bit of confidence that the tool is doing the right thing seems like a good thing. Keep in mind it's likely to run for quite a while, too, and this would provide a progress indicator. I'll defer to whatever the consensus is here but my gut feeling is that if you don't want that extra output, there's a good chance you're misusing the tool. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Move unused buffers to freelist
On Wednesday, July 03, 2013 6:10 PM Simon Riggs wrote: On 3 July 2013 12:56, Amit Kapila amit.kap...@huawei.com wrote: My perspectives here would be * BufFreelistLock is a huge issue. Finding a next victim block needs to be an O(1) operation, yet it is currently much worse than that. Measuring contention on that lock hides that problem, since having shared buffers lock up for 100ms or more but only occasionally is a huge problem, even if it doesn't occur frequently enough for the averaged contention to show as an issue. To optimize finding next victim buffer, I am planning to run the clock sweep in background. Apart from that do you have any idea to make it closer to O(1)? Yes, I already posted patches to attentuate the search time. Please check back last few CFs of 9.3 Okay, I got it. I think you mean 9.2. Patch: Reduce locking on StrategySyncStart() https://commitfest.postgresql.org/action/patch_view?id=743 Patch: Reduce freelist locking during DROP TABLE/DROP DATABASE https://commitfest.postgresql.org/action/patch_view?id=744 I shall pay attention to patches and the discussion during my work on enhancement of this patch. With Regards, Amit Kapila. -- 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.4 CF 1] The Commitfest Slacker List
On Tue, Jul 02, 2013 at 04:00:22PM -0400, Robert Haas wrote: make you review patches against your will. Don't take it for more than what Josh meant it as. And that was what? Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org Jabber: michael.meskes at gmail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL -- 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.4 CF 1] The Commitfest Slacker List
On Tue, Jul 02, 2013 at 09:42:43PM -0700, Josh Berkus wrote: Clearly I ticked off a bunch of people by publishing the list. On the other hand, in the 5 days succeeding the post, more than a dozen additional people signed up to review patches, and we got some of the ready for committer patches cleared out -- something which nothing else I did, including dozens of private emails, general pleas to this mailing list, mails to the RRReviewers list, served to accomplish, in this or previous CFs. So your saying the end justifies the means? I beg to disagree. So, as an experiment, call it a mixed result. I would like to have some other way to motivate reviewers than public shame. I'd like to have Doesn't shame imply that people knew that were supposed to review patches in the first place? An implication that is not true, at least for some on your list. I think I better not bring up the word I would describe your email with, just for the fear of mistranslating it. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org Jabber: michael.meskes at gmail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL -- 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] Improvement of checkpoint IO scheduler for stable transaction responses
On Wed, Jul 3, 2013 at 4:18 AM, KONDO Mitsumasa
kondo.mitsum...@lab.ntt.co.jp wrote:
I tested and changed segsize=0.25GB which is max partitioned table file size
and
default setting is 1GB in configure option (./configure --with-segsize=0.25).
Because I thought that small segsize is good for fsync phase and background
disk
write in OS in checkpoint. I got significant improvements in DBT-2 result!
This is interesting. Unfortunately, it has a significant downside:
potentially, there will be a lot more files in the data directory. As
it is, the number of files that exist there today has caused
performance problems for some of our customers. I'm not sure off-hand
to what degree those problems have been related to overall inode
consumption vs. the number of files in the same directory.
If the problem is mainly with number of of files in the same
directory, we could consider revising our directory layout. Instead
of:
base/${DBOID}/${RELFILENODE}_{FORK}
We could have:
base/${DBOID}/${FORK}/${RELFILENODE}
That would move all the vm and fsm forks to separate directories,
which would cut down the number of files in the main-fork directory
significantly. That might be worth doing independently of the issue
you're raising here. For large clusters, you'd even want one more
level to keep the directories from getting too big:
base/${DBOID}/${FORK}/${X}/${RELFILENODE}
...where ${X} is two hex digits, maybe just the low 16 bits of the
relfilenode number. But this would be not as good for small clusters
where you'd end up with oodles of little-tiny directories, and I'm not
sure it'd be practical to smoothly fail over from one system to the
other.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Wednesday, July 03, 2013 6:29 PM Simon Riggs wrote: On 3 July 2013 07:45, Amit Kapila amit.kap...@huawei.com wrote: postgresql.auto.conf is similar enough to postgresql.conf that it will prevent tab completion from working as it does now. As a result it will cause people to bring that file up for editing when we wish to avoid that. This new file postgresql.auto.conf will be created inside a new directory config, so both will be in different directories. Will there be any confusion with tab completion for different directories? Tab completion will not be confused, no. But I think everything else will be. How will I know that some settings have been set by ALTER SYSTEM and some by other means? Other means by hand editing postgresql.auto.conf? If not we can check in pg_settings, it shows sourcefile. So if the setting is from new file postgresql.auto.conf, this means it is set by ALTER SYSTEM. With Regards, Amit Kapila. -- 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] Improvement of checkpoint IO scheduler for stable transaction responses
On 2013-07-03 17:18:29 +0900, KONDO Mitsumasa wrote: Hi, I tested and changed segsize=0.25GB which is max partitioned table file size and default setting is 1GB in configure option (./configure --with-segsize=0.25). Because I thought that small segsize is good for fsync phase and background disk write in OS in checkpoint. I got significant improvements in DBT-2 result! * Performance result in DBT-2 (WH340) | NOTPM90%tileAverage Maximum -+--- original_0.7 (baseline) | 3474.62 18.348328 5.73936.977713 fsync + write| 3586.85 14.459486 4.96027.266958 fsync + write + segsize=0.25 | 3661.17 8.288164.11717.23191 Changing segsize with my checkpoint patches improved original over 50% at 90%tile and maximum response time. Hm. I wonder how much of this could be gained by doing a sync_file_range(SYNC_FILE_RANGE_WRITE) (or similar) either while doing the original checkpoint-pass through the buffers or when fsyncing the files. Presumably the smaller segsize is better because we don't completely stall the system by submitting up to 1GB of io at once. So, if we were to do it in 32MB chunks and then do a final fsync() afterwards we might get most of the benefits. 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] [9.4 CF 1] The Commitfest Slacker List
On Wed, Jul 3, 2013 at 9:21 AM, Michael Meskes mes...@postgresql.org wrote: On Tue, Jul 02, 2013 at 04:00:22PM -0400, Robert Haas wrote: make you review patches against your will. Don't take it for more than what Josh meant it as. And that was what? An attempt to prod a few more people into helping review. I can see that this pissed you off, and I'm sorry about that. But I don't think that was his intent. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review: Patch to compute Max LSN of Data Pages
-Original Message- From: Robert Haas [mailto:robertmh...@gmail.com] Sent: Wednesday, July 03, 2013 6:40 PM To: Amit Kapila Cc: Andres Freund; Josh Berkus; pgsql-hackers@postgresql.org Subject: Re: [HACKERS] Review: Patch to compute Max LSN of Data Pages On Wed, Jul 3, 2013 at 8:44 AM, Amit Kapila amit.kap...@huawei.com wrote: Why does this patch need all of this fancy path-handling logic - specifically, remove_parent_refernces() and make_absolute_path()? Surely its needs are not that different from pg_ctl or pg_resetxlog or pg_controldata. If they all need it and it's duplicated, we should fix that. Otherwise, why the special logic here? I don't think we need getLinkPath() either. The server has no trouble finding its files by just using a pathname that follows the symlink. Why do we need anything different here? The whole point of symlinks is that you can traverse them transparently, without knowing where they point. It is to handle negative scenario's like if there is any recursion in path. However if you feel this is not important, it can be removed. I'm having a hard time imagining a situation where that would be a problem. If the symlink points to itself somehow, the OS will throw an error. If your filesystem is so badly hosed that the directory structure is more fundamentally broken than the OS's circular-symlink detection code can handle, whether or not this utility works is a second-order consideration. What kind of scenario are you imagining? amit@linux:~ md test amit@linux:~ cd test amit@linux:~/test ln -s ~/test link_test amit@linux:~/test ls link_test amit@linux:~/test cd link_test amit@linux:~/test/link_test ls link_test amit@linux:~/test/link_test cd link_test amit@linux:~/test/link_test/link_test cd link_test amit@linux:~/test/link_test/link_test/link_test pwd /home/amit/test/link_test/link_test/link_test amit@linux:~/test/link_test/link_test/link_test Platform details Suse - 11.2 Kernel - 3.0.13 This is to avoid when user has given some path where db files are present. I think it would be a good idea to have a mode that prints out the max LSN found in *each data file* scanned, and then prints out the overall max found at the end. In fact, I think that should perhaps be the default mode, with -q, --quiet to disable it. Printing too many LSN for each file might fill user's screen and he might be needing only overall LSN. Should we keep -p --printall as option to print all LSN's and keep default as overall max LSN? Honestly, I have a hard time imagining the use case for that mode. This isn't a tool that people should be running regularly, and some output that lends a bit of confidence that the tool is doing the right thing seems like a good thing. Keep in mind it's likely to run for quite a while, too, and this would provide a progress indicator. I'll defer to whatever the consensus is here but my gut feeling is that if you don't want that extra output, there's a good chance you're misusing the tool. With Regards, Amit Kapila. -- 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: Patch to compute Max LSN of Data Pages
On Wed, Jul 3, 2013 at 9:51 AM, Amit Kapila amit.kap...@huawei.com wrote: amit@linux:~ md test amit@linux:~ cd test amit@linux:~/test ln -s ~/test link_test amit@linux:~/test ls link_test amit@linux:~/test cd link_test amit@linux:~/test/link_test ls link_test amit@linux:~/test/link_test cd link_test amit@linux:~/test/link_test/link_test cd link_test amit@linux:~/test/link_test/link_test/link_test pwd /home/amit/test/link_test/link_test/link_test amit@linux:~/test/link_test/link_test/link_test So what? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Support for REINDEX CONCURRENTLY
On 2013-07-03 10:03:26 +0900, Michael Paquier wrote:
+static int
+toast_open_indexes(Relation toastrel,
+LOCKMODE lock,
+Relation **toastidxs,
+int *num_indexes)
+ /*
+ * Free index list, not necessary as relations are opened and a valid
index
+ * has been found.
+ */
+ list_free(indexlist);
Missing anymore or such.
index 9ee9ea2..23e0373 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -2778,10 +2778,9 @@ binary_upgrade_set_pg_class_oids(Archive *fout,
PQExpBuffer upgrade_query = createPQExpBuffer();
PGresult *upgrade_res;
Oid pg_class_reltoastrelid;
- Oid pg_class_reltoastidxid;
appendPQExpBuffer(upgrade_query,
- SELECT c.reltoastrelid,
t.reltoastidxid
+ SELECT c.reltoastrelid
FROM pg_catalog.pg_class c LEFT JOIN
pg_catalog.pg_class t ON
(c.reltoastrelid = t.oid)
WHERE c.oid = '%u'::pg_catalog.oid;,
@@ -2790,7 +2789,6 @@ binary_upgrade_set_pg_class_oids(Archive *fout,
upgrade_res = ExecuteSqlQueryForSingleRow(fout, upgrade_query-data);
pg_class_reltoastrelid = atooid(PQgetvalue(upgrade_res, 0,
PQfnumber(upgrade_res, reltoastrelid)));
- pg_class_reltoastidxid = atooid(PQgetvalue(upgrade_res, 0,
PQfnumber(upgrade_res, reltoastidxid)));
appendPQExpBuffer(upgrade_buffer,
\n-- For binary upgrade, must preserve
pg_class oids\n);
@@ -2803,6 +2801,10 @@ binary_upgrade_set_pg_class_oids(Archive *fout,
/* only tables have toast tables, not indexes */
if (OidIsValid(pg_class_reltoastrelid))
{
+ PQExpBuffer index_query = createPQExpBuffer();
+ PGresult *index_res;
+ Oid indexrelid;
+
/*
* One complexity is that the table definition might
not require
* the creation of a TOAST table, and the TOAST table
might have
@@ -2816,10 +2818,23 @@ binary_upgrade_set_pg_class_oids(Archive *fout,
SELECT
binary_upgrade.set_next_toast_pg_class_oid('%u'::pg_catalog.oid);\n,
pg_class_reltoastrelid);
- /* every toast table has an index */
+ /* Every toast table has one valid index, so fetch it
first... */
+ appendPQExpBuffer(index_query,
+ SELECT c.indexrelid
+ FROM
pg_catalog.pg_index c
+ WHERE c.indrelid =
%u AND c.indisvalid;,
+
pg_class_reltoastrelid);
+ index_res = ExecuteSqlQueryForSingleRow(fout,
index_query-data);
+ indexrelid = atooid(PQgetvalue(index_res, 0,
+
PQfnumber(index_res, indexrelid)));
+
+ /* Then set it */
appendPQExpBuffer(upgrade_buffer,
SELECT
binary_upgrade.set_next_index_pg_class_oid('%u'::pg_catalog.oid);\n,
-
pg_class_reltoastidxid);
+ indexrelid);
+
+ PQclear(index_res);
+ destroyPQExpBuffer(index_query);
Wouldn't it make more sense to fetch the toast index oid in the query
ontop instead of making a query for every relation?
Looking good!
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] refresh materialized view concurrently
Robert Haas robertmh...@gmail.com wrote: Hitoshi Harada umi.tan...@gmail.com wrote: Other than these, I've found index is opened with NoLock, relying on ExclusiveLock of parent matview, and ALTER INDEX SET TABLESPACE or something similar can run concurrently, but it is presumably safe. DROP INDEX, REINDEX would be blocked by the ExclusiveLock. I doubt very much that this is safe. And even if it is safe today, I think it's a bad idea, because we're likely to try to reduce lock levels in the future. Taking no lock on a relation we're opening, even an index, seems certain to be a bad idea. What we're talking about is taking a look at the index definition while the indexed table involved is covered by an ExclusiveLock. Why is that more dangerous than inserting entries into an index without taking a lock on that index while the indexed table is covered by a RowExclusiveLock, as happens on INSERT? RowExclusiveLock is a much weaker lock, and we can't add entries to an index without looking at its definition. Should we be taking out locks on every index for every INSERT? If not, what makes that safe while merely looking at the definition is too risky? -- Kevin Grittner EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] dynamic background workers
On Mon, Jun 24, 2013 at 3:51 AM, Michael Paquier michael.paqu...@gmail.com wrote: 3) Why not adding an other function in worker_spi.c being the opposite of worker_spi_launch to stop dynamic bgworkers for a given index number? This would be only a wrapper of pg_terminate_backend, OK, but at least it would give the user all the information needed to start and to stop a dynamic bgworker with a single extension, here worker_spi.c. It can be painful to stop Well, there's currently no mechanism for the person who starts a new backend to know the PID of the process that actually got started. I plan to write a patch to address that problem, but it's not this patch. 4) Not completely related to this patch, but one sanity check in SanityCheckBackgroundWorker:bgworker.c is not listed in the documentation: when requesting a database connection, bgworker needs to have access to shmem. It looks that this should be also fixed in REL9_3_STABLE. That's fine; I think it's separate from this patch. Please feel free to propose something. 5) Why not adding some documentation? Both dynamic and static bgworkers share the same infrastructure, so some lines in the existing chapter might be fine? I'll take a look. 6) Just wondering something: it looks that the current code is not able to identify what was the way used to start a given bgworker. Would it be interesting to be able to identify if a bgworker has been registered though RegisterBackgroundWorker or RegisterDynamicBackgroundWorker? I don't think that's a good thing to expose. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] refresh materialized view concurrently
Kevin Grittner kgri...@ymail.com writes: Robert Haas robertmh...@gmail.com wrote: I doubt very much that this is safe. And even if it is safe today, I think it's a bad idea, because we're likely to try to reduce lock levels in the future. Taking no lock on a relation we're opening, even an index, seems certain to be a bad idea. I'm with Robert on this. What we're talking about is taking a look at the index definition while the indexed table involved is covered by an ExclusiveLock. Why is that more dangerous than inserting entries into an index without taking a lock on that index while the indexed table is covered by a RowExclusiveLock, as happens on INSERT? I don't believe that that happens. If it does, it's a bug. Either the planner or the executor should be taking a lock on each index touched by a query. 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] refresh materialized view concurrently
On Wed, Jul 3, 2013 at 10:25 AM, Tom Lane t...@sss.pgh.pa.us wrote: Kevin Grittner kgri...@ymail.com writes: Robert Haas robertmh...@gmail.com wrote: I doubt very much that this is safe. And even if it is safe today, I think it's a bad idea, because we're likely to try to reduce lock levels in the future. Taking no lock on a relation we're opening, even an index, seems certain to be a bad idea. I'm with Robert on this. What we're talking about is taking a look at the index definition while the indexed table involved is covered by an ExclusiveLock. Why is that more dangerous than inserting entries into an index without taking a lock on that index while the indexed table is covered by a RowExclusiveLock, as happens on INSERT? I don't believe that that happens. If it does, it's a bug. Either the planner or the executor should be taking a lock on each index touched by a query. It seems Kevin's right. Not sure why that doesn't break. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] New regression test time
On Wed, Jul 3, 2013 at 2:28 AM, Simon Riggs si...@2ndquadrant.com wrote: It's sad to simply reject meaningful automated tests on the basis of doubt that they're important enough to belong in every human-in-the-loop test run. I share the broader vision for automated testing represented by these patches. +1 We should be encouraging people to submit automated tests. It seems like there is now a clear consensus to proceed here, so I'll start looking at committing some of these tests. Automated testing is about x10-100 faster than manual testing, so I see new tests as saving me time not wasting it. +1. Let's have a new schedule called minute-check with the objective to run the common tests in 60 secs. We can continue to expand the normal schedules from here. Anybody that wants short tests can run that, everyone else can run the full test suite. We should be encouraging people to run the full test suite, not the fast one. I like this idea, or some variant of it (fastcheck?). -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] refresh materialized view concurrently
Robert Haas robertmh...@gmail.com writes: On Wed, Jul 3, 2013 at 10:25 AM, Tom Lane t...@sss.pgh.pa.us wrote: I don't believe that that happens. If it does, it's a bug. Either the planner or the executor should be taking a lock on each index touched by a query. It seems Kevin's right. Not sure why that doesn't break. Are we somehow not going through ExecOpenIndices? 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] refresh materialized view concurrently
On Wed, Jul 3, 2013 at 10:47 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Wed, Jul 3, 2013 at 10:25 AM, Tom Lane t...@sss.pgh.pa.us wrote: I don't believe that that happens. If it does, it's a bug. Either the planner or the executor should be taking a lock on each index touched by a query. It seems Kevin's right. Not sure why that doesn't break. Are we somehow not going through ExecOpenIndices? I dunno. I just did a quick black-box test: CREATE TABLE foo (a int primary key); BEGIN; INSERT INTO foo VALUES (1); SELECT relation::regclass, locktype, mode, granted FROM pg_locks; I get: relation | locktype| mode | granted --+---+--+- pg_locks | relation | AccessShareLock | t foo | relation | RowExclusiveLock | t | virtualxid| ExclusiveLock| t | transactionid | ExclusiveLock| t No foo_pkey anywhere. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add more regression tests for ASYNC
On Mon, May 13, 2013 at 8:37 PM, Robins Tharakan thara...@gmail.com wrote: Hi, Patch to add more regression tests to ASYNC (/src/backend/commands/async). Take code-coverage from 39% to 75%. Any and all feedback is obviously welcome. p.s.: Please let me know whether these tests are useless or would not be committed owing to unrelated reasons. As also, if these tests need to be clubbed (bundled up in 2-3) and submitted as a single submit. Committed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] refresh materialized view concurrently
Robert Haas robertmh...@gmail.com writes: On Wed, Jul 3, 2013 at 10:47 AM, Tom Lane t...@sss.pgh.pa.us wrote: Are we somehow not going through ExecOpenIndices? I dunno. I just did a quick black-box test: CREATE TABLE foo (a int primary key); BEGIN; INSERT INTO foo VALUES (1); SELECT relation::regclass, locktype, mode, granted FROM pg_locks; I get: relation | locktype| mode | granted --+---+--+- pg_locks | relation | AccessShareLock | t foo | relation | RowExclusiveLock | t | virtualxid| ExclusiveLock| t | transactionid | ExclusiveLock| t No foo_pkey anywhere. That proves nothing, as we don't keep such locks after the query (and there's no reason to AFAICS). See ExecCloseIndices. 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] (trivial patch) remove superfluous semicolons from pg_dump
I noticed an instance of 'appendPQExpBuffer(query, ;);' in pg_dump.c which seems pointless and mildly confusing. There's another seemingly useless one in pg_dumpall.c. Attached patch removes both (if that makes any sense). Regards Ian Barwick pg_dump-cull-semicolons.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] Add some regression tests for SEQUENCE
On Tue, May 7, 2013 at 6:40 PM, Robins Tharakan thara...@gmail.com wrote: Have provided an updated patch as per Fabien's recent response on Commitfest site. Any and all feedback is appreciated. I think you should rename the roles used here to regress_rol_seq1 etc. to match the CREATE OPERATOR patch. And you need to update the expected output. Setting this one to Waiting on Author. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] possible/feasible to specify field and value in error msg?
On Wed, Jul 3, 2013 at 10:54:48AM +0200, Willy-Bas Loos wrote: Hi, I have some complicated query that truncates and fills a table and i get this message: ERROR: smallint out of range STATEMENT: my huge query This is in postgres 8.4 I don't know where the error is, and the query takes rather long. So it is going to be a bit cumbersome for me to debug this. Would it be possible/feasible to specify, in future versions of postgres: * what value * which field (of which table) * the offending tuple? (possibly truncated to some threshold nr of characters) I ask because i can imagine that, inside the code that handles this, you might not have access to that information and adding access to it might be inefficient. I do get the whole query of course, and that is very handy for automated things. But in this case, it doesn't help me. We will add optional error details in Postgres 9.3: http://momjian.us/main/blogs/pgblog/2013.html#April_11_2013 I don't know if an out-of-range error would generate the column name. -- 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] dynamic background workers
Andres Freund escribió: Just as a datapoint, if you benchmark the numbers of forks that can be performed by a single process (i.e. postmaster) the number is easily in the 10s of thousands. Now forking that much has some scalability implications inside the kernel, but still. I'd be surprised if the actual fork is more than 5-10% of the current cost of starting a new backend. I played at having some thousands of registered bgworkers on my laptop, and there wasn't even that much load. So yeah, you can have lots of forks. -- Á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
Re: [HACKERS] Patch to add regression tests for SCHEMA
On Wed, May 8, 2013 at 9:19 PM, Robins Tharakan thara...@gmail.com wrote: Please find attached an updated patch with the said changes. I'll try to update the other patches (if they pertain to this feedback) and update on their respective threads (as well as on Commitfest). This patch updates the parallel schedule but not the serial schedule. Please try to remember to update both files when adding new test files. Also, please observe the admonitions in the parallel schedule, at top of file: # By convention, we put no more than twenty tests in any one parallel group; # this limits the number of connections needed to run the tests. In this particular case, I think that adding a new set of tests isn't the right thing anyway. Schemas are also known as namespaces, and the existing namespace test is where related test cases live. Add these tests there instead. Rename regression users to regress_rol_nsp1 etc. per convention established in the CREATE OPERATOR patch. Setting patch to Waiting on Author. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] possible/feasible to specify field and value in error msg?
On Wed, Jul 3, 2013 at 11:14:18AM -0400, Bruce Momjian wrote: On Wed, Jul 3, 2013 at 10:54:48AM +0200, Willy-Bas Loos wrote: Hi, I have some complicated query that truncates and fills a table and i get this message: ERROR: smallint out of range STATEMENT: my huge query This is in postgres 8.4 I don't know where the error is, and the query takes rather long. So it is going to be a bit cumbersome for me to debug this. Would it be possible/feasible to specify, in future versions of postgres: * what value * which field (of which table) * the offending tuple? (possibly truncated to some threshold nr of characters) I ask because i can imagine that, inside the code that handles this, you might not have access to that information and adding access to it might be inefficient. I do get the whole query of course, and that is very handy for automated things. But in this case, it doesn't help me. We will add optional error details in Postgres 9.3: http://momjian.us/main/blogs/pgblog/2013.html#April_11_2013 I don't know if an out-of-range error would generate the column name. I just tested this and it doesn't show the offending column name; sorry: test= CREATE TABLE test(x smallint); CREATE TABLE test= \set VERBOSITY verbose test= INSERT INTO test VALUES (1000); ERROR: 22003: smallint out of range LOCATION: i4toi2, int.c:349 -- 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] refresh materialized view concurrently
On 2013-07-03 11:08:32 -0400, Tom Lane wrote: Robert Haas robertmh...@gmail.com writes: On Wed, Jul 3, 2013 at 10:47 AM, Tom Lane t...@sss.pgh.pa.us wrote: Are we somehow not going through ExecOpenIndices? I dunno. I just did a quick black-box test: CREATE TABLE foo (a int primary key); BEGIN; INSERT INTO foo VALUES (1); SELECT relation::regclass, locktype, mode, granted FROM pg_locks; I get: relation | locktype| mode | granted --+---+--+- pg_locks | relation | AccessShareLock | t foo | relation | RowExclusiveLock | t | virtualxid| ExclusiveLock| t | transactionid | ExclusiveLock| t No foo_pkey anywhere. That proves nothing, as we don't keep such locks after the query (and there's no reason to AFAICS). See ExecCloseIndices. Should be easy enough to test by hacking LOCK TABLE to lock indexes and taking out a conflicting lock (SHARE?) in a second transaction. I wonder if we shouldn't just generally allow that, I remember relaxing that check before (when playing with CREATE/DROP CONCURRENTLY afair). 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] proposal: simple date constructor from numeric values
On 3 July 2013 21:41, Pavel Stehule pavel.steh...@gmail.com wrote: I am thinking so for these functions exists some consensus - minimally for function date(year, month, int) - I dream about this function ten years :) I am not sure about datetime: a) we use timestamp name for same thing in pg b) we can simply construct timestamp as sum of date + time, what is little bit more practical (for me), because it doesn't use too wide parameter list. I agree. I've got no issues with using date + time arithmetic to build a timestamp. what do you think about names? make_date make_time I am fine with those names. 'make', 'construct', 'build', etc. are all reasonable verbs for what the functions do, but 'make' is nice and short, and will be familiar to people who've used a 'mktime'. I don't would to use to_date, to_time functions, a) because these functions use formatted input, b) we hold some compatibility with Oracle. Yes, I agree. Cheers, BJ -- 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] Add regression tests for ROLE (USER)
On Thu, May 9, 2013 at 4:29 AM, Fabien COELHO coe...@cri.ensmp.fr wrote: This updated version works for me and addresses previous comments. I think that such tests are definitely valuable, especially as many corner cases which must trigger errors are covered. I recommend to apply it. I'm attaching an update of this patch that renames both the new test file (user-role), and the regression users that get created. It also fixes the serial schedule to match the parallel schedule. However, before it can get committed, I think this set of tests needs streamlining. It does seem to me valuable, but I think it's wasteful in terms of runtime to create so many roles, do just one thing with them, and then drop them. I recommend consolidating some of the tests. For example: +-- Should work. ALTER ROLE with (UN)ENCRYPTED PASSWORD +CREATE ROLE regress_rol_rol14; +ALTER ROLE regress_rol_rol14 WITH ENCRYPTED PASSWORD 'abc'; +DROP ROLE regress_rol_rol14; +CREATE ROLE regress_rol_rol15; +ALTER ROLE regress_rol_rol15 WITH UNENCRYPTED PASSWORD 'abc'; +DROP ROLE regress_rol_rol15; + +-- Should fail. ALTER ROLE with (UN)ENCRYPTED PASSWORD but no password value +CREATE ROLE regress_rol_rol16; +ALTER ROLE regress_rol_rol16 WITH ENCRYPTED PASSWORD; +DROP ROLE regress_rol_rol16; +CREATE ROLE regress_rol_rol17; +ALTER ROLE regress_rol_rol17 WITH UNENCRYPTED PASSWORD; +DROP ROLE regress_rol_rol17; + +-- Should fail. ALTER ROLE with both UNENCRYPTED and ENCRYPTED +CREATE ROLE regress_rol_rol18; +ALTER ROLE regress_rol_rol18 WITH ENCRYPTED UNENCRYPTED PASSWORD 'abc'; +DROP ROLE regress_rol_rol18; There's no reason that couldn't be written this way: CREATE ROLE regress_rol_rol14; ALTER ROLE regress_rol_rol14 WITH ENCRYPTED PASSWORD 'abc'; ALTER ROLE regress_rol_rol14 WITH UNENCRYPTED PASSWORD 'abc'; ALTER ROLE regress_rol_rol14 WITH ENCRYPTED PASSWORD; ALTER ROLE regress_rol_rol14 WITH UNENCRYPTED PASSWORD; ALTER ROLE regress_rol_rol14 WITH ENCRYPTED UNENCRYPTED PASSWORD 'abc'; DROP ROLE regress_rol_rol14; Considering the concerns already expressed about the runtime of the test, I think it's important to minimize the number of create/drop role cycles that the tests perform. Generally, I think that the tests which return a syntax error are of limited value and should probably be dropped. That is unlikely to get broken by accident. If the syntax error is being thrown by something outside of bison proper, that's probably worth testing. But I think that testing random syntax variations is a pretty low-value proposition. Setting this to Waiting on Author. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company regress_role_v3.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] proposal: simple date constructor from numeric values
Peter Eisentraut escribió: On 7/1/13 3:47 AM, Pavel Stehule wrote: CREATE OR REPLACE FUNCTION construct_time(hour int DEFAULT 0, mi int DEFAULT 0, sec int DEFAULT 0, ms float DEFAULT 0.0); If we are using integer datetime storage, we shouldn't use floats to construct them. I think this is wrong. Datetime storage may be int, but since they're microseconds underneath, we'd be unable to specify a full-resolution timestamp if we didn't have float ms or integer µs. So either the seconds argument should allow fractions (probably not a good idea), or we should have another integer argument for microseconds (not milliseconds as the above signature implies). -- Á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
Re: [HACKERS] proposal: simple date constructor from numeric values
2013/7/3 Alvaro Herrera alvhe...@2ndquadrant.com: Peter Eisentraut escribió: On 7/1/13 3:47 AM, Pavel Stehule wrote: CREATE OR REPLACE FUNCTION construct_time(hour int DEFAULT 0, mi int DEFAULT 0, sec int DEFAULT 0, ms float DEFAULT 0.0); If we are using integer datetime storage, we shouldn't use floats to construct them. I think this is wrong. Datetime storage may be int, but since they're microseconds underneath, we'd be unable to specify a full-resolution timestamp if we didn't have float ms or integer µs. So either the seconds argument should allow fractions (probably not a good idea), or we should have another integer argument for microseconds (not milliseconds as the above signature implies). so make_time(hour int, mi int, sec int, usec int DEFAULT 0) Is good for all ? Regards Pavel -- Á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
Re: [HACKERS] Add regression tests for COLLATE
On Thu, May 9, 2013 at 9:27 PM, Robins Tharakan thara...@gmail.com wrote: One workaround is to use DROP COLLATION IF EXISTS, that conveys the message without UTF8 in the message string. But three other tests use ALTER COLLATION and I see no way but to comment / disable them. Currently have them disabled (with comments as to what they do, and why they're disabled). This updated patch doesn't have UTF8 string anywhere. Let me know if you prefer to remove the commented out tests completely. I did remove those, plus made some other cleanups, and committed this. I think that there's now some duplication between this and the collate.linux.utf8 test that should be cleaned up by removing the duplicative stuff from that test, assuming this holds up OK when the buildfarm - and other developers - test it out. Could you prepare a patch towards that end? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] proposal: simple date constructor from numeric values
Alvaro Herrera alvhe...@2ndquadrant.com writes: Peter Eisentraut escribió: On 7/1/13 3:47 AM, Pavel Stehule wrote: CREATE OR REPLACE FUNCTION construct_time(hour int DEFAULT 0, mi int DEFAULT 0, sec int DEFAULT 0, ms float DEFAULT 0.0); If we are using integer datetime storage, we shouldn't use floats to construct them. I think this is wrong. Datetime storage may be int, but since they're microseconds underneath, we'd be unable to specify a full-resolution timestamp if we didn't have float ms or integer µs. So either the seconds argument should allow fractions (probably not a good idea), or we should have another integer argument for microseconds (not milliseconds as the above signature implies). FWIW, I'd vote for allowing the seconds to be fractional. That's the way the user perceives things: regression=# select '12:34:56.789'::time; time -- 12:34:56.789 (1 row) Moreover, an integer microseconds argument would be a shortsighted idea because it wires the precision limit into the function API. As long as we make the seconds argument be float8, it will work fine even when the underlying precision switches to, say, nanoseconds. And lastly, those default arguments are a bad idea as well. There's no reasonable use-case for make_time(12); that's almost certainly an error. Even more so for make_time(). While you could make some case for make_time(12,34) being useful, I don't think it buys much compared to writing out make_time(12,34,0), and having just one function signature is that much less cognitive load on users. So my vote is for make_time(hour int, min int, sec float8). 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] preserving forensic information when we freeze
On 2013-07-02 16:29:56 -0400, Robert Haas wrote: On Mon, Jun 24, 2013 at 8:12 AM, Andres Freund and...@2ndquadrant.com wrote: Agreed. And it also improves the status quo when debugging. I wish this would have been the representation since the beginning. Some remarks: * I don't really like that HeapTupleHeaderXminFrozen() now is frequently performed without checking for FrozenTransactionId. I think the places where that's done are currently safe, but it seems likely that we introduce bugs due to people writing similar code. I think replacing *GetXmin by a wrapper that returns FrozenTransactionId if the hint bit tell us so would be a good idea. Then add *GetRawXmin for the rest (probably mostly display). Seems like it would make the patch actually smaller. I did think about this approach, but it seemed like it would add cycles in a significant number of places. For example, consider the HeapTupleSatisfies() routines, which are perhaps PostgreSQL's finest example of a place where you DON'T want to add any cycles. All of the checks on xmin are conditional on HeapTupleHeaderXminCommitted() having been found already to be false. That implies that the frozen bits aren't set either, so if HeapTupleHeaderGetXmin() were to recheck the bits it would be a waste. As I got to the end of the patch I was a little dismayed by the number of places that did need adjustment to check both things, but there are still plenty of important places that don't. Well, nothing would prevent using the HeapTupleHeaderGetRawXmin() in those places. Exactly the number of callsites is what makes me think that somebody will get this wrong in the future. * The PLs imo should set fn_xmin to FrozenTransactionId if the hint bit tell us the tuple is frozen. Why? I thought about that, but it seemed to me that the purpose of that caching was to avoid confusing two functions whose pg_proc tuples ended up at the same TID. [good reasoning] Man. I hate this hack. I wonder what happens if somebody does a VACUUM FULL of pg_class and there are a bunch of functions created in the same transaction... * I think rewrite_heap_dead_tuple needs to check for a frozen xmin and store that. We might looking at a chain which partially was done in 9.4. Not sure if that's a realistic scenario, but I'd rather be safe. IIUC, you're talking about the scenario where we have an update chain X - Y, where X is dead but not actually removed and Y is (forensically) frozen. We're examining tuple Y and trying to determine whether X has been entered in rs_unresolved_tups. If, as I think you're proposing, we consider the xmin of Y to be FrozenTransactionId, we will definitely not find it - because the way it got into the table in the first place was based on the value returned by HeapTupleHeaderGetUpdateXid(). And that value is certain not to be FrozenTransactionId, because we never set the xmax of a tuple to FrozenTransactionId. I am thinking of something slightly different. rewrite_heap_dead_tuple() now removes tuples/xids from the unresolved table that could be from a different xid epoch since it unconditionally does a HASH_REMOVE if it finds an entry doing a lookup using the *preserved* xid. Earlier that was harmless since for frozen tuples it only ever used FrozenTransactionId which obviously cannot be part of a valid chain and couldn't even get entered into unresolved_tups. I am not sure at all if that actually can be harmful but there isn't any reason we would need to do the delete so I wouldn't. There can be complex enough situation where later parts of a ctid chain are dead and earlier ones are recently dead and such that I would rather be cautious. There's no possibility of getting confused here; if X is still around at all, it's xmax is of the same generation as Y's xmin. Otherwise, we've had an undetected XID wraparound. Another issue I thought about is what we will return for SELECT xmin FROM blarg; Some people use that in their applications (IIRC skytools/pqg/londiste does so) and they might get confused if we suddently return xids from the future. On the other hand, not returning the original xid would be a shame as well... 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] proposal: simple date constructor from numeric values
2013/7/3 Tom Lane t...@sss.pgh.pa.us: Alvaro Herrera alvhe...@2ndquadrant.com writes: Peter Eisentraut escribió: On 7/1/13 3:47 AM, Pavel Stehule wrote: CREATE OR REPLACE FUNCTION construct_time(hour int DEFAULT 0, mi int DEFAULT 0, sec int DEFAULT 0, ms float DEFAULT 0.0); If we are using integer datetime storage, we shouldn't use floats to construct them. I think this is wrong. Datetime storage may be int, but since they're microseconds underneath, we'd be unable to specify a full-resolution timestamp if we didn't have float ms or integer ľs. So either the seconds argument should allow fractions (probably not a good idea), or we should have another integer argument for microseconds (not milliseconds as the above signature implies). FWIW, I'd vote for allowing the seconds to be fractional. That's the way the user perceives things: regression=# select '12:34:56.789'::time; time -- 12:34:56.789 (1 row) Moreover, an integer microseconds argument would be a shortsighted idea because it wires the precision limit into the function API. As long as we make the seconds argument be float8, it will work fine even when the underlying precision switches to, say, nanoseconds. And lastly, those default arguments are a bad idea as well. There's no reasonable use-case for make_time(12); that's almost certainly an error. Even more so for make_time(). While you could make some case for make_time(12,34) being useful, I don't think it buys much compared to writing out make_time(12,34,0), and having just one function signature is that much less cognitive load on users. I had a plan use DEFAULT only for usec parameter (if it was used). Seconds was mandatory parameter. After tests on prototype I think so fractional sec is better. Separate value (in usec) is really big number - not practical for hand writing So my vote is for make_time(hour int, min int, sec float8). +1 Pavel 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] Add regression tests for COLLATE
Robert Haas robertmh...@gmail.com writes: I did remove those, plus made some other cleanups, and committed this. I think that there's now some duplication between this and the collate.linux.utf8 test that should be cleaned up by removing the duplicative stuff from that test, assuming this holds up OK when the buildfarm - and other developers - test it out. I don't even need to test it: regression=# CREATE COLLATION collate_coll2 FROM C; ERROR: nondefault collations are not supported on this platform Please revert. 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] Add regression tests for COLLATE
On Wed, Jul 3, 2013 at 1:11 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: I did remove those, plus made some other cleanups, and committed this. I think that there's now some duplication between this and the collate.linux.utf8 test that should be cleaned up by removing the duplicative stuff from that test, assuming this holds up OK when the buildfarm - and other developers - test it out. I don't even need to test it: regression=# CREATE COLLATION collate_coll2 FROM C; ERROR: nondefault collations are not supported on this platform Please revert. OK, sure, but just for my education and general enlightenment ... what platform is that? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add regression tests for COLLATE
On Wed, Jul 3, 2013 at 1:26 PM, Robert Haas robertmh...@gmail.com wrote: Please revert. OK, sure, but just for my education and general enlightenment ... what platform is that? Ah, never mind. I see that the buildfarm is quickly turning red - NetBSD, OmniOS, and HP/UX are all unhappy. I think that's a killer blow for this particular patch. I'm going to set this to rejected in the CF app. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add regression tests for COLLATE
On 2013-07-03 13:29:18 -0400, Robert Haas wrote: On Wed, Jul 3, 2013 at 1:26 PM, Robert Haas robertmh...@gmail.com wrote: Please revert. OK, sure, but just for my education and general enlightenment ... what platform is that? Ah, never mind. I see that the buildfarm is quickly turning red - NetBSD, OmniOS, and HP/UX are all unhappy. I think that's a killer blow for this particular patch. I'm going to set this to rejected in the CF app. Can't we use a alternative expected file for those? 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] New regression test time
On 3 July 2013 15:43, Robert Haas robertmh...@gmail.com wrote: Let's have a new schedule called minute-check with the objective to run the common tests in 60 secs. We can continue to expand the normal schedules from here. Anybody that wants short tests can run that, everyone else can run the full test suite. We should be encouraging people to run the full test suite, not the fast one. I like this idea, or some variant of it (fastcheck?). I thought about that, but then people will spend time trying to tune it. If its called minute-check and it runs in 60 secs then they'll leave it alone... -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services
Re: [HACKERS] Support for REINDEX CONCURRENTLY
On Wed, Jul 3, 2013 at 11:16 PM, Andres Freund and...@2ndquadrant.com wrote:
On 2013-07-03 10:03:26 +0900, Michael Paquier wrote:
index 9ee9ea2..23e0373 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -2778,10 +2778,9 @@ binary_upgrade_set_pg_class_oids(Archive *fout,
PQExpBuffer upgrade_query = createPQExpBuffer();
PGresult *upgrade_res;
Oid pg_class_reltoastrelid;
- Oid pg_class_reltoastidxid;
appendPQExpBuffer(upgrade_query,
- SELECT c.reltoastrelid,
t.reltoastidxid
+ SELECT c.reltoastrelid
FROM pg_catalog.pg_class c LEFT
JOIN
pg_catalog.pg_class t ON
(c.reltoastrelid = t.oid)
WHERE c.oid =
'%u'::pg_catalog.oid;,
@@ -2790,7 +2789,6 @@ binary_upgrade_set_pg_class_oids(Archive *fout,
upgrade_res = ExecuteSqlQueryForSingleRow(fout, upgrade_query-data);
pg_class_reltoastrelid = atooid(PQgetvalue(upgrade_res, 0,
PQfnumber(upgrade_res, reltoastrelid)));
- pg_class_reltoastidxid = atooid(PQgetvalue(upgrade_res, 0,
PQfnumber(upgrade_res, reltoastidxid)));
appendPQExpBuffer(upgrade_buffer,
\n-- For binary upgrade, must preserve
pg_class oids\n);
@@ -2803,6 +2801,10 @@ binary_upgrade_set_pg_class_oids(Archive *fout,
/* only tables have toast tables, not indexes */
if (OidIsValid(pg_class_reltoastrelid))
{
+ PQExpBuffer index_query = createPQExpBuffer();
+ PGresult *index_res;
+ Oid indexrelid;
+
/*
* One complexity is that the table definition might
not require
* the creation of a TOAST table, and the TOAST table
might have
@@ -2816,10 +2818,23 @@ binary_upgrade_set_pg_class_oids(Archive *fout,
SELECT
binary_upgrade.set_next_toast_pg_class_oid('%u'::pg_catalog.oid);\n,
pg_class_reltoastrelid);
- /* every toast table has an index */
+ /* Every toast table has one valid index, so fetch it
first... */
+ appendPQExpBuffer(index_query,
+ SELECT c.indexrelid
+ FROM
pg_catalog.pg_index c
+ WHERE c.indrelid =
%u AND c.indisvalid;,
+
pg_class_reltoastrelid);
+ index_res = ExecuteSqlQueryForSingleRow(fout,
index_query-data);
+ indexrelid = atooid(PQgetvalue(index_res, 0,
+
PQfnumber(index_res, indexrelid)));
+
+ /* Then set it */
appendPQExpBuffer(upgrade_buffer,
SELECT
binary_upgrade.set_next_index_pg_class_oid('%u'::pg_catalog.oid);\n,
-
pg_class_reltoastidxid);
+ indexrelid);
+
+ PQclear(index_res);
+ destroyPQExpBuffer(index_query);
Wouldn't it make more sense to fetch the toast index oid in the query
ontop instead of making a query for every relation?
With something like a CASE condition in the upper query for
reltoastrelid? This code path is not only taken by indexes but also by
tables. So I thought that it was cleaner and more readable to fetch
the index OID only if necessary as a separate query.
Regards,
--
Michael
--
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] Support for REINDEX CONCURRENTLY
On 2013-07-04 02:32:32 +0900, Michael Paquier wrote: Wouldn't it make more sense to fetch the toast index oid in the query ontop instead of making a query for every relation? With something like a CASE condition in the upper query for reltoastrelid? This code path is not only taken by indexes but also by tables. So I thought that it was cleaner and more readable to fetch the index OID only if necessary as a separate query. A left join should do the trick? 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] Add regression tests for COLLATE
Andres Freund and...@2ndquadrant.com writes: On 2013-07-03 13:29:18 -0400, Robert Haas wrote: I think that's a killer blow for this particular patch. I'm going to set this to rejected in the CF app. Can't we use a alternative expected file for those? Alternative expected files are a PITA to maintain, at least for committers who don't have a platform that can reproduce the alternative behavior. If this test were of somewhat higher value I'd be in favor of fixing it that way, but given that it's been seriously constrained by the portability issues that were already considered, I'm not sure it's worth our trouble. (There's also no very strong reason to believe that we found out all the remaining portability issues. Maybe we should have left it in there for a day, just to see if the buildfarm would show any other symptoms besides this one.) 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] proposal: simple date constructor from numeric values
Hello So my vote is for make_time(hour int, min int, sec float8). so here is a patch Regards Pavel regards, tom lane make_date.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] Add regression tests for COLLATE
On Wed, Jul 3, 2013 at 1:38 PM, Tom Lane t...@sss.pgh.pa.us wrote: Andres Freund and...@2ndquadrant.com writes: On 2013-07-03 13:29:18 -0400, Robert Haas wrote: I think that's a killer blow for this particular patch. I'm going to set this to rejected in the CF app. Can't we use a alternative expected file for those? Alternative expected files are a PITA to maintain, at least for committers who don't have a platform that can reproduce the alternative behavior. If this test were of somewhat higher value I'd be in favor of fixing it that way, but given that it's been seriously constrained by the portability issues that were already considered, I'm not sure it's worth our trouble. (There's also no very strong reason to believe that we found out all the remaining portability issues. Maybe we should have left it in there for a day, just to see if the buildfarm would show any other symptoms besides this one.) I agree. I think it'd be a good idea to get the buildfarm to run the existing collate.utf8.linux test regularly on platforms where it passes, but this particular approach is valuable mostly because (supposedly) it was going to work everywhere. However, it doesn't. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [9.4 CF 1] The Commitfest Slacker List
Clearly I ticked off a bunch of people by publishing the list. On the other hand, in the 5 days succeeding the post, more than a dozen additional people signed up to review patches, and we got some of the ready for committer patches cleared out -- something which nothing else I did, including dozens of private emails, general pleas to this mailing list, mails to the RRReviewers list, served to accomplish, in this or previous CFs. Others rules appeared, like the 5 days limit. To me it outlines that some are abusing the CF app and pushing there useless patches (not still ready or complete, WIP, ...) So, as an experiment, call it a mixed result. I would like to have some other way to motivate reviewers than public shame. I'd like to have some positive motivations for reviewers, such as public recognition by our project and respect from hackers, but I'm doubting that those are actually going to happen, given the feedback I've gotten on this list to the idea. You're looking at a short term, big effect. And long term ? Will people listed still be interested to participate in a project which stamps people ? With or without review, it's a shame if people stop proposing patches because they are not sure to get time to review other things *in time*. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Support for REINDEX CONCURRENTLY
On Thu, Jul 4, 2013 at 2:41 AM, Fujii Masao masao.fu...@gmail.com wrote: On Thu, Jul 4, 2013 at 2:36 AM, Andres Freund and...@2ndquadrant.com wrote: On 2013-07-04 02:32:32 +0900, Michael Paquier wrote: Wouldn't it make more sense to fetch the toast index oid in the query ontop instead of making a query for every relation? +1 I changed the query that way. Updated version of the patch attached. Also I updated the rules.out because Michael changed the system_views.sql. Otherwise, the regression test would fail. Will commit this patch. Committed. So, let's get to REINDEX CONCURRENTLY patch! Regards, -- Fujii Masao -- 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] Redesigning checkpoint_segments
On 6/6/13 4:09 PM, Heikki Linnakangas wrote: On 06.06.2013 20:24, Josh Berkus wrote: Yeah, something like that :-). I was thinking of letting the estimate decrease like a moving average, but react to any increases immediately. Same thing we do in bgwriter to track buffer allocations: Seems reasonable. Here's a patch implementing that. Docs not updated yet. I did not change the way checkpoint_segments triggers checkpoints - that'll can be a separate patch. This only decouples the segment preallocation behavior from checkpoint_segments. With the patch, you can set checkpoint_segments really high, without consuming that much disk space all the time. I don't understand what this patch, by itself, will accomplish in terms of the originally stated goals of making checkpoint_segments easier to tune, and controlling disk space used. To some degree, it makes both of these things worse, because you can no longer use checkpoint_segments to control the disk space. Instead, it is replaced by magic. What sort of behavior are you expecting to come out of this? In testing, I didn't see much of a difference. Although I'd expect that this would actually preallocate fewer segments than the old formula. Two small issues in the code: Code change doesn't match comment: +* +* XXX: We don't have a good estimate of how many WAL files we should keep +* preallocated here. Quite arbitrarily, use max_advance=5. That's good +* enough for current use of this function; this only gets called when +* there are no more preallocated WAL segments available. */ installed_segno = logsegno; - max_advance = XLOGfileslop; + max_advance = CheckPointSegments; KB should be kB. -- 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] LATERAL quals revisited
I wrote:
So attached is a draft patch for this. It's not complete yet because
there are various comments that are now wrong and need to be updated;
but I think the code is functioning correctly.
Hm, spoke too soon :-(. This query causes an assertion failure, with or
without my draft patch:
select c.*,a.*,ss1.q1,ss2.q1,ss3.* from
int8_tbl c left join (
int8_tbl a left join
(select q1, coalesce(q2,f1) as x from int8_tbl b, int4_tbl b2) ss1
on a.q2 = ss1.q1
cross join
lateral (select q1, coalesce(ss1.x,q2) as y from int8_tbl d) ss2
) on c.q2 = ss2.q1,
lateral (select * from int4_tbl i where ss2.y f1) ss3;
TRAP: FailedAssertion(!(bms_is_subset(phinfo-ph_needed,
phinfo-ph_may_need)), File: initsplan.c, Line: 213)
What's happening is that distribute_qual_to_rels concludes (correctly)
that the ss2.y f1 clause must be postponed until after the nest of
left joins, since those could null ss2.y. So the PlaceHolderVar for
ss2.y is marked as being needed at the topmost join level. However,
find_placeholders_in_jointree had only marked the PHV as being maybe
needed to scan the i relation, since that's what the syntactic
location of the reference implies. Since we depend on the assumption
that ph_needed is always a subset of ph_may_need, there's an assertion
that fires if that stops being true, and that's what's crashing.
After some thought about this, I'm coming to the conclusion that lateral
references destroy the ph_maybe_needed optimization altogether: we
cannot derive an accurate estimate of where a placeholder will end up in
the final qual distribution, short of essentially doing all the work in
deconstruct_jointree over again. I guess in principle we could repeat
deconstruct_jointree until we had stable estimates of the ph_needed
locations, but that would be expensive and probably would induce a lot
of new planner bugs (since the data structure changes performed during
deconstruct_jointree aren't designed to be backed out easily).
The only place where ph_may_need is actually used is in this bit in
make_outerjoininfo():
/*
* Examine PlaceHolderVars. If a PHV is supposed to be evaluated within
* this join's nullable side, and it may get used above this join, then
* ensure that min_righthand contains the full eval_at set of the PHV.
* This ensures that the PHV actually can be evaluated within the RHS.
* Note that this works only because we should already have determined the
* final eval_at level for any PHV syntactically within this join.
*/
foreach(l, root-placeholder_list)
{
PlaceHolderInfo *phinfo = (PlaceHolderInfo *) lfirst(l);
Relidsph_syn_level = phinfo-ph_var-phrels;
/* Ignore placeholder if it didn't syntactically come from RHS */
if (!bms_is_subset(ph_syn_level, right_rels))
continue;
/* We can also ignore it if it's certainly not used above this join */
/* XXX this test is probably overly conservative */
if (bms_is_subset(phinfo-ph_may_need, min_righthand))
continue;
/* Else, prevent join from being formed before we eval the PHV */
min_righthand = bms_add_members(min_righthand, phinfo-ph_eval_at);
}
Looking at it again, it's not really clear that skipping placeholders in
this way results in very much optimization --- sometimes we can avoid
constraining join order, but how often? I tried diking out the check
on ph_may_need from this loop, and saw no changes in the regression test
results (not that that proves a whole lot about optimization of complex
queries). So I'm pretty tempted to just remove ph_may_need, along with
the machinery that computes it.
Another possibility would be to keep the optimization, but disable it in
queries that use LATERAL. I don't much care for that though --- seems
too Rube Goldbergish, and in any case I have a lot less faith in the
whole concept now than I had before I started digging into this issue.
Thoughts?
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] Support for REINDEX CONCURRENTLY
On Thu, Jul 4, 2013 at 3:26 AM, Fujii Masao masao.fu...@gmail.com wrote: On Thu, Jul 4, 2013 at 2:41 AM, Fujii Masao masao.fu...@gmail.com wrote: On Thu, Jul 4, 2013 at 2:36 AM, Andres Freund and...@2ndquadrant.com wrote: On 2013-07-04 02:32:32 +0900, Michael Paquier wrote: Wouldn't it make more sense to fetch the toast index oid in the query ontop instead of making a query for every relation? +1 I changed the query that way. Updated version of the patch attached. Also I updated the rules.out because Michael changed the system_views.sql. Otherwise, the regression test would fail. Will commit this patch. Committed. So, let's get to REINDEX CONCURRENTLY patch! Thanks for the hard work! I'll work on something based on MVCC catalogs, so at least lock will be lowered at swap phase and isolation tests will be added. -- Michael -- 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] New regression test time
On 07/03/2013 07:43 AM, Robert Haas wrote: Let's have a new schedule called minute-check with the objective to run the common tests in 60 secs. Note that we're below 60s even with assert and CLOBBER_CACHE_ALWAYS, at least on my laptop. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.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] refresh materialized view concurrently
Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Tom Lane t...@sss.pgh.pa.us wrote: Are we somehow not going through ExecOpenIndices? I dunno. I just did a quick black-box test: [ begin; insert; without commit ] No foo_pkey anywhere. That proves nothing, as we don't keep such locks after the query (and there's no reason to AFAICS). See ExecCloseIndices. OK. I had seen that no locks were held after the insert and wasn't aware that we acquired and then released them for each insert within a transaction. On the other hand, we acquire locks on all indexes even for a HOT UPDATE which uses a seqscan, and hold those until end of transaction. Is there a reason for that? I suppose that since a concurrent refresh is very likely to lock all these indexes with RowExclusiveLock anyway, as a result of the DELETE, UPDATE and INSERT statements subsequently issued through SPI, I might was well acquire that lock when I look at the definition, and not release it -- so that the subsequent locks are local to the backend, and therefore faster. -- Kevin Grittner EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix pgstattuple/pgstatindex to use regclass-type as the argument
On Wed, Jun 26, 2013 at 12:39 AM, Robert Haas robertmh...@gmail.com wrote:
On Thu, Jun 20, 2013 at 2:32 PM, Fujii Masao masao.fu...@gmail.com wrote:
On Thu, Jun 20, 2013 at 11:43 AM, Satoshi Nagayasu sn...@uptime.jp wrote:
(2013/06/17 4:02), Fujii Masao wrote:
On Sat, Mar 9, 2013 at 3:23 PM, Satoshi Nagayasu sn...@uptime.jp wrote:
It is obviously easy to keep two types of function interfaces,
one with regclass-type and another with text-type, in the next
release for backward-compatibility like below:
pgstattuple(regclass) -- safer interface.
pgstattuple(text) -- will be depreciated in the future release.
So you're thinking to remove pgstattuple(oid) soon?
AFAIK, a regclass type argument would accept an OID value,
which means regclass type has upper-compatibility against
oid type.
So, even if the declaration is changed, compatibility could
be kept actually. This test case (in sql/pgstattuple.sql)
confirms that.
select * from pgstatindex('myschema.test_pkey'::regclass::oid);
version | tree_level | index_size | root_block_no | internal_pages |
leaf_pages | empty_pages | deleted_pages | avg_leaf_density |
leaf_fragmentation
-+++---+++-+---+--+
2 | 0 | 8192 | 1 | 0 |
1 | 0 | 0 | 0.79 |0
(1 row)
Having both interfaces for a while would allow users to have enough
time to rewrite their applications.
Then, we will be able to obsolete (or just drop) old interfaces
in the future release, maybe 9.4 or 9.5. I think this approach
would minimize an impact of such interface change.
So, I think we can clean up function arguments in the pgstattuple
module, and also we can have two interfaces, both regclass and text,
for the next release.
Any comments?
In the document, you should mark old functions as deprecated.
I'm still considering changing the function name as Tom pointed
out. How about pgstatbtindex?
Or just adding pgstatindex(regclass)?
In fact, pgstatindex does support only BTree index.
So, pgstatbtindex seems to be more appropriate for this function.
Can most ordinary users realize bt means btree?
We can keep having both (old) pgstatindex and (new) pgstatbtindex
during next 2-3 major releases, and the old one will be deprecated
after that.
Since pg_relpages(oid) doesn't exist, pg_relpages() is in the same
situation as pgstatindex(), i.e., we cannot just replace pg_relpages(text)
with pg_relpages(regclass) for the backward-compatibility. How do you
think we should solve the pg_relpages() problem? Rename? Just
add pg_relpages(regclass)?
Adding a function with a new name seems likely to be smoother, since
that way you don't have to worry about problems with function calls
being thought ambiguous.
Could you let me know the example that this problem happens?
For the test, I just implemented the regclass-version of pg_relpages()
(patch attached) and tested some cases. But I could not get that problem.
SELECT pg_relpages('hoge');-- OK
SELECT pg_relpages(oid) FROM pg_class WHERE relname = 'hoge';-- OK
SELECT pg_relpages(relname) FROM pg_class WHERE relname = 'hoge';-- OK
Regards,
--
Fujii Masao
pg_relpages_test.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] Add regression tests for COLLATE
Robert Haas escribió: I agree. I think it'd be a good idea to get the buildfarm to run the existing collate.utf8.linux test regularly on platforms where it passes, +1 -- Á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
Re: [HACKERS] preserving forensic information when we freeze
On Wed, Jul 3, 2013 at 1:07 PM, Andres Freund and...@2ndquadrant.com wrote: Well, nothing would prevent using the HeapTupleHeaderGetRawXmin() in those places. Exactly the number of callsites is what makes me think that somebody will get this wrong in the future. Well, I guess I could go rework the whole patch that way. It's a fair request, but I kinda doubt it's going to make the patch smaller. * I think rewrite_heap_dead_tuple needs to check for a frozen xmin and store that. We might looking at a chain which partially was done in 9.4. Not sure if that's a realistic scenario, but I'd rather be safe. IIUC, you're talking about the scenario where we have an update chain X - Y, where X is dead but not actually removed and Y is (forensically) frozen. We're examining tuple Y and trying to determine whether X has been entered in rs_unresolved_tups. If, as I think you're proposing, we consider the xmin of Y to be FrozenTransactionId, we will definitely not find it - because the way it got into the table in the first place was based on the value returned by HeapTupleHeaderGetUpdateXid(). And that value is certain not to be FrozenTransactionId, because we never set the xmax of a tuple to FrozenTransactionId. I am thinking of something slightly different. rewrite_heap_dead_tuple() now removes tuples/xids from the unresolved table that could be from a different xid epoch since it unconditionally does a HASH_REMOVE if it finds an entry doing a lookup using the *preserved* xid. Earlier that was harmless since for frozen tuples it only ever used FrozenTransactionId which obviously cannot be part of a valid chain and couldn't even get entered into unresolved_tups. I am not sure at all if that actually can be harmful but there isn't any reason we would need to do the delete so I wouldn't. There can be complex enough situation where later parts of a ctid chain are dead and earlier ones are recently dead and such that I would rather be cautious. OK, I think I see your point, and I think you're right. There's no possibility of getting confused here; if X is still around at all, it's xmax is of the same generation as Y's xmin. Otherwise, we've had an undetected XID wraparound. Another issue I thought about is what we will return for SELECT xmin FROM blarg; Some people use that in their applications (IIRC skytools/pqg/londiste does so) and they might get confused if we suddently return xids from the future. On the other hand, not returning the original xid would be a shame as well... Yeah. I think the system columns that we have today are pretty much crap. I wonder if we couldn't throw them out and replace them with some kind of functions that you can pass a row to. That would allow us to expose a lot more detail without adding a bajillion hidden columns, and for a bonus we'd save substantially on catalog bloat. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [9.4 CF 1] The Commitfest Slacker List
On Wed, Jul 3, 2013 at 2:24 PM, Cédric Villemain ced...@2ndquadrant.comwrote: Clearly I ticked off a bunch of people by publishing the list. On the other hand, in the 5 days succeeding the post, more than a dozen additional people signed up to review patches, and we got some of the ready for committer patches cleared out -- something which nothing else I did, including dozens of private emails, general pleas to this mailing list, mails to the RRReviewers list, served to accomplish, in this or previous CFs. Others rules appeared, like the 5 days limit. To me it outlines that some are abusing the CF app and pushing there useless patches (not still ready or complete, WIP, ... Seems to me that useless overstates things, but it does seem fair to say that some patches are not sufficiently well prepared to be efficiently added into Postgres. So, as an experiment, call it a mixed result. I would like to have some other way to motivate reviewers than public shame. I'd like to have some positive motivations for reviewers, such as public recognition by our project and respect from hackers, but I'm doubting that those are actually going to happen, given the feedback I've gotten on this list to the idea. You're looking at a short term, big effect. And long term ? Will people listed still be interested to participate in a project which stamps people ? With or without review, it's a shame if people stop proposing patches because they are not sure to get time to review other things *in time*. Well, if the project is hampered by not being able to get *all* the changes that people imagine that they want to put in, then we have a real problem of needing a sort of triage to determine which changes will be accepted, and which will not. Perhaps we need an extra status in the CommitFest application, namely one that characterizes: Insufficiently Important To Warrant Review That's too long a term. Perhaps Not Review-worthy expresses it better? -- When confronted by a difficult problem, solve it by reducing it to the question, How would the Lone Ranger handle this?
[HACKERS] Re: [COMMITTERS] pgsql: Revert Hopefully-portable regression tests for CREATE/ALTER/DRO
The buildfarm is sad. Do you have a pointer about the issue? Is it that builds are performed in some particular locale? -- Fabien. -- 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] big test separation POC
Here is a v2 which is more likely to work under VPATH. Here is a POC v4 which relies on multiple --schedule instead of creating concatenated schedule files. -- Fabien.diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile index d5935b6..8a39f7d 100644 --- a/src/test/regress/GNUmakefile +++ b/src/test/regress/GNUmakefile @@ -86,7 +86,7 @@ regress_data_files = \ $(wildcard $(srcdir)/output/*.source) \ $(filter-out $(addprefix $(srcdir)/,$(input_files)),$(wildcard $(srcdir)/sql/*.sql)) \ $(wildcard $(srcdir)/data/*.data) \ - $(srcdir)/parallel_schedule $(srcdir)/serial_schedule $(srcdir)/resultmap + $(srcdir)/parallel_schedule $(srcdir)/parallel_big_schedule $(srcdir)/resultmap install-tests: all install install-lib installdirs-tests $(MAKE) -C $(top_builddir)/contrib/spi install @@ -137,19 +137,43 @@ tablespace-setup: ## Run tests ## +# installcheck vs check: +# - whether test is run against installed or compiled version +# test schedules: parallel, parallel_big, standby +# serial schedules can be derived from parallel schedules + +derived_schedules = serial_schedule serial_big_schedule + +serial_%: parallel_% + echo # this file is generated automatically, do not edit! $@ + egrep '^(test|ignore):' $ | \ + while read op list ; do \ + for test in $$list ; do \ + echo $$op $$test ; \ + done ; \ + done $@ + REGRESS_OPTS = --dlpath=. $(EXTRA_REGRESS_OPTS) +# before installation, parallel check: all tablespace-setup - $(pg_regress_check) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(TEMP_CONF) $(EXTRA_TESTS) + $(pg_regress_check) $(REGRESS_OPTS) $(MAXCONNOPT) $(TEMP_CONF) \ + --schedule=$(srcdir)/parallel_schedule $(EXTRA_TESTS) -installcheck: all tablespace-setup - $(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/serial_schedule $(EXTRA_TESTS) +# after installation, serial +installcheck: all tablespace-setup serial_schedule + $(pg_regress_installcheck) $(REGRESS_OPTS) \ + --schedule=serial_schedule $(EXTRA_TESTS) +# after installation, parallel installcheck-parallel: all tablespace-setup - $(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(EXTRA_TESTS) + $(pg_regress_installcheck) $(REGRESS_OPTS) $(MAXCONNOPT) \ + --schedule=$(srcdir)/parallel_schedule $(EXTRA_TESTS) +# after installation standbycheck: all - $(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/standby_schedule --use-existing + $(pg_regress_installcheck) $(REGRESS_OPTS) \ + --schedule=$(srcdir)/standby_schedule --use-existing # old interfaces follow... @@ -157,11 +181,19 @@ runcheck: check runtest: installcheck runtest-parallel: installcheck-parallel -bigtest: all tablespace-setup - $(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/serial_schedule numeric_big +bigtest: installcheck-big + +# test = after installation, serial +installcheck-big: all tablespace-setup serial_schedule serial_big_schedule + $(pg_regress_installcheck) $(REGRESS_OPTS) \ + --schedule=serial_schedule \ + --schedule=serial_big_schedule +# check = before installation, parallel bigcheck: all tablespace-setup - $(pg_regress_check) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) numeric_big + $(pg_regress_check) $(REGRESS_OPTS) $(MAXCONNOPT) \ + --schedule=$(srcdir)/parallel_schedule \ + --schedule=$(srcdir)/parallel_big_schedule ## @@ -173,6 +205,6 @@ clean distclean maintainer-clean: clean-lib rm -f $(OBJS) refint$(DLSUFFIX) autoinc$(DLSUFFIX) dummy_seclabel$(DLSUFFIX) rm -f pg_regress_main.o pg_regress.o pg_regress$(X) # things created by various check targets - rm -f $(output_files) $(input_files) + rm -f $(output_files) $(input_files) $(derived_schedules) rm -rf testtablespace rm -rf $(pg_regress_clean_files) diff --git a/src/test/regress/parallel_big_schedule b/src/test/regress/parallel_big_schedule new file mode 100644 index 000..9434abf --- /dev/null +++ b/src/test/regress/parallel_big_schedule @@ -0,0 +1 @@ +test: numeric_big diff --git a/src/test/regress/serial_schedule b/src/test/regress/serial_schedule deleted file mode 100644 index ceeca73..000 --- a/src/test/regress/serial_schedule +++ /dev/null @@ -1,140 +0,0 @@ -# src/test/regress/serial_schedule -# This should probably be in an order similar to parallel_schedule. -test: tablespace -test: boolean -test: char -test: name -test: varchar -test: text -test: int2 -test: int4 -test: int8 -test: oid -test: float4 -test: float8 -test: bit -test: numeric -test: txid -test: uuid -test: enum -test: money -test: rangetypes -test: strings -test: numerology -test: point -test: lseg -test: box -test: path -test: polygon -test: circle -test: date -test: time -test: timetz -test: timestamp -test: timestamptz -test: interval -test: abstime -test: reltime -test: tinterval -test: inet -test: macaddr -test: tstypes -test: comments -test: geometry -test: horology -test: regex -test: oidjoins
Re: [HACKERS] New regression test time
On 07/03/2013 02:50 PM, Josh Berkus wrote: On 07/03/2013 07:43 AM, Robert Haas wrote: Let's have a new schedule called minute-check with the objective to run the common tests in 60 secs. Note that we're below 60s even with assert and CLOBBER_CACHE_ALWAYS, at least on my laptop. I find that hard to believe. On friarbird, which has CLOBBER_CACHE_ALWAYS turned on, make check takes 110 minutes. The same machine runs nightjar which takes 34 seconds. 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] Add regression tests for COLLATE
On 07/03/2013 03:00 PM, Alvaro Herrera wrote: Robert Haas escribió: I agree. I think it'd be a good idea to get the buildfarm to run the existing collate.utf8.linux test regularly on platforms where it passes, +1 I can probably whip up a module for it. (I created the buildfarm module system so people could create their own addons, but sadly nobody seems to have risen to the bait.) 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] preserving forensic information when we freeze
On Wed, Jul 3, 2013 at 8:07 PM, Andres Freund and...@2ndquadrant.com wrote: On 2013-07-02 16:29:56 -0400, Robert Haas wrote: There's no possibility of getting confused here; if X is still around at all, it's xmax is of the same generation as Y's xmin. Otherwise, we've had an undetected XID wraparound. Another issue I thought about is what we will return for SELECT xmin FROM blarg; Some people use that in their applications (IIRC skytools/pqg/londiste does so) and they might get confused if we suddently return xids from the future. On the other hand, not returning the original xid would be a shame as well... Skytools/pqg/londiste use only txid_* APIs, they don't look at 4-byte xids on table rows. -- marko -- 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.4 CF 1] The Commitfest Slacker List
Tatsuo, Because I did not register the patch into CF page myself. I should have not posted it until I find any patch which I can take care of. Sorry for this. My apologies! I did post the list of patches I'd added to the CF in my patch sweep to -hackers, but I forgot to match it against the list of contributors who weren't reviewing. Sorry about that. In general, I prefer to do the patch sweep 5 days out from the start of the CF so that I have time to email people about whether or not their patches should have been included. However, this time an emergency cropped up just before the CF started and I found myself doing the patch sweep the day before the CF, which didn't leave time for an email response. This is one of those areas where better tooling could help a lot. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.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] Improvement of checkpoint IO scheduler for stable transaction responses
On 04/07/13 01:31, Robert Haas wrote:
On Wed, Jul 3, 2013 at 4:18 AM, KONDO Mitsumasa
kondo.mitsum...@lab.ntt.co.jp wrote:
I tested and changed segsize=0.25GB which is max partitioned table file size and
default setting is 1GB in configure option (./configure --with-segsize=0.25).
Because I thought that small segsize is good for fsync phase and background disk
write in OS in checkpoint. I got significant improvements in DBT-2 result!
This is interesting. Unfortunately, it has a significant downside:
potentially, there will be a lot more files in the data directory. As
it is, the number of files that exist there today has caused
performance problems for some of our customers. I'm not sure off-hand
to what degree those problems have been related to overall inode
consumption vs. the number of files in the same directory.
If the problem is mainly with number of of files in the same
directory, we could consider revising our directory layout. Instead
of:
base/${DBOID}/${RELFILENODE}_{FORK}
We could have:
base/${DBOID}/${FORK}/${RELFILENODE}
That would move all the vm and fsm forks to separate directories,
which would cut down the number of files in the main-fork directory
significantly. That might be worth doing independently of the issue
you're raising here. For large clusters, you'd even want one more
level to keep the directories from getting too big:
base/${DBOID}/${FORK}/${X}/${RELFILENODE}
...where ${X} is two hex digits, maybe just the low 16 bits of the
relfilenode number. But this would be not as good for small clusters
where you'd end up with oodles of little-tiny directories, and I'm not
sure it'd be practical to smoothly fail over from one system to the
other.
16 bits == 4 hex digits
Could you perhaps start with 1 hex digit, and automagically increase it
to 2, 3, .. as needed? There could be a status file at that level, that
would indicate the current number of hex digits, plus a temporary
mapping file when in transition.
Cheers,
Gavin
Re: [HACKERS] refresh materialized view concurrently
Kevin Grittner kgri...@ymail.com writes: OK. I had seen that no locks were held after the insert and wasn't aware that we acquired and then released them for each insert within a transaction. On the other hand, we acquire locks on all indexes even for a HOT UPDATE which uses a seqscan, and hold those until end of transaction. Is there a reason for that? Sounds dubious to me; although in the HOT code it might be that there's no convenient place to release the index locks. 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] [9.4 CF 1] The Commitfest Slacker List
Michael Meskes wrote: So, as an experiment, call it a mixed result. I would like to have some other way to motivate reviewers than public shame. I'd like to have Doesn't shame imply that people knew that were supposed to review patches in the first place? An implication that is not true, at least for some on your list. I think I better not bring up the word I would describe your email with, just for the fear of mistranslating it. If you didn't feel obligated, you wouldn't be pissed at me. You'd just blow it off (like Bruce did). I think you're angry with me because you feel guilty. My *personal* viewpoint is that all committers should feel obligated to review and commit patches from other contributors. That's why they're committers in the first place. Certainly if a committer looks at the CF application and notices that 80% of the reviewing and committing is being done by three people, none of whom have any more spare time than they do, they should feel obligated to help those people out. We have a problem with patch reviewing and committing in this project; it's not being done in a timely fashion in general (every CF last year ended late), and the people who are doing most of the work feel overworked and frustrated. This problem is getting worse every year, and will kill the project if it continues on its current trajectory. There are *only* three ways out of this hole, all three of which I'm trying to address: 1) more automation and better tools in order to reduce the total time required of each reviewer/committer; 2) a program of recruitment of new reviewers, including giving respect and recognition to people for their reviewing efforts 3) getting most of our existing contributors to shoulder their fair share of patch review. (3) is what I'm addressing on this thread. The reason I volunteered to be CFM this time was directly because of our discussion in Ottawa of how the review process wasn't working. I decided to find out *why* it wasn't working, and the first obvious thing I ran across was that most of our current and our long-term contributors weren't doing any patch review. For CF1, the number of people submitting patches outnumbered those who had volunteered for review 2 to 1. That *is* the review problem in a nutshell; everybody wants someone else to do the work. I don't think it's too much to ask people who are listed on the project developers page as major contributors to review one patch per CommitFest most of the time. If they did just *one* it would substantially decrease the workload on the people who are currently doing the vast majority of review and commit. On 07/03/2013 11:24 AM, Cédric Villemain wrote: You're looking at a short term, big effect. And long term ? Will people listed still be interested to participate in a project which stamps people ? With or without review, it's a shame if people stop proposing patches because they are not sure to get time to review other things *in time*. Yes, I am, because the CF is only supposed to be 30 days long, and I plan to finish it on time. That's my job as CFM. Several people on this thread have raised the fear of discouraging patch submitters, but the consistent evidence is that we have more submissions than we can currently handle. I'd rather have half as many submissions, but do a really good job of reviewing, improving, and integrating those than the current mess. Furthermore, there are quite a number of people who are submitting patches on paid company time. For those people, submit one, review one has to be an ironclad rule so that they can tell their bosses that they *have* to spend time on patch review. Otherwise, the review doesn't happen. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.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] [9.4 CF 1] The Commitfest Slacker List
Le mercredi 3 juillet 2013 21:03:42, Christopher Browne a écrit : On Wed, Jul 3, 2013 at 2:24 PM, Cédric Villemain ced...@2ndquadrant.comwrote: Clearly I ticked off a bunch of people by publishing the list. On the other hand, in the 5 days succeeding the post, more than a dozen additional people signed up to review patches, and we got some of the ready for committer patches cleared out -- something which nothing else I did, including dozens of private emails, general pleas to this mailing list, mails to the RRReviewers list, served to accomplish, in this or previous CFs. Others rules appeared, like the 5 days limit. To me it outlines that some are abusing the CF app and pushing there useless patches (not still ready or complete, WIP, ... Seems to me that useless overstates things, but it does seem fair to say that some patches are not sufficiently well prepared to be efficiently added into Postgres. Oops! You just made me realized my choice of words was not good at all! I didn't considered under this angle, I just meant that some patches were added to CF to help patches authors, it was a good idea, but this had some unexpected corner case. Sorry for the confusion. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] [PATCH] big test separation POC
On 2013-07-03 21:07:03 +0200, Fabien COELHO wrote: Here is a v2 which is more likely to work under VPATH. Here is a POC v4 which relies on multiple --schedule instead of creating concatenated schedule files. -- Fabien. diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile index d5935b6..8a39f7d 100644 --- a/src/test/regress/GNUmakefile +++ b/src/test/regress/GNUmakefile @@ -86,7 +86,7 @@ regress_data_files = \ $(wildcard $(srcdir)/output/*.source) \ $(filter-out $(addprefix $(srcdir)/,$(input_files)),$(wildcard $(srcdir)/sql/*.sql)) \ $(wildcard $(srcdir)/data/*.data) \ - $(srcdir)/parallel_schedule $(srcdir)/serial_schedule $(srcdir)/resultmap + $(srcdir)/parallel_schedule $(srcdir)/parallel_big_schedule $(srcdir)/resultmap install-tests: all install install-lib installdirs-tests $(MAKE) -C $(top_builddir)/contrib/spi install @@ -137,19 +137,43 @@ tablespace-setup: ## Run tests ## +# installcheck vs check: +# - whether test is run against installed or compiled version +# test schedules: parallel, parallel_big, standby +# serial schedules can be derived from parallel schedules + +derived_schedules = serial_schedule serial_big_schedule + +serial_%: parallel_% + echo # this file is generated automatically, do not edit! $@ + egrep '^(test|ignore):' $ | \ + while read op list ; do \ + for test in $$list ; do \ + echo $$op $$test ; \ + done ; \ + done $@ + This won't work on windows all that easily. Maybe we should instead add a --run-serially parameter to pg_regress? -installcheck: all tablespace-setup - $(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/serial_schedule $(EXTRA_TESTS) +# after installation, serial +installcheck: all tablespace-setup serial_schedule + $(pg_regress_installcheck) $(REGRESS_OPTS) \ + --schedule=serial_schedule $(EXTRA_TESTS) Why do we use the serial schedule for installcheck anyway? Just because of max_connections? 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] refresh materialized view concurrently
Tom Lane t...@sss.pgh.pa.us wrote: Kevin Grittner kgri...@ymail.com writes: we acquire locks on all indexes even for a HOT UPDATE which uses a seqscan, and hold those until end of transaction. Is there a reason for that? Sounds dubious to me; although in the HOT code it might be that there's no convenient place to release the index locks. Further testing shows that any UPDATE or DELETE statement acquires a RowExclusiveLock on every index on the table and holds it until end of transaction, whether or not any rows are affected and regardless of whether an index scan or a seqscan is used. In fact, just an EXPLAIN of an UPDATE or DELETE does so. It is only INSERT which releases the locks at the end of the statement. -- Kevin Grittner EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [9.4 CF 1] The Commitfest Slacker List
On Wed, Jul 3, 2013 at 12:34:50PM -0700, Josh Berkus wrote: Michael Meskes wrote: So, as an experiment, call it a mixed result. I would like to have some other way to motivate reviewers than public shame. I'd like to have Doesn't shame imply that people knew that were supposed to review patches in the first place? An implication that is not true, at least for some on your list. I think I better not bring up the word I would describe your email with, just for the fear of mistranslating it. If you didn't feel obligated, you wouldn't be pissed at me. You'd just blow it off (like Bruce did). I think you're angry with me because you feel guilty. People are supposed to feel guilty because they are not volunteering enough time, or enough time in the places the community wants? How does that make sense? Doesn't that contradict the term volunteer? Also, don't assume everyone has the thick skin I do. I do understand Josh's frustration that something different had to be done. -- 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] refresh materialized view concurrently
Kevin Grittner kgri...@ymail.com writes: Further testing shows that any UPDATE or DELETE statement acquires a RowExclusiveLock on every index on the table and holds it until end of transaction, whether or not any rows are affected and regardless of whether an index scan or a seqscan is used. In fact, just an EXPLAIN of an UPDATE or DELETE does so. It is only INSERT which releases the locks at the end of the statement. Hm, possibly the planner is taking those locks. I don't think the executor's behavior is any different. But the planner only cares about indexes in SELECT/UPDATE/DELETE, since an INSERT has no interest in scanning the target table. 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] [9.4 CF 1] The Commitfest Slacker List
On Wed, Jul 03, 2013 at 09:47:13AM -0400, Robert Haas wrote: An attempt to prod a few more people into helping review. I can see that this pissed you off, and I'm sorry about that. But I don't think that was his intent. I hoped for this kind of answer from him but ... Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org Jabber: michael.meskes at gmail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL -- 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.4 CF 1] The Commitfest Slacker List
On Wed, Jul 03, 2013 at 04:03:08PM -0400, Bruce Momjian wrote: I do understand Josh's frustration that something different had to be done. As a matter of fact I do, too. I just think the style of blaming people in public like this is not ideal. As I said I didn't even notice this email in the first hand until I was approached from other people and called a slacker in communication not related to the CF at all. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org Jabber: michael.meskes at gmail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL -- 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.4 CF 1] The Commitfest Slacker List
On Wed, Jul 03, 2013 at 12:34:50PM -0700, Josh Berkus wrote: If you didn't feel obligated, you wouldn't be pissed at me. You'd just blow it off (like Bruce did). I think you're angry with me because you feel guilty. That is outrageous bullshit! My *personal* viewpoint is that all committers should feel obligated to And my *personal* viewpoint is that nobody should be offended like this. But apparently I don't get my wish either. review and commit patches from other contributors. That's why they're committers in the first place. Certainly if a committer looks at the CF application and notices that 80% of the reviewing and committing is being done by three people, none of whom have any more spare time than they do, they should feel obligated to help those people out. How many patches did you review? You don't have to be a committer to do that. We have a problem with patch reviewing and committing in this project; it's not being done in a timely fashion in general (every CF last year ended late), and the people who are doing most of the work feel overworked and frustrated. This problem is getting worse every year, and will kill the project if it continues on its current trajectory. As if publicly blaming people for not behaving the way you would like them to will do the project a lot of good. Let me stress again that you didn't even try talking to the people in question in private before, nor did you bother putting your *suggestion* up for discussion before flaming people. Also let me stress again that I did *not* put a patch into the CF. 3) getting most of our existing contributors to shoulder their fair share of patch review. (3) is what I'm addressing on this thread. The reason I volunteered to be CFM this time was directly because of our discussion in Ottawa of how the review process wasn't working. I decided to find out *why* it wasn't working, and the first obvious thing I ran across was that most of our current and our long-term contributors weren't doing any patch review. For CF1, the number of people submitting patches outnumbered those who had volunteered for review 2 to 1. That *is* the review problem in a nutshell; everybody wants someone else to do the work. Great, I wasn't part of any discussion as I didn't make it to Ottawa this time. Neither am I part of the problem with 0 patches, but still I've got to shoulder the blame in a less than friendly way. I don't think it's too much to ask people who are listed on the project developers page as major contributors to review one patch per CommitFest most of the time. If they did just *one* it would substantially decrease the workload on the people who are currently doing the vast majority of review and commit. You didn't ask! You blamed and offended people! I won't go into details here because frankly why I have no time for reviewing a patch is none of your business. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org Jabber: michael.meskes at gmail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL -- 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.4 CF 1] The Commitfest Slacker List
On 07/03/2013 02:03 PM, Michael Meskes wrote: I won't go into details here because frankly why I have no time for reviewing a patch is none of your business. Then just send an email saying Sorry, I don't have any time for patch review this time. Maybe next time. It's pretty simple. I'm not going to apologize for expecting *committers* to participate in patch review and commit. As I said I didn't even notice this email in the first hand until I was approached from other people and called a slacker in communication not related to the CF at all. Ah, now, *that* wasn't my intent, sorry about that. It's rather a surprise to me that anyone off of the -hackers list would care. Possibly slacker was a poor choice of word given translations; in colloquial American English it's a casual term, even affectionate under some conditions. I'll make sure to use different words if I ever end up doing a list again. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.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] [v9.4] row level security -- needs a reviewer
Hackers, Please, oh please, won't someone review this? This patch has been 3 years in the making, so I suspect that it will NOT be a fast review. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.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] request a new feature in fuzzystrmatch
On 06/29/2013 01:37 PM, Joe Conway wrote: On 06/25/2013 01:37 PM, Liming Hu wrote: please remove dameraulevenshteinnocompatible related stuff, I just followed the template you created. dameraulevenshteinnocompatible was used in my first testing. diff -cNr /opt/src/pgsql-git/master/contrib/fuzzystrmatch/fuzzystrmatch--1.0.sql diff -cNr /opt/src/pgsql-git/master/contrib/fuzzystrmatch/fuzzystrmatch--unpackaged--1.0.sql Actually, given that this change will create version 1.1 of the extension, I believe the 1.0 versions of the sql scripts should probably be removed entirely. Can someone with more knowledge of the extension facility comment on that? I believe that's correct. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
