Re: [HACKERS] regression tests fails
Pavel Stehule writes: >> 2016-11-21 8:09 GMT+01:00 Craig Ringer : >>> Simple fix here is to append COLLATE "C" after the ORDER BY. > here is a patch Pushed, thanks. 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] regression tests fails
2016-11-21 8:13 GMT+01:00 Pavel Stehule :
>
>
> 2016-11-21 8:09 GMT+01:00 Craig Ringer :
>
>> On 21 November 2016 at 14:45, Pavel Stehule
>> wrote:
>>
>> > SELECT (regexp_split_to_array(data, ':'))[4], COUNT(*),
>> > (array_agg(data))[1], (array_agg(data))[count(*)]
>> > FROM pg_logical_slot_get_changes('regression_slot', NULL,NULL) WHERE
>> data
>> > ~ 'INSERT'
>> > GROUP BY 1 ORDER BY 1;
>> >
>> > but result is sensitive on locales setting - doesn't work well with
>> czech
>> > locales.
>>
>> Simple fix here is to append COLLATE "C" after the ORDER BY.
>>
>>
>>
> it needs little bit bigger change - COLLATE cannot be used with positional
> ORDER BY
>
here is a patch
Regards
Pavel
>
> Regards
>
> Pavel
>
>
>>
>> --
>> Craig Ringer http://www.2ndQuadrant.com/
>> PostgreSQL Development, 24x7 Support, Training & Services
>>
>
>
diff --git a/contrib/test_decoding/expected/spill.out b/contrib/test_decoding/expected/spill.out
index 363e9a3..10734bd 100644
--- a/contrib/test_decoding/expected/spill.out
+++ b/contrib/test_decoding/expected/spill.out
@@ -164,7 +164,7 @@ SAVEPOINT s2;
INSERT INTO spill_test SELECT 'serialize-subsmall-subbig--2:'||g.i FROM generate_series(2, 5001) g(i);
RELEASE SAVEPOINT s2;
COMMIT;
-SELECT (regexp_split_to_array(data, ':'))[4], COUNT(*), (array_agg(data))[1], (array_agg(data))[count(*)]
+SELECT (regexp_split_to_array(data, ':'))[4] COLLATE "C", COUNT(*), (array_agg(data))[1], (array_agg(data))[count(*)]
FROM pg_logical_slot_get_changes('regression_slot', NULL,NULL) WHERE data ~ 'INSERT'
GROUP BY 1 ORDER BY 1;
regexp_split_to_array | count | array_agg |array_agg
@@ -182,7 +182,7 @@ INSERT INTO spill_test SELECT 'serialize-nested-subbig-subbig--2:'||g.i FROM gen
RELEASE SAVEPOINT s2;
RELEASE SAVEPOINT s1;
COMMIT;
-SELECT (regexp_split_to_array(data, ':'))[4], COUNT(*), (array_agg(data))[1], (array_agg(data))[count(*)]
+SELECT (regexp_split_to_array(data, ':'))[4] COLLATE "C", COUNT(*), (array_agg(data))[1], (array_agg(data))[count(*)]
FROM pg_logical_slot_get_changes('regression_slot', NULL,NULL) WHERE data ~ 'INSERT'
GROUP BY 1 ORDER BY 1;
regexp_split_to_array| count | array_agg | array_agg
@@ -200,7 +200,7 @@ INSERT INTO spill_test SELECT 'serialize-nested-subbig-subsmall--2:'||g.i FROM g
RELEASE SAVEPOINT s2;
RELEASE SAVEPOINT s1;
COMMIT;
-SELECT (regexp_split_to_array(data, ':'))[4], COUNT(*), (array_agg(data))[1], (array_agg(data))[count(*)]
+SELECT (regexp_split_to_array(data, ':'))[4] COLLATE "C", COUNT(*), (array_agg(data))[1], (array_agg(data))[count(*)]
FROM pg_logical_slot_get_changes('regression_slot', NULL,NULL) WHERE data ~ 'INSERT'
GROUP BY 1 ORDER BY 1;
regexp_split_to_array | count | array_agg| array_agg
@@ -218,7 +218,7 @@ INSERT INTO spill_test SELECT 'serialize-nested-subsmall-subbig--2:'||g.i FROM g
RELEASE SAVEPOINT s2;
RELEASE SAVEPOINT s1;
COMMIT;
-SELECT (regexp_split_to_array(data, ':'))[4], COUNT(*), (array_agg(data))[1], (array_agg(data))[count(*)]
+SELECT (regexp_split_to_array(data, ':'))[4] COLLATE "C", COUNT(*), (array_agg(data))[1], (array_agg(data))[count(*)]
FROM pg_logical_slot_get_changes('regression_slot', NULL,NULL) WHERE data ~ 'INSERT'
GROUP BY 1 ORDER BY 1;
regexp_split_to_array | count | array_agg | array_agg
@@ -238,7 +238,7 @@ SAVEPOINT s3;
INSERT INTO spill_test SELECT 'serialize-nested-subbig-subbigabort-subbig-3:'||g.i FROM generate_series(5001, 1) g(i);
RELEASE SAVEPOINT s1;
COMMIT;
-SELECT (regexp_split_to_array(data, ':'))[4], COUNT(*), (array_agg(data))[1], (array_agg(data))[count(*)]
+SELECT (regexp_split_to_array(data, ':'))[4] COLLATE "C", COUNT(*), (array_agg(data))[1], (array_agg(data))[count(*)]
FROM pg_logical_slot_get_changes('regression_slot', NULL,NULL) WHERE data ~ 'INSERT'
GROUP BY 1 ORDER BY 1;
regexp_split_to_array | count |array_agg|array_agg
diff --git a/contrib/test_decoding/sql/spill.sql b/contrib/test_decoding/sql/spill.sql
index 358af0b..e638cac 100644
--- a/contrib/test_decoding/sql/spill.sql
+++ b/contrib/test_decoding/sql/spill.sql
@@ -116,7 +116,7 @@ SAVEPOINT s2;
INSERT INTO spill_test SELECT 'serializ
Re: [HACKERS] regression tests fails
2016-11-21 8:09 GMT+01:00 Craig Ringer :
> On 21 November 2016 at 14:45, Pavel Stehule
> wrote:
>
> > SELECT (regexp_split_to_array(data, ':'))[4], COUNT(*),
> > (array_agg(data))[1], (array_agg(data))[count(*)]
> > FROM pg_logical_slot_get_changes('regression_slot', NULL,NULL) WHERE
> data
> > ~ 'INSERT'
> > GROUP BY 1 ORDER BY 1;
> >
> > but result is sensitive on locales setting - doesn't work well with czech
> > locales.
>
> Simple fix here is to append COLLATE "C" after the ORDER BY.
>
>
>
it needs little bit bigger change - COLLATE cannot be used with positional
ORDER BY
Regards
Pavel
>
> --
> Craig Ringer http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Training & Services
>
Re: [HACKERS] regression tests fails
On 21 November 2016 at 14:45, Pavel Stehule wrote:
> SELECT (regexp_split_to_array(data, ':'))[4], COUNT(*),
> (array_agg(data))[1], (array_agg(data))[count(*)]
> FROM pg_logical_slot_get_changes('regression_slot', NULL,NULL) WHERE data
> ~ 'INSERT'
> GROUP BY 1 ORDER BY 1;
>
> but result is sensitive on locales setting - doesn't work well with czech
> locales.
Simple fix here is to append COLLATE "C" after the ORDER BY.
--
Craig Ringer 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] regression tests fails
2016-11-16 5:54 GMT+01:00 Pavel Stehule :
> Hi
>
> I have a repeated problem with regress tests
>
> master, Fedora 25,
>
> running on port 50848 with PID 5548
> == creating database "regression" ==
> CREATE DATABASE
> ALTER DATABASE
> == running regression test queries==
> test ddl ... ok
> test xact ... ok
> test rewrite ... ok
> test toast... ok
> test permissions ... ok
> test decoding_in_xact ... ok
> test decoding_into_rel... ok
> test binary ... ok
> test prepared ... ok
> test replorigin ... ok
> test time ... ok
> test messages ... ok
> test spill... FAILED
> == shutting down postmaster ==
>
> The result is in unstable order.
>
I was wrong - there is ORDER BY clause
SELECT (regexp_split_to_array(data, ':'))[4], COUNT(*),
(array_agg(data))[1], (array_agg(data))[count(*)]
FROM pg_logical_slot_get_changes('regression_slot', NULL,NULL) WHERE data
~ 'INSERT'
GROUP BY 1 ORDER BY 1;
but result is sensitive on locales setting - doesn't work well with czech
locales.
Regards
Pavel
> Regards
>
> Pavel
>
[HACKERS] regression tests fails
Hi I have a repeated problem with regress tests master, Fedora 25, running on port 50848 with PID 5548 == creating database "regression" == CREATE DATABASE ALTER DATABASE == running regression test queries== test ddl ... ok test xact ... ok test rewrite ... ok test toast... ok test permissions ... ok test decoding_in_xact ... ok test decoding_into_rel... ok test binary ... ok test prepared ... ok test replorigin ... ok test time ... ok test messages ... ok test spill... FAILED == shutting down postmaster == The result is in unstable order. Regards Pavel regression.diffs 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] Regression tests vs existing users in an installation
On Mon, Jul 18, 2016 at 11:13 AM, Tom Lane wrote: >> I don't particularly like your suggestion of spooky action at a >> distance between force_parallel_mode and regression_test_mode. That >> just seems kooky. > > It's certainly a judgment call as to which way is cleaner, but I don't > understand your objection. There are plenty of ways in which multiple > GUCs determine a given behavior already. Also, breaking this behavior > into two variables would let us document the user-useful behavior (do > this to test parallel safety of functions) in a different place from the > developer-useful behavior (do this to make EXPLAIN lie to you, which > surely has no possible use except for regression testing). There are certainly cases where the behavior that you get depends on multiple GUCs. For example, the vacuum cost limit stuff is like that, and so are the cost factors which control the query planner. But in each of those cases, each GUC has a function that is fully orthogonal to each other GUC. That doesn't seem to be the case here. You're saying that force_parallel_mode will decide whether we get behavior A, and regression_test_mode will decide whether we get behavior B, but if you ask for both A and B you will also get an additional behavior C which would not have been selected by either GUC taken alone. Because the different settings are now non-orthogonal, there's now no way to get A and B without C, or A and C without B. Moreover, I don't want to end up in a situation where regression_test_mode=on enables a score of minor behavior changes that can't be separated out and tested individually. It's true that checking the names of regression roles is such a very minor thing that a generic name like regression_test_mode might be OK, with the idea that anything else similarly minor that comes along later can be folded into that as well. But I fear that it will become a crutch: my code makes the regression test fail. Instead of writing better tests, add another thing that's conditional on regression_test_mode! Eventually, we'll have a bug that the regression tests fail to catch because regression_test_mode papers over the problem. We're some distance from such a situation now, of course, but I bet the temptation to be undisciplined about hooking more behavior into that GUC will be almost irresistible for new developers, and in some cases experienced ones, too. -- 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] Regression tests vs existing users in an installation
On Mon, Jul 18, 2016 at 1:34 AM, Michael Paquier wrote: > One downside of the plugin is that any users willing to do make > installcheck would need to install it as well. Not really. If the only purpose of the plugin is to verify that we're not creating regression users whose names don't start with "regress", it should be good enough to run it for "make check" but not for "make installcheck". It's not there to test functionality, just to verify that we've followed our own rules for regression tests. -- 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] Regression tests vs existing users in an installation
Peter Eisentraut writes: > On 7/15/16 6:13 PM, Tom Lane wrote: >> We've talked before about how the regression tests should be circumspect >> about what role names they create/drop, so as to avoid possibly blowing >> up an installation's existing users during "make installcheck". > I'm not particularly sure that that is a useful goal anymore. Setting > up a new instance is cheap, so if users are concerned, they should not > run the tests against their important instance. To my mind, the main point of "make installcheck" is to verify that your actual installation, not some other instance, is working right. This is far from a trivial issue; for instance on a SELinux machine, you need to be able to verify that the installed policy allows the DB to work, and that is very likely to be path-sensitive. So this remains an important requirement to me. It's true that it might be something you need to run only right after making the installation --- but that doesn't mean the DB is empty. Consider wanting to test a freshly pg_upgrade'd installation, for example. 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] Regression tests vs existing users in an installation
On 7/15/16 6:13 PM, Tom Lane wrote: > We've talked before about how the regression tests should be circumspect > about what role names they create/drop, so as to avoid possibly blowing > up an installation's existing users during "make installcheck". I'm not particularly sure that that is a useful goal anymore. Setting up a new instance is cheap, so if users are concerned, they should not run the tests against their important instance. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, 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] Regression tests vs existing users in an installation
Robert Haas writes: > On Sat, Jul 16, 2016 at 11:38 AM, Tom Lane wrote: >> I'm coming to the conclusion that the only thing that will make this >> materially better in the long run is automatic enforcement of a convention >> about what role names may be created in the regression tests. See my >> response to Stephen just now for a concrete proposal. > We could also do this by loading a C module during the regression > tests, which seems maybe less ugly than adding a GUC. Meh, I'm not convinced. As Michael points out, arranging for such a module to get loaded in an installcheck context would be difficult --- maybe not impossible, but complicated. Also, we'd have to add hook function calls in all the places it would need to get control; most of those places would probably be one-off hooks with no other conceivable use. And we'd still need to have a GUC, because I think it's inevitable that we'd need to be able to turn off the restrictions for specific tests. So that seems like a lot of work and complication just to make a GUC be custom to some undocumented extension rather than built-in. If we had no other debugging GUCs then there might be some point in rejecting this one, but we have a bunch: https://www.postgresql.org/docs/devel/static/runtime-config-developer.html > I don't particularly like your suggestion of spooky action at a > distance between force_parallel_mode and regression_test_mode. That > just seems kooky. It's certainly a judgment call as to which way is cleaner, but I don't understand your objection. There are plenty of ways in which multiple GUCs determine a given behavior already. Also, breaking this behavior into two variables would let us document the user-useful behavior (do this to test parallel safety of functions) in a different place from the developer-useful behavior (do this to make EXPLAIN lie to you, which surely has no possible use except for regression testing). Possibly a single "regression_test_mode" variable is a bad idea and we should instead have distinct developer-oriented GUCs for each special behavior we decide we need. I'm not particularly set on that, but to me it seems like less of a mess to have just 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] Regression tests vs existing users in an installation
On Mon, Jul 18, 2016 at 10:37 AM, Robert Haas wrote: > On Sat, Jul 16, 2016 at 11:38 AM, Tom Lane wrote: > We could also do this by loading a C module during the regression > tests, which seems maybe less ugly than adding a GUC. > I don't particularly like your suggestion of spooky action at a > distance between force_parallel_mode and regression_test_mode. That > just seems kooky. One downside of the plugin is that any users willing to do make installcheck would need to install it as well. -- 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] Regression tests vs existing users in an installation
On Sat, Jul 16, 2016 at 11:38 AM, Tom Lane wrote: > I'm coming to the conclusion that the only thing that will make this > materially better in the long run is automatic enforcement of a convention > about what role names may be created in the regression tests. See my > response to Stephen just now for a concrete proposal. We could also do this by loading a C module during the regression tests, which seems maybe less ugly than adding a GUC. I don't particularly like your suggestion of spooky action at a distance between force_parallel_mode and regression_test_mode. That just seems kooky. -- 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] Regression tests vs existing users in an installation
I've gone ahead and pushed a patch that does all of the cosmetic renamings needed to clean up the global-object-names situation. I've not done anything yet about those special cases in the rolenames test, since it's open for discussion exactly what to do there. I figured that this patch was bulky enough, and mechanical enough, that there wasn't much point in putting it up for review; the buildfarm will do a lot better at finding any mistakes I may have made. 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] Regression tests vs existing users in an installation
Alvaro Herrera writes: > Tom Lane wrote: >> We've talked before about how the regression tests should be circumspect >> about what role names they create/drop, so as to avoid possibly blowing >> up an installation's existing users during "make installcheck". In >> particular I believe there was consensus that such names should begin >> with, or at least include, "regress". I got around today to instrumenting >> CreateRole to see what names we were actually creating, and was quite >> depressed as to how thoroughly that guideline is being ignored (see >> attached). > I would propose that we have one test run near the beginning or right at > the beginning of the serial schedule that sets up a small number of > roles to cover most of the needs of every other test, so that most such > other tests do not need to create any roles at all. I don't think that's a very attractive idea. It would create hazards for concurrent test cases, I fear. Moreover, an un-enforced convention of "don't create roles" isn't really any safer than an un-enforced convention of "only create roles named thus-and-such"; it just takes one person who is not familiar with the convention, and one committer not paying close attention, and we're right back where we started. I'm coming to the conclusion that the only thing that will make this materially better in the long run is automatic enforcement of a convention about what role names may be created in the regression tests. See my response to Stephen just now for a concrete proposal. 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] Regression tests vs existing users in an installation
Stephen Frost writes: > * Tom Lane (t...@sss.pgh.pa.us) wrote: >> One could certainly argue that these are safe enough because nobody would >> ever create real roles by those names anyway. I'm not very comfortable >> with that though; if we believe that, why did we go to the trouble of >> making sure these cases work? > Sadly, it's not quite so simple: > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=831234 Hah. Well, I have zero interest in supporting "user" as the name of the bootstrap superuser. Even if that seemed like a good idea, why not also current_user, session_user, Public, or any other name we might want to use in the tests? The variant-output-files answer definitely doesn't scale. What seems a more useful approach is for packagers to not allow the O/S username to affect the results of "make check". initdb already has the --username switch to override its choice of the bootstrap superuser name, and pg_regress has a --user switch, so in principle it should be possible for a package to ensure that its build tests are run with the standard superuser name rather than something dependent on the environment. I'm not sure how *easy* it is, mind you. We might want to add some Makefile plumbing to make it easier to do that. But that's not what I'm on about today ... >> A more aggressive answer would be to decide we don't need these test cases >> at all and drop them. An advantage of that is that then we could >> configure some buildfarm animal to fail the next time somebody ignores >> the "test role names should contain 'regress'" rule. > I'd really like to have more test coverage rather than less, so I'd find > this a bit hard to swallow, but it might still be better than the > alternatives. As Greg mentioned, we could improve things if we were willing to invent something like a "regression_test_mode" GUC. The rough sketch would be: 1. pg_regress adds "regression_test_mode = on" to the ALTER DATABASE SET settings it already applies to created databases. 2. One of the effects of the GUC would be to throw an error (or warning?) for creation of disallowed-by-convention role names. 3. The rolenames test could turn off the GUC during creation of these specific test names. Or if we make it report WARNING not ERROR, we don't even have to do that, just include the warnings in the expected output. I find myself liking this idea, because there would be other uses for such a GUC. One thing that is awful darn tempting right now is to get rid of the "force_parallel_mode = regress" wart, making that variable back into a plain bool, and instead have that behavior emerge from having both force_parallel_mode and regression_test_mode turned on. We'd still want creation of these specific role names to be wrapped in a rolled-back transaction, so that we could document that only role names beginning with "regress_" are at hazard from "make installcheck". But I don't think that doing that really represents any meaningful loss of coverage. 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] Regression tests vs existing users in an installation
Tom, * Tom Lane (t...@sss.pgh.pa.us) wrote: > We've talked before about how the regression tests should be circumspect > about what role names they create/drop, so as to avoid possibly blowing > up an installation's existing users during "make installcheck". In > particular I believe there was consensus that such names should begin > with, or at least include, "regress". I got around today to instrumenting > CreateRole to see what names we were actually creating, and was quite > depressed as to how thoroughly that guideline is being ignored (see > attached). I had started in on this but hadn't made as much progress as I had hoped, so I'm glad to see that you're taking a look at it. > I propose to go through the regression tests and fix this (in HEAD only). > However, there's one place where it's not so easy to just substitute a > different name, because rolenames.sql is intentionally doing this: > > CREATE ROLE "Public"; > CREATE ROLE "None"; > CREATE ROLE "current_user"; > CREATE ROLE "session_user"; > CREATE ROLE "user"; > > in order to test whether we properly distinguish role-related keywords > from quoted identifiers. Obviously, modifying these would defeat the > point of the test. > > One could certainly argue that these are safe enough because nobody would > ever create real roles by those names anyway. I'm not very comfortable > with that though; if we believe that, why did we go to the trouble of > making sure these cases work? Sadly, it's not quite so simple: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=831234 Note that Christoph went ahead and closed out the bug report as it was just an issue in the regression test and not unexpected behavior, but if we're going to do something in this area then we may wish to consider fixing the case that caused that bug report. > What I'm inclined to do with this is to reduce the test to be something > like > > BEGIN; > CREATE ROLE "Public"; > CREATE ROLE "None"; > CREATE ROLE "current_user"; > CREATE ROLE "session_user"; > CREATE ROLE "user"; > ROLLBACK; > > with maybe a couple of ALTERs and GRANTs inside the transaction to verify > that the names can be referenced as well as created. This would be safe > against modifying any conflicting existing users; the only bad consequence > would be a phony failure of the test. Unfortunately, the above wouldn't fix the case where someone attempts to run the regression tests as a Unix user named "user". One suggestion discussed on the bug report was to include different result files, but that does seem pretty special-cased and overkill, though if the overall set of tests in rolenames.sql is reduced then perhaps it isn't as much of an issue. Then again, how many different result files would be needed to cover all cases? Seems like there could be quite a few, though, with this specific case, it looks like we'd at least only have to deal with one for each role which is allowed to exist already (such as "user"), without any multiplicative factors (can't run the regression test as more than one Unix user at a time). > A more aggressive answer would be to decide we don't need these test cases > at all and drop them. An advantage of that is that then we could > configure some buildfarm animal to fail the next time somebody ignores > the "test role names should contain 'regress'" rule. I'd really like to have more test coverage rather than less, so I'd find this a bit hard to swallow, but it might still be better than the alternatives. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Regression tests vs existing users in an installation
On 16 Jul 2016 12:59 pm, "Michael Paquier" wrote: > > Thanks for doing this. +1 Though I might highlight this as the kind of issue that a bug tracker would help avoid falling through the cracks and make visible to newcomers. > I am -1 for dropping the tests. We could just have a CFLAGS that adds > an elog(ERROR) in CreateRole and checks that the created role has a > wanted prefix, or have a plugin that uses the utility hook to do this > filtering. If we make a hidden regression_test_safety GUC then we could have pg_regress enable it and have these specific tests disable it explicitly with comments on why it's safe. It might even be handy for other people writing application regression tests depending on what other things it blocked. A hook might even be possible to use the same way. pg_regress would have to build and install a .so which might be tricky.
Re: [HACKERS] Regression tests vs existing users in an installation
On Sat, Jul 16, 2016 at 7:13 AM, Tom Lane wrote: > We've talked before about how the regression tests should be circumspect > about what role names they create/drop, so as to avoid possibly blowing > up an installation's existing users during "make installcheck". In > particular I believe there was consensus that such names should begin > with, or at least include, "regress". I got around today to instrumenting > CreateRole to see what names we were actually creating, and was quite > depressed as to how thoroughly that guideline is being ignored (see > attached). Thanks for doing this. > A more aggressive answer would be to decide we don't need these test cases > at all and drop them. An advantage of that is that then we could > configure some buildfarm animal to fail the next time somebody ignores > the "test role names should contain 'regress'" rule. I am -1 for dropping the tests. We could just have a CFLAGS that adds an elog(ERROR) in CreateRole and checks that the created role has a wanted prefix, or have a plugin that uses the utility hook to do this filtering. -- 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] Regression tests vs existing users in an installation
Tom Lane wrote: > We've talked before about how the regression tests should be circumspect > about what role names they create/drop, so as to avoid possibly blowing > up an installation's existing users during "make installcheck". In > particular I believe there was consensus that such names should begin > with, or at least include, "regress". I got around today to instrumenting > CreateRole to see what names we were actually creating, and was quite > depressed as to how thoroughly that guideline is being ignored (see > attached). > > I propose to go through the regression tests and fix this (in HEAD only). I would propose that we have one test run near the beginning or right at the beginning of the serial schedule that sets up a small number of roles to cover most of the needs of every other test, so that most such other tests do not need to create any roles at all. (Of course, there would be a few cases where this would defeat the purpose of the test because creating or dropping the role is precisely what is being created; those cases would have additional roles, with the proposed prefix.) So currently we have 97 roles? Probably we can get away with a dozen or so, maybe even less than that. > What I'm inclined to do with this is to reduce the test to be something > like > > BEGIN; > CREATE ROLE "Public"; > CREATE ROLE "None"; > CREATE ROLE "current_user"; > CREATE ROLE "session_user"; > CREATE ROLE "user"; > ROLLBACK; > > with maybe a couple of ALTERs and GRANTs inside the transaction to verify > that the names can be referenced as well as created. This would be safe > against modifying any conflicting existing users; the only bad consequence > would be a phony failure of the test. > > I thought about trying to preserve all the existing test cases while still > keeping these roles inside a transaction, by inserting savepoints around > the intentional failures. But there are enough intentional failures in > rolenames.sql that that would be really tedious. The existing test cases > seem enormously duplicative to me anyway, so I think a fairly short > transaction with a few tests would be sufficient to cover this territory. > A more aggressive answer would be to decide we don't need these test cases > at all and drop them. Hmm ... I think a blanket removal would go against generally accepted principle that our tests need to cover more functionality. Maybe we did go overboard on that one and the rolled-back creation is enough test for that functionality. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Regression tests vs existing users in an installation
We've talked before about how the regression tests should be circumspect about what role names they create/drop, so as to avoid possibly blowing up an installation's existing users during "make installcheck". In particular I believe there was consensus that such names should begin with, or at least include, "regress". I got around today to instrumenting CreateRole to see what names we were actually creating, and was quite depressed as to how thoroughly that guideline is being ignored (see attached). I propose to go through the regression tests and fix this (in HEAD only). However, there's one place where it's not so easy to just substitute a different name, because rolenames.sql is intentionally doing this: CREATE ROLE "Public"; CREATE ROLE "None"; CREATE ROLE "current_user"; CREATE ROLE "session_user"; CREATE ROLE "user"; in order to test whether we properly distinguish role-related keywords from quoted identifiers. Obviously, modifying these would defeat the point of the test. One could certainly argue that these are safe enough because nobody would ever create real roles by those names anyway. I'm not very comfortable with that though; if we believe that, why did we go to the trouble of making sure these cases work? What I'm inclined to do with this is to reduce the test to be something like BEGIN; CREATE ROLE "Public"; CREATE ROLE "None"; CREATE ROLE "current_user"; CREATE ROLE "session_user"; CREATE ROLE "user"; ROLLBACK; with maybe a couple of ALTERs and GRANTs inside the transaction to verify that the names can be referenced as well as created. This would be safe against modifying any conflicting existing users; the only bad consequence would be a phony failure of the test. I thought about trying to preserve all the existing test cases while still keeping these roles inside a transaction, by inserting savepoints around the intentional failures. But there are enough intentional failures in rolenames.sql that that would be really tedious. The existing test cases seem enormously duplicative to me anyway, so I think a fairly short transaction with a few tests would be sufficient to cover this territory. A more aggressive answer would be to decide we don't need these test cases at all and drop them. An advantage of that is that then we could configure some buildfarm animal to fail the next time somebody ignores the "test role names should contain 'regress'" rule. Comments? regards, tom lane LOG: created role tablespace_testuser1 LOG: created role tablespace_testuser2 LOG: created role regtestrole LOG: created role regress_rol_op1 LOG: created role regress_rol_op3 LOG: created role regress_rol_op4 LOG: created role regress_rol_op5 LOG: created role regress_rol_op6 LOG: created role regression_reindexuser LOG: created role regtest_unpriv_user LOG: created role test_def_superuser LOG: created role test_superuser LOG: created role Public LOG: created role None LOG: created role current_user LOG: created role session_user LOG: created role user LOG: created role testrol0 LOG: created role testrolx LOG: created role testrol2 LOG: created role testrol1 LOG: created role test_def_inherit LOG: created role test_inherit LOG: created role test_def_createrole LOG: created role test_createrole LOG: created role test_def_createdb LOG: created role test_createdb LOG: created role test_def_role_canlogin LOG: created role test_role_canlogin LOG: created role test_def_user_canlogin LOG: created role test_user_canlogin LOG: created role test_def_replication LOG: created role test_replication LOG: created role tu1 LOG: created role tr1 LOG: created role tg1 LOG: created role test_def_bypassrls LOG: created role test_bypassrls LOG: created role view_user1 LOG: created role view_user2 LOG: created role selinto_user LOG: created role regtest_addr_user LOG: created role regress_rls_alice LOG: created role regress_rls_bob LOG: created role regress_rls_carol LOG: created role regress_rls_exempt_user LOG: created role regress_rls_group1 LOG: created role regress_rls_group2 LOG: created role regress_rol_lock1 LOG: created role regressuser1 LOG: created role regressuser2 LOG: created role regressuser3 LOG: created role regressuser4 LOG: created role regressuser5 LOG: created role regressgroup1 LOG: created role regressgroup2 LOG: created role seclabel_user1 LOG: created role seclabel_user2 LOG: created role schemauser1 LOG: created role schemauser2 LOG: renamed role schemauser2 to schemauser_renamed LOG: created role locktable_user LOG: created role regress_rls_eve LOG: created role regress_rls_frank LOG: created role regress_rls_dob_role1 LOG: created role regress_rls_dob_role2 LOG: created role regress_user_mvtest LOG: created role regtest_alter_user3 LOG: created role regtest_alter_user2 LOG: created role regtest_alter_user1 LOG: created role regtest_alter_user LOG: created role regtest_alter_user5 LOG: created ro
Re: [HACKERS] regression tests fail for different block sizes
Jeff Davis writes: > In master, 4 tests fail due to plan changes with blocksize 32K. The > failures started creeping in around 9.0. AFAIR, that's been true since the dark ages, not since 9.0. > I don't see a clean way to make the plans consistent with 8K and 32K > pages, so I was about to write this off as "we don't care much". Indeed. There are any number of parameters that you can change that will break the regression tests by changing plan choices. If we tried to cover all such cases we'd end up with lobotomized tests that detect fewer issues, not more. > Then I ran with block size 1KB, and I got what appears to be a genuine > failure related to range types and/or spgist. This might be worth investigating, but it doesn't mean that we need automated regression tests covering the case. 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] regression tests fail for different block sizes
In master, 4 tests fail due to plan changes with blocksize 32K. The failures started creeping in around 9.0. I don't see a clean way to make the plans consistent with 8K and 32K pages, so I was about to write this off as "we don't care much". Then I ran with block size 1KB, and I got what appears to be a genuine failure related to range types and/or spgist. I'm still investigating that, which will be a separate discussion, but the fact that different block sizes can catch real errors makes me think we should properly support them in the regression tests. Any ideas how we can make the plans consistent enough when the blocksize varies? Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Regression tests failing if not launched on db "regression"
On Tue, Feb 4, 2014 at 12:49 AM, Robert Haas wrote: > On Fri, Jan 31, 2014 at 5:48 PM, Michael Paquier > wrote: >> The part about the planning parameter looks good, thanks. The places >> used to mention the databases used also makes more sense. Thanks for >> your input. > > OK, committed and back-patched to 9.3. Thanks. -- 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] Regression tests failing if not launched on db "regression"
On Fri, Jan 31, 2014 at 5:48 PM, Michael Paquier wrote: > On Sat, Feb 1, 2014 at 12:42 AM, Robert Haas wrote: >> On Fri, Jan 31, 2014 at 2:28 AM, Michael Paquier >> wrote: I took a look at this with a view to committing it but on examination I'm not sure this is the best way to proceed. The proposed text documents that the tests should be run in a database called regression, but the larger documentation chapter of which this section is a part never explains how to run them anywhere else, so it feels a bit like telling a ten-year-old not to burn out the clutch. The bit about not changing enable_* probably belongs, if anywhere, in section 30.2, on test evaluation, rather than here. >>> And what about the attached? I have moved all the content to 30.2, and >>> added two paragraphs: one about the planner flags, the other about the >>> database used. >>> Regards, >> >> Well, it doesn't really address my first concern, which was that you >> talk about running the tests in a database named regression, but >> that's exactly what "make check" does and it's unclear how you would >> do anything else without modifying the source code. It's not the >> purpose of the documentation to tell you all the ways that you could >> break things if you patch the tree. I also don't want to document >> exactly which tests would fail if you did hack things like that; that >> documentation is likely to become outdated. >> >> I think the remaining points you raise are worth mentioning. I'm >> attaching a patch with my proposed rewording of your changes. I made >> the section about configuration parameters a bit more generic and >> adjusted the phrasing to sound more natural in English, and I moved >> your mention of the other issues around a bit. What do you think of >> this version? > The part about the planning parameter looks good, thanks. The places > used to mention the databases used also makes more sense. Thanks for > your input. OK, committed and back-patched to 9.3. -- 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] Regression tests failing if not launched on db "regression"
On Sat, Feb 1, 2014 at 12:42 AM, Robert Haas wrote: > On Fri, Jan 31, 2014 at 2:28 AM, Michael Paquier > wrote: >>> I took a look at this with a view to committing it but on examination >>> I'm not sure this is the best way to proceed. The proposed text >>> documents that the tests should be run in a database called >>> regression, but the larger documentation chapter of which this section >>> is a part never explains how to run them anywhere else, so it feels a >>> bit like telling a ten-year-old not to burn out the clutch. >>> >>> The bit about not changing enable_* probably belongs, if anywhere, in >>> section 30.2, on test evaluation, rather than here. >> And what about the attached? I have moved all the content to 30.2, and >> added two paragraphs: one about the planner flags, the other about the >> database used. >> Regards, > > Well, it doesn't really address my first concern, which was that you > talk about running the tests in a database named regression, but > that's exactly what "make check" does and it's unclear how you would > do anything else without modifying the source code. It's not the > purpose of the documentation to tell you all the ways that you could > break things if you patch the tree. I also don't want to document > exactly which tests would fail if you did hack things like that; that > documentation is likely to become outdated. > > I think the remaining points you raise are worth mentioning. I'm > attaching a patch with my proposed rewording of your changes. I made > the section about configuration parameters a bit more generic and > adjusted the phrasing to sound more natural in English, and I moved > your mention of the other issues around a bit. What do you think of > this version? The part about the planning parameter looks good, thanks. The places used to mention the databases used also makes more sense. Thanks for your input. -- 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] Regression tests failing if not launched on db "regression"
On Fri, Jan 31, 2014 at 2:28 AM, Michael Paquier wrote: >> I took a look at this with a view to committing it but on examination >> I'm not sure this is the best way to proceed. The proposed text >> documents that the tests should be run in a database called >> regression, but the larger documentation chapter of which this section >> is a part never explains how to run them anywhere else, so it feels a >> bit like telling a ten-year-old not to burn out the clutch. >> >> The bit about not changing enable_* probably belongs, if anywhere, in >> section 30.2, on test evaluation, rather than here. > And what about the attached? I have moved all the content to 30.2, and > added two paragraphs: one about the planner flags, the other about the > database used. > Regards, Well, it doesn't really address my first concern, which was that you talk about running the tests in a database named regression, but that's exactly what "make check" does and it's unclear how you would do anything else without modifying the source code. It's not the purpose of the documentation to tell you all the ways that you could break things if you patch the tree. I also don't want to document exactly which tests would fail if you did hack things like that; that documentation is likely to become outdated. I think the remaining points you raise are worth mentioning. I'm attaching a patch with my proposed rewording of your changes. I made the section about configuration parameters a bit more generic and adjusted the phrasing to sound more natural in English, and I moved your mention of the other issues around a bit. What do you think of this version? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml index 2b95587..edb476a 100644 --- a/doc/src/sgml/regress.sgml +++ b/doc/src/sgml/regress.sgml @@ -125,7 +125,9 @@ gmake installcheck-parallel The tests will expect to contact the server at the local host and the default port number, unless directed otherwise by PGHOST and - PGPORT environment variables. + PGPORT environment variables. The tests will be run in a + database named regression; any existing database by this name + will be dropped. @@ -147,7 +149,9 @@ gmake installcheck The contrib modules must have been built and installed first. You can also do this in a subdirectory of contrib to run - the tests for just one module. + the tests for just one module. Tests of contrib modules will + be run in a database named contrib_regression; any existing + database by this name will be dropped. @@ -471,6 +475,18 @@ diff results/random.out expected/random.out not worry unless the random test fails repeatedly. + + +Configuration Parameters + + + When running the tests against an existing installation, some non-default + parameter settings could cause the tests to fail. For example, the + settings described in + could cause plan changes that would affect the results of tests which + use EXPLAIN. + + -- 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] Regression tests failing if not launched on db "regression"
On Fri, Jan 31, 2014 at 6:09 AM, Robert Haas wrote: > On Thu, Jan 30, 2014 at 3:32 AM, Christian Kruse > wrote: >>> For the documentation patch, I propose the attached to avoid future >>> confusions. Comments? It might make sense to back-patch as well. >> >> Compiles, didn't find any typos and I think it is comprehensible. > > I took a look at this with a view to committing it but on examination > I'm not sure this is the best way to proceed. The proposed text > documents that the tests should be run in a database called > regression, but the larger documentation chapter of which this section > is a part never explains how to run them anywhere else, so it feels a > bit like telling a ten-year-old not to burn out the clutch. > > The bit about not changing enable_* probably belongs, if anywhere, in > section 30.2, on test evaluation, rather than here. And what about the attached? I have moved all the content to 30.2, and added two paragraphs: one about the planner flags, the other about the database used. Regards, -- Michael *** a/doc/src/sgml/regress.sgml --- b/doc/src/sgml/regress.sgml *** *** 471,476 diff results/random.out expected/random.out --- 471,510 not worry unless the random test fails repeatedly. + + + Planner Configuration parameters + + + Parameters able to change the planning of queries like enable/disable + flags described in for + EXPLAIN should use default values. + + + + + Database Name + + + Regression tests should be run on a database named regression + to prevent failures of tests using directly or indirectly the current + database name in output results. The tests that would fail in this case + include updatable_views, foreign_data, + xml_map and sequence. + + + + regression is the default database name for tests of core, + and contrib modules use contrib_regression as default. + + + + Running regression tests with pg_regress + causes the existed database to be dropped before running the tests, + so be sure that there is not already a database with the same name + existing on server before running it. + + -- 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] Regression tests failing if not launched on db "regression"
On Thu, Jan 30, 2014 at 3:32 AM, Christian Kruse wrote: >> For the documentation patch, I propose the attached to avoid future >> confusions. Comments? It might make sense to back-patch as well. > > Compiles, didn't find any typos and I think it is comprehensible. I took a look at this with a view to committing it but on examination I'm not sure this is the best way to proceed. The proposed text documents that the tests should be run in a database called regression, but the larger documentation chapter of which this section is a part never explains how to run them anywhere else, so it feels a bit like telling a ten-year-old not to burn out the clutch. The bit about not changing enable_* probably belongs, if anywhere, in section 30.2, on test evaluation, rather than here. -- 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] Regression tests failing if not launched on db "regression"
Hi, > For the documentation patch, I propose the attached to avoid future > confusions. Comments? It might make sense to back-patch as well. Compiles, didn't find any typos and I think it is comprehensible. Looks fine for me. Best regards, -- Christian Kruse http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services pgpCD1puVcmJQ.pgp Description: PGP signature
Re: [HACKERS] Regression tests failing if not launched on db "regression"
On Fri, Dec 6, 2013 at 12:02 PM, Peter Eisentraut wrote: > On Thu, 2013-12-05 at 17:12 +0900, Michael Paquier wrote: >> IMHO, the regression test suite would gain in consistency and >> portability if we do not refer to data that is database-dependent. > > I once did some similar fixes (e3d9dceef62e072cf9a433ae6c74a1c5a10d94d3) > but then didn't pursue it any longer, because it would restrict what we > could actually test. I don't remember what I was trying to do there, > but why do you need to run the tests in a different database? I don't know, but by looking at this test code I could guess that using a custom database name (that also changed depending on the environment used) made errors easier to track. So, I just went ahead, cleaned up our code to use "regression" and made things a bit smarter for the environment information. For the documentation patch, I propose the attached to avoid future confusions. Comments? It might make sense to back-patch as well. Also... An advice for the archives and other people here: never update test output dynamically and use the existing ones. I wouldn't even recommend adding new outputs to the existing tests except if you want to make your maintenance a pain as you would need to track new tests and update accordingly. Even better, submit patches if new outputs make sense, or write new tests and break things independently as much as you want. -- Michael commit 0e40cb47189062433b99c1e33e75a096c7c97dd8 Author: Michael Paquier Date: Fri Dec 6 13:13:23 2013 +0900 Addition of in-core test suite restrictions in documentation Mention in the documentation of regression tests limitations related to the database where tests are run as well as possible inconsistencies for server parameters. diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml index 2b95587..719210b 100644 --- a/doc/src/sgml/regress.sgml +++ b/doc/src/sgml/regress.sgml @@ -257,6 +257,24 @@ gmake check EXTRA_TESTS=collate.linux.utf8 LANG=en_US.utf8 platforms, and only when run in a database that uses UTF-8 encoding. + + + Restrictions + + +Regression tests should be run on a database named regression +to prevent failures of tests using directly or indirectly the current +database name in output results. regression is the default +database name for tests on core, and contrib modules use +contrib_regression as default. + + + +Parameters able to change the output of queries like enable/disable flags +described in for +EXPLAIN should use default values as well. + + -- 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] Regression tests failing if not launched on db "regression"
On Thu, 2013-12-05 at 17:12 +0900, Michael Paquier wrote: > IMHO, the regression test suite would gain in consistency and > portability if we do not refer to data that is database-dependent. I once did some similar fixes (e3d9dceef62e072cf9a433ae6c74a1c5a10d94d3) but then didn't pursue it any longer, because it would restrict what we could actually test. I don't remember what I was trying to do there, but why do you need to run the tests in a different database? -- 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] Regression tests failing if not launched on db "regression"
> On 2013/12/06, at 3:52, Tom Lane wrote: > > Robert Haas writes: >>> On Thu, Dec 5, 2013 at 9:24 AM, Tom Lane wrote: >>> Michael Paquier writes: It happens that the following regression tests are failing if they are run on a database not named "regression": > >>> This does not seem like a bug to me, although maybe we'd better update the >>> documentation to specify that you need to use a DB named regression. > >> At the same thing, supporting it might not cost anything. > > Well, changing these specific tests today might not be terribly expensive, > but what is it that prevents more such tests from being committed next > week? Perhaps by somebody who feels current_database() should be included > in code coverage, for example? > > More generally, we never have and never can promise that the regression > tests pass regardless of environment. If you turn off enable_seqscan, > for instance, you'll get a whole lot of not-terribly-exciting diffs. > I see no particular benefit to promising that the name of the regression > database isn't significant. You got a point here. Classifying that as a "don't do that" in the documentation is fine for me as well, as I recall that --dbname has been added to pg_regress only for contrib regression support. Regards, -- Michael (Sent from my mobile phone) -- 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] Regression tests failing if not launched on db "regression"
Robert Haas writes: > On Thu, Dec 5, 2013 at 9:24 AM, Tom Lane wrote: >> Michael Paquier writes: >>> It happens that the following regression tests are failing if they are >>> run on a database not named "regression": >> This does not seem like a bug to me, although maybe we'd better update the >> documentation to specify that you need to use a DB named regression. > At the same thing, supporting it might not cost anything. Well, changing these specific tests today might not be terribly expensive, but what is it that prevents more such tests from being committed next week? Perhaps by somebody who feels current_database() should be included in code coverage, for example? More generally, we never have and never can promise that the regression tests pass regardless of environment. If you turn off enable_seqscan, for instance, you'll get a whole lot of not-terribly-exciting diffs. I see no particular benefit to promising that the name of the regression database isn't significant. 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] Regression tests failing if not launched on db "regression"
On Thu, Dec 5, 2013 at 9:24 AM, Tom Lane wrote: > Michael Paquier writes: >> It happens that the following regression tests are failing if they are >> run on a database not named "regression": > > This does not seem like a bug to me, although maybe we'd better update the > documentation to specify that you need to use a DB named regression. At the same thing, supporting it might not cost anything. -- 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] Regression tests failing if not launched on db "regression"
Michael Paquier writes: > It happens that the following regression tests are failing if they are > run on a database not named "regression": This does not seem like a bug to me, although maybe we'd better update the documentation to specify that you need to use a DB named regression. 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] Regression tests failing if not launched on db "regression"
On Thu, Dec 5, 2013 at 5:12 PM, Michael Paquier wrote: > Those tests are failing because some relations of information_schemas > contain information that are database-dependent. I forgot to mention that it is our QE team at VMware that reported me a portion of this failure, and I just had to dig a little bit to find the root cause. And for the curious people: yes, we run regressions on customized database names. 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
[HACKERS] Regression tests failing if not launched on db "regression"
Hi all, It happens that the following regression tests are failing if they are run on a database not named "regression": - updatable_views - foreign_data - sequence Those tests are failing because some relations of information_schemas contain information that are database-dependent. Please see the diffs attached. Note that this can be easily reproduced by running pg_regress with a command of this type from src/test/regress: ./pg_regress --inputdir=. --temp-install=./tmp_check \ --top-builddir=../../.. --dlpath=. \ --schedule=./parallel_schedule \ --dbname=foo IMHO, the regression test suite would gain in consistency and portability if we do not refer to data that is database-dependent. Opinions? A patch fixing that would be trivial to do, and I do not mind writing it. Also, if we consider that as a bug, and I think it is, the fix should be back-patched as well. Regards, -- Michael dbname_regressions.diffs 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] regression tests
On Fri, Sep 6, 2013 at 1:34 PM, Jeremy Harris wrote: >> What's an index-hash operation? > > Ones that hit tuplesort_begin_index_hash() Oh. Well, it looks to me like that function can only get called when building a hash index. Specifically, according to the comment in hashbuild(), a hash index projected to be larger than shared_buffers. The regression tests are generally designed to work on small amounts of data since they need to run quickly, so this isn't too surprising. Hash indexes are a somewhat underwhelming feature anyway, since btrees typically perform as well or better, and since hash indexes are not WAL-logged and therefore can be corrupted on a crash. -- 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] regression tests
On 06/09/13 15:44, Robert Haas wrote: On Fri, Sep 6, 2013 at 3:34 AM, Jeremy Harris wrote: I don't see the regression tests running any index-hash operations. What am I missing? What's an index-hash operation? Ones that hit tuplesort_begin_index_hash() -- Cheers, Jeremy -- 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] regression tests
On Fri, Sep 6, 2013 at 3:34 AM, Jeremy Harris wrote: >I don't see the regression tests running any index-hash operations. > What am I missing? What's an index-hash operation? ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] regression tests
Hi, I don't see the regression tests running any index-hash operations. What am I missing? -- Cheers, Jeremy -- 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] Regression tests fail once XID counter exceeds 2 billion
On Wed, Nov 16, 2011 at 07:08:27PM -0500, Tom Lane wrote: > I wrote: > > Simon Riggs writes: > >> We need a function called transactionid_current() so a normal user can > >> write > > >> select virtualtransaction > >> from pg_locks > >> where transactionid = transactionid_current() > > >> and have it "just work". > > > That would solve that one specific use-case. The reason I suggested > > txid_from_xid is that it could also be used to compare XIDs seen in > > tuples to members of a txid_snapshot, which is not possible now. > > BTW, a pgsql-general question just now made me realize that > txid_from_xid() could have another use-case too. Right now, there are > no inequality comparisons on XIDs, which is necessary because XIDs in > themselves don't have a total order. However, you could > > ORDER BY txid_from_xid(xmin) > > and it would work, ie, give you rows in their XID order. This could be > useful for finding the latest-modified rows in a table, modulo the fact > that it would be ordering by transaction start time not commit time. Added to TODO: Add function to allow easier transaction id comparisons http://archives.postgresql.org/pgsql-hackers/2011-11/msg00786.php -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Regression tests for preload extension
"Kevin Grittner" writes: > I wrote a little extension to store a few small strings in shared > memory. It seems to be working fine, and now I would like to write > some regression tests; but it's not immediately obvious to me how I > can do that. The approach used by, for example, citext doesn't > work, because I don't see how to set shared_preload_libraries for > the server startup. The existing contrib extensions which preload > either seem to do something ad hoc or skip regression tests > entirely, so I suspect that is my choice; but I figured I should > ask. Hm. pg_regress.c goes to some trouble to allow you to set session-level options by setting PGOPTIONS in its environment, but that won't work for options that have to be given to the postmaster. Maybe we should invent a pg_regress switch that allows additional switches to be given to the temp postmaster. Of course, this is never gonna work for "make installcheck". 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] Regression tests for preload extension
I wrote a little extension to store a few small strings in shared memory. It seems to be working fine, and now I would like to write some regression tests; but it's not immediately obvious to me how I can do that. The approach used by, for example, citext doesn't work, because I don't see how to set shared_preload_libraries for the server startup. The existing contrib extensions which preload either seem to do something ad hoc or skip regression tests entirely, so I suspect that is my choice; but I figured I should ask. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Regression tests fail once XID counter exceeds 2 billion
I wrote: > Simon Riggs writes: >> We need a function called transactionid_current() so a normal user can write >> select virtualtransaction >> from pg_locks >> where transactionid = transactionid_current() >> and have it "just work". > That would solve that one specific use-case. The reason I suggested > txid_from_xid is that it could also be used to compare XIDs seen in > tuples to members of a txid_snapshot, which is not possible now. BTW, a pgsql-general question just now made me realize that txid_from_xid() could have another use-case too. Right now, there are no inequality comparisons on XIDs, which is necessary because XIDs in themselves don't have a total order. However, you could ORDER BY txid_from_xid(xmin) and it would work, ie, give you rows in their XID order. This could be useful for finding the latest-modified rows in a table, modulo the fact that it would be ordering by transaction start time not commit time. 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] Regression tests fail once XID counter exceeds 2 billion
Simon Riggs writes: > We need a function called transactionid_current() so a normal user can write >select virtualtransaction >from pg_locks >where transactionid = transactionid_current() > and have it "just work". That would solve that one specific use-case. The reason I suggested txid_from_xid is that it could also be used to compare XIDs seen in tuples to members of a txid_snapshot, which is not possible now. 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] Regression tests fail once XID counter exceeds 2 billion
On Sun, Nov 13, 2011 at 11:16 PM, Tom Lane wrote: > While investigating bug #6291 I was somewhat surprised to discover > $SUBJECT. The cause turns out to be this kluge in alter_table.sql: > > select virtualtransaction > from pg_locks > where transactionid = txid_current()::integer ... > that plasters on the appropriate epoch value for an > assumed-to-be-current-or-recent xid, and returns something that squares > with the txid_snapshot functions. Then the test could be coded without > kluges as That fixes the test, but it doesn't fix the unreasonability of this situation. We need a function called transactionid_current() so a normal user can write select virtualtransaction from pg_locks where transactionid = transactionid_current() and have it "just work". We need a function whose behaviour matches xid columns in pg_locks and elsewhere and that doesn't need to have anything to do with txid datatype. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Regression tests fail once XID counter exceeds 2 billion
On Sun, Nov 13, 2011 at 6:16 PM, Tom Lane wrote: > While investigating bug #6291 I was somewhat surprised to discover > $SUBJECT. The cause turns out to be this kluge in alter_table.sql: > > select virtualtransaction > from pg_locks > where transactionid = txid_current()::integer > > which of course starts to fail with "integer out of range" as soon as > txid_current() gets past 2^31. Right now, since there is no cast > between xid and any integer type, and no comparison operator except the > dubious xideqint4 one, the only way we could fix this is something > like > > where transactionid::text = (txid_current() % (2^32))::text > > which is surely pretty ugly. Is it worth doing something less ugly? > I'm not sure if there are any other use-cases for this type of > comparison, but if there are, seems like it would be sensible to invent > a function along the lines of > > txid_from_xid(xid) returns bigint > > that plasters on the appropriate epoch value for an > assumed-to-be-current-or-recent xid, and returns something that squares > with the txid_snapshot functions. Then the test could be coded without > kluges as > > where txid_from_xid(transactionid) = txid_current() > > Thoughts? Well, the mod-2^32 arithmetic doesn't bother me, but if you're feeling motivated to invent txid_from_xid() I think that would be fine, too. -- 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
[HACKERS] Regression tests fail once XID counter exceeds 2 billion
While investigating bug #6291 I was somewhat surprised to discover $SUBJECT. The cause turns out to be this kluge in alter_table.sql: select virtualtransaction from pg_locks where transactionid = txid_current()::integer which of course starts to fail with "integer out of range" as soon as txid_current() gets past 2^31. Right now, since there is no cast between xid and any integer type, and no comparison operator except the dubious xideqint4 one, the only way we could fix this is something like where transactionid::text = (txid_current() % (2^32))::text which is surely pretty ugly. Is it worth doing something less ugly? I'm not sure if there are any other use-cases for this type of comparison, but if there are, seems like it would be sensible to invent a function along the lines of txid_from_xid(xid) returns bigint that plasters on the appropriate epoch value for an assumed-to-be-current-or-recent xid, and returns something that squares with the txid_snapshot functions. Then the test could be coded without kluges as where txid_from_xid(transactionid) = txid_current() 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] Regression Tests (opr) Sanity
Tom Lane writes: > Just make two pg_proc entries that are pointing at two C functions. > The C functions can call a common subroutine after extracting their > arguments. Mmmm, ok, will adapt the idea to the current code, where the extracting is mingled into the processing. Thanks for the idea, that's much simpler this way. Cleaner ain't always better :) Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Regression Tests (opr) Sanity
Dimitri Fontaine writes: > The pg_execute_from_file() function is designed to work with either a > filename as its sole argument, or the filename and a VARIADIC text list > of arguments containing placeholder names and values. It works fine with > two entries in pg_proc using the same backend function, and it looks > like the following from a psql shell: Just make two pg_proc entries that are pointing at two C functions. The C functions can call a common subroutine after extracting their arguments. 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] Regression Tests (opr) Sanity
Hi,
Trying to fix a regression test problem I've left for better days while
developping the extensions, some help is needed.
The pg_execute_from_file() function is designed to work with either a
filename as its sole argument, or the filename and a VARIADIC text list
of arguments containing placeholder names and values. It works fine with
two entries in pg_proc using the same backend function, and it looks
like the following from a psql shell:
List of functions
Schema | Name | Result data type | Argument data types |
Type
+--+--+-+
pg_catalog | pg_execute_from_file | void | text|
normal
pg_catalog | pg_execute_from_file | void | text, VARIADIC text |
normal
(2 rows)
Now the opr_sanity check includes the following query, which is expected
not to return any row:
=# SELECT p1.oid, p1.proname, p2.oid, p2.proname
-# FROM pg_proc AS p1, pg_proc AS p2
-# WHERE p1.oid < p2.oid AND
-# p1.prosrc = p2.prosrc AND
-# p1.prolang = 12 AND p2.prolang = 12 AND
-# (p1.proisagg = false OR p2.proisagg = false) AND
-# (p1.prolang != p2.prolang OR
(# p1.proisagg != p2.proisagg OR
(# p1.prosecdef != p2.prosecdef OR
(# p1.proisstrict != p2.proisstrict OR
(# p1.proretset != p2.proretset OR
(# p1.provolatile != p2.provolatile OR
(# p1.pronargs != p2.pronargs);
oid | proname| oid | proname
--+--+--+--
3927 | pg_execute_from_file | 3928 | pg_execute_from_file
(1 row)
Oops. I'm not granted to do it this way. So I've been trying to setup
pg_proc.h with a single entry and the default arguments. That's a weird
thing in there, pg_node_tree. So I've tried to copy/paste what I get
from pg_proc when I create a function in SQL with the same prototype:
create or replace function foo(text, variadic text[] default '{}'::text[])
returns text
language sql
as $$
select $1 || coalesce(',' || (select array_to_string(array_agg(x), ',') from
unnest($2) x), '')
$$;
({CONST :consttype 1009 :consttypmod -1 :constlen -1 :constbyval false
:constisnull false :location 61 :constvalue 16 [ 64 0 0 0 0 0 0 0 0 0 0 0 25 0
0 0 ]})
Then initdb says FATAL: cannot accept a value of type pg_node_tree.
So, should I fix the opr_sanity check, and if so, what would be the
right approach? Or should we get the proargdefaults supported in the
bootstrap mode somehow? Or should I create the function in a SQL script
that initdb will use, somewhere?
Of course having a single entry in pg_proc without default values for
the placeholders won't fly, because the user is expected to be able to
actually use the 1-argument version of the function (no placeholder).
And I don't think having 2 names is a great answer, but if it comes to
that, of course, it's easy to do.
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Regression tests versus the buildfarm environment
On 08/12/2010 10:22 AM, Tom Lane wrote: Peter Eisentraut writes: On ons, 2010-08-11 at 16:17 -0400, Tom Lane wrote: We should have the buildfarm configuration such that any one run uses the same port number for both installed and uninstalled regression tests. I'm getting lost here what the actual requirements are. The above is obviously not going to work as a default for pg_regress, because the port number for an installed test is determined by the user and is likely to be in the public range, whereas the uninstalled test should use something from the private range. Certainly, but the buildfarm's "installed" test doesn't try to start on 5432. It starts on whatever branch_port the buildfarm owner has specified for that animal and that branch. It's the owner's responsibility to make that nonconflicting across the services and buildfarm critters he has running on a given machine. What I'm saying is that *in the buildfarm* we want the "make check" case to also use this predetermined safe port number. That has nothing whatever to do with what is good practice for other cases. Well, I think the steps to do that are: * change src/test/GNUmakefile to provide for a TMP_PORT setting that gets passed to pg_regress * change src/tools/msvc/vcregress.pl to match * backpatch these changes to all live branches * change the buildfarm script to use the new options. I don't think this should preclude changing the way we calculate the default port for pg_regress, for the reasons mentioned upthread. cheers andrew
Re: [HACKERS] Regression tests versus the buildfarm environment
Peter Eisentraut writes: > On ons, 2010-08-11 at 16:17 -0400, Tom Lane wrote: >> We should have the buildfarm configuration such that any one run uses >> the same port number for both installed and uninstalled regression >> tests. > I'm getting lost here what the actual requirements are. The above is > obviously not going to work as a default for pg_regress, because the > port number for an installed test is determined by the user and is > likely to be in the public range, whereas the uninstalled test should > use something from the private range. Certainly, but the buildfarm's "installed" test doesn't try to start on 5432. It starts on whatever branch_port the buildfarm owner has specified for that animal and that branch. It's the owner's responsibility to make that nonconflicting across the services and buildfarm critters he has running on a given machine. What I'm saying is that *in the buildfarm* we want the "make check" case to also use this predetermined safe port number. That has nothing whatever to do with what is good practice for other cases. 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] Regression tests versus the buildfarm environment
On 08/12/2010 08:43 AM, Peter Eisentraut wrote: On ons, 2010-08-11 at 15:06 -0400, Andrew Dunstan wrote: You original email said: For some historic reasons, I have my local scripts set up so that they build development instances using the hardcoded port 65432. I think my response would be "Don't do that". Do you have a concrete suggestion for a different way to handle it? Well, I do all my builds under a common directory, and my setup shell script has stuff like this to choose a port: for port in `seq -w 5701 5799` ; do grep -q -- "--with-pgport=$port" $base/*/config.log || break done It's worked fairly well for me for about five years now. No doubt there could be many variations on this theme. cheers andrew
Re: [HACKERS] Regression tests versus the buildfarm environment
On ons, 2010-08-11 at 15:06 -0400, Andrew Dunstan wrote: > You original email said: > > For some historic reasons, I have my local scripts set up so > that they build development instances using the hardcoded port > 65432. > > I think my response would be "Don't do that". Do you have a concrete suggestion for a different way to handle it? -- 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] Regression tests versus the buildfarm environment
On ons, 2010-08-11 at 16:17 -0400, Tom Lane wrote: > We should have the buildfarm configuration such that any one run uses > the same port number for both installed and uninstalled regression > tests. I'm getting lost here what the actual requirements are. The above is obviously not going to work as a default for pg_regress, because the port number for an installed test is determined by the user and is likely to be in the public range, whereas the uninstalled test should use something from the private range. -- 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] Regression tests versus the buildfarm environment
On Wed, Aug 11, 2010 at 5:16 PM, Tom Lane wrote: > Andrew Dunstan writes: >> On 08/11/2010 04:47 PM, Tom Lane wrote: >>> I prefer the change-the-default approach mainly because it wouldn't >>> require any documentation, > >> Yeah. The other advantage is that we would not need to wait until we had >> got everyone to update their versions of the buildfarm code. > > Um. That's actually a pretty darn strong point, considering how > slow some buildfarm owners are to update the script :-( Well, it's a convenient time to get such changes in, now, because folks are going to need to update their scripts rather soon as a consequence of the Git migration :-). -- http://linuxfinances.info/info/linuxdistributions.html -- 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] Regression tests versus the buildfarm environment
Andrew Dunstan writes: > On 08/11/2010 04:47 PM, Tom Lane wrote: >> I prefer the change-the-default approach mainly because it wouldn't >> require any documentation, > Yeah. The other advantage is that we would not need to wait until we had > got everyone to update their versions of the buildfarm code. Um. That's actually a pretty darn strong point, considering how slow some buildfarm owners are to update the script :-( 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] Regression tests versus the buildfarm environment
On 08/11/2010 04:47 PM, Tom Lane wrote: Andrew Dunstan writes: Another way would be to have pg_regress honour an environment var like PG_REGRESS_PORT, which the buildfarm script could use. Yeah, that would work too. (Is it portable to Windows, though?) Should be I prefer the change-the-default approach mainly because it wouldn't require any documentation, whereas it'd be a bit hard to argue that environment variables etc shouldn't be documented ... Yeah. The other advantage is that we would not need to wait until we had got everyone to update their versions of the buildfarm code. So I agree this is the nicest solution. 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] Regression tests versus the buildfarm environment
Andrew Dunstan writes: > Another way would be to have pg_regress honour an environment var like > PG_REGRESS_PORT, which the buildfarm script could use. Yeah, that would work too. (Is it portable to Windows, though?) I prefer the change-the-default approach mainly because it wouldn't require any documentation, whereas it'd be a bit hard to argue that environment variables etc shouldn't be documented ... 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] Regression tests versus the buildfarm environment
On 08/11/2010 04:17 PM, Tom Lane wrote: We should have the buildfarm configuration such that any one run uses the same port number for both installed and uninstalled regression tests. If Peter is dead set on not changing pg_regress's default, then changing the makefiles to enable use of the --port switch is the way to do that. If you really want it to use the exact same port, then we'll probably need to change the Makefiles regardless of how we eventually decide to set the default. Another way would be to have pg_regress honour an environment var like PG_REGRESS_PORT, which the buildfarm script could use. 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] Regression tests versus the buildfarm environment
Andrew Dunstan writes: > You original email said: > For some historic reasons, I have my local scripts set up so that > they build development instances using the hardcoded port 65432. > I think my response would be "Don't do that". Yeah ... or at least use a different port per branch. Or make use of the ability to force pg_regress to use a nondefault port (which I still say we need to make accessible through "make check", whether or not the buildfarm does it that way). > Having said that, maybe we could reasonably use something like > DEF_PGPORT + 10 * major_version + minor_version in the calculation and > advise buildfarm members with multiple animals to keep their port ranges > say, 200 or more apart. I think that just makes it more prone to failure. We should have the buildfarm configuration such that any one run uses the same port number for both installed and uninstalled regression tests. If Peter is dead set on not changing pg_regress's default, then changing the makefiles to enable use of the --port switch is the way to do that. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Regression tests versus the buildfarm environment
On 08/11/2010 02:12 PM, Peter Eisentraut wrote: Even if you don't, changing this would only mean that you couldn't safely run "make check" concurrently in multiple branches. That's exactly the point. The original discussion is here: http://archives.postgresql.org/message-id/491d9935.9010...@gmx.net You original email said: For some historic reasons, I have my local scripts set up so that they build development instances using the hardcoded port 65432. I think my response would be "Don't do that". Having said that, maybe we could reasonably use something like DEF_PGPORT + 10 * major_version + minor_version in the calculation and advise buildfarm members with multiple animals to keep their port ranges say, 200 or more apart. But maybe we should just stick with my earlier advice :-) cheers andrew
Re: [HACKERS] Regression tests versus the buildfarm environment
On ons, 2010-08-11 at 11:47 -0400, Tom Lane wrote: > Peter Eisentraut writes: > > On ons, 2010-08-11 at 10:15 -0400, Tom Lane wrote: > >> How about just this: > >>port = 0xC000 | (DEF_PGPORT & 0x3FFF); > > > The version number was put in there intentionally, for developers who > > work on multiple branches at once. That's the whole reason this code > > exists. Please don't remove it. > > I work on multiple branches all day every day. This wouldn't hinder > me in the slightest, because I use a different DEF_PGPORT for each > branch. If you don't, it's hard to see how you manage to deal with > multiple branches on one machine ... do you not ever actually install > them? No, not nearly as much as I run "make check". > Even if you don't, changing this would only mean that you > couldn't safely run "make check" concurrently in multiple branches. That's exactly the point. The original discussion is here: http://archives.postgresql.org/message-id/491d9935.9010...@gmx.net -- 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] Regression tests versus the buildfarm environment
On ons, 2010-08-11 at 11:53 -0400, Andrew Dunstan wrote: > > The version number was put in there intentionally, for developers > who > > work on multiple branches at once. That's the whole reason this > code > > exists. Please don't remove it. > > > > Do they run "make check" by hand simultaneously on multiple branches? > That's the only way you'd get a collision here, I think. That is exactly what I'm talking about. -- 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] Regression tests versus the buildfarm environment
"Kevin Grittner" writes: > Peter Eisentraut wrote: >> Why not just compare pg_backend_pid() with postmaster.pid? > See the prior discussion in the archives. We started with that and > found problems, to which Tom suggested a random number as the best > solution. I think Peter's idea is a bit different though. The previous concern was about what information would be okay to expose in a pg_ping response packet, which presumably would be available to anybody who could open a connection to the postmaster port. What he's suggesting is to crosscheck against data that is available after a successful login. That eliminates the security complaint. It strikes me we could do something without adding a postmaster-PID SQL function, too. What about doing SHOW DATA_DIRECTORY and comparing that to what pg_regress expects? 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] Regression tests versus the buildfarm environment
On ons, 2010-08-11 at 11:48 -0400, Tom Lane wrote: > How's that help? pg_backend_pid isn't going to return the > postmaster's > PID ... maybe we could add a new function that does return the > postmaster's PID, though. Hmm, is there a portable way to find the parent PID of some other process, given its PID? -- 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] Regression tests versus the buildfarm environment
On 08/11/2010 11:42 AM, Peter Eisentraut wrote: On ons, 2010-08-11 at 10:15 -0400, Tom Lane wrote: One of us is missing something. I didn't say to run the checks using the configured port. I had in mind something like: port = 0xC000 | ((PG_VERSION_NUM + DEF_PGPORT)& 0x3FFF); Oh, I see, modify the DEF_PGPORT don't just use it as-is. OK, except that I think something like the above is still pretty risky for the buildfarm, because you would still have conflicts for assorted combinations of version numbers and branch_port settings. How about just this: port = 0xC000 | (DEF_PGPORT& 0x3FFF); The version number was put in there intentionally, for developers who work on multiple branches at once. That's the whole reason this code exists. Please don't remove it. Do they run "make check" by hand simultaneously on multiple branches? That's the only way you'd get a collision here, I think. 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] Regression tests versus the buildfarm environment
Peter Eisentraut wrote: > On ons, 2010-08-11 at 09:55 -0400, Tom Lane wrote: >> BTW, I don't know why anyone would think that "a random number" >> would offer any advantage here. I'd use the postmaster PID, >> which is guaranteed to be unique across the space that you're >> worried about. In fact, you could implement this off the >> existing postmaster.pid, no need for any new file. What's >> lacking is the pg_ping protocol. > > Why not just compare pg_backend_pid() with postmaster.pid? See the prior discussion in the archives. We started with that and found problems, to which Tom suggested a random number as the best solution. Let's at least start any further discussion informed by what's gone before; if there was a flaw in the reasoning, please point that out. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Regression tests versus the buildfarm environment
Peter Eisentraut writes: > On ons, 2010-08-11 at 10:15 -0400, Tom Lane wrote: >> How about just this: >> port = 0xC000 | (DEF_PGPORT & 0x3FFF); > The version number was put in there intentionally, for developers who > work on multiple branches at once. That's the whole reason this code > exists. Please don't remove it. I work on multiple branches all day every day. This wouldn't hinder me in the slightest, because I use a different DEF_PGPORT for each branch. If you don't, it's hard to see how you manage to deal with multiple branches on one machine ... do you not ever actually install them? Even if you don't, changing this would only mean that you couldn't safely run "make check" concurrently in multiple branches. 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] Regression tests versus the buildfarm environment
Peter Eisentraut writes: > On ons, 2010-08-11 at 09:55 -0400, Tom Lane wrote: >> BTW, I don't know why anyone would think that "a random number" would >> offer any advantage here. I'd use the postmaster PID, which is >> guaranteed to be unique across the space that you're worried about. >> In fact, you could implement this off the existing postmaster.pid, >> no need for any new file. What's lacking is the pg_ping protocol. > Why not just compare pg_backend_pid() with postmaster.pid? How's that help? pg_backend_pid isn't going to return the postmaster's PID ... maybe we could add a new function that does return the postmaster's PID, though. 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] Regression tests versus the buildfarm environment
On ons, 2010-08-11 at 09:55 -0400, Tom Lane wrote: > BTW, I don't know why anyone would think that "a random number" would > offer any advantage here. I'd use the postmaster PID, which is > guaranteed to be unique across the space that you're worried about. > In fact, you could implement this off the existing postmaster.pid, > no need for any new file. What's lacking is the pg_ping protocol. Why not just compare pg_backend_pid() with postmaster.pid? -- 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] Regression tests versus the buildfarm environment
On ons, 2010-08-11 at 10:15 -0400, Tom Lane wrote: > > One of us is missing something. I didn't say to run the checks using > the > > configured port. I had in mind something like: > > > port = 0xC000 | ((PG_VERSION_NUM + DEF_PGPORT) & 0x3FFF); > > Oh, I see, modify the DEF_PGPORT don't just use it as-is. OK, except > that I think something like the above is still pretty risky for the > buildfarm, because you would still have conflicts for assorted > combinations of version numbers and branch_port settings. > > How about just this: > > port = 0xC000 | (DEF_PGPORT & 0x3FFF); The version number was put in there intentionally, for developers who work on multiple branches at once. That's the whole reason this code exists. Please don't remove it. -- 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] Regression tests versus the buildfarm environment
On 08/11/2010 10:23 AM, Robert Haas wrote: Or we could do something like port = 0xC000 ^ (DEF_PGPORT& 0x7FFF); which is absolutely guaranteed not to conflict with DEF_PGPORT, at the cost of possibly shifting into the 32K-48K port number range if you had set DEF_PGPORT above 48K. I like XOR a lot better than OR. For years we told people to make sure they picked 4 digit port numbers for the buildfarm, and while I removed that note recently it can be put back. So I don't think there's much danger - let's got with XOR. 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] Regression tests versus the buildfarm environment
Robert Haas writes: > On Wed, Aug 11, 2010 at 10:15 AM, Tom Lane wrote: >> Or we could do something like >> >> port = 0xC000 ^ (DEF_PGPORT & 0x7FFF); >> >> which is absolutely guaranteed not to conflict with DEF_PGPORT, at the >> cost of possibly shifting into the 32K-48K port number range if you >> had set DEF_PGPORT above 48K. > I like XOR a lot better than OR. Yeah, on reflection that seems better. Barring objection I will see about making this happen in all live branches. 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] Regression tests versus the buildfarm environment
Vik Reykja writes: > We just put in the possibility to name the client connections. Would it be > interesting to be able to name the server installation itself? Wouldn't do anything for this problem --- it would just introduce something else the buildfarm would have to worry about uniqueness of. 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] Regression tests versus the buildfarm environment
On Wed, Aug 11, 2010 at 10:15 AM, Tom Lane wrote: > Andrew Dunstan writes: >> On 08/11/2010 09:43 AM, Tom Lane wrote: >>> Andrew Dunstan writes: Why not just add the configured port (DEF_PGPORT) into the calculation of the port to run on? > >>> No, that would be just about the worst possible choice. It'd be >>> guaranteed to fail in the standard scenario that you are running >>> "make check" before updating an existing installation. > >> One of us is missing something. I didn't say to run the checks using the >> configured port. I had in mind something like: > >> port = 0xC000 | ((PG_VERSION_NUM + DEF_PGPORT) & 0x3FFF); > > Oh, I see, modify the DEF_PGPORT don't just use it as-is. OK, except > that I think something like the above is still pretty risky for the > buildfarm, because you would still have conflicts for assorted > combinations of version numbers and branch_port settings. > > How about just this: > > port = 0xC000 | (DEF_PGPORT & 0x3FFF); > > If anyone was actually using a DEF_PGPORT above 0xC000, this would mean > that they couldn't run "make check" on the same machine as their running > installation (at least not without adjusting pg_regress's port choice, > which I still think we need to tweak the makefiles to make easier). > But for ordinary buildfarm usage, this would be guaranteed not to > conflict as long as you'd chosen nonconflicting branch_ports for all > your branches and animals. > > Or we could do something like > > port = 0xC000 ^ (DEF_PGPORT & 0x7FFF); > > which is absolutely guaranteed not to conflict with DEF_PGPORT, at the > cost of possibly shifting into the 32K-48K port number range if you > had set DEF_PGPORT above 48K. I like XOR a lot better than OR. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] Regression tests versus the buildfarm environment
Andrew Dunstan writes: > On 08/11/2010 09:43 AM, Tom Lane wrote: >> Andrew Dunstan writes: >>> Why not just add the configured port (DEF_PGPORT) into the calculation >>> of the port to run on? >> No, that would be just about the worst possible choice. It'd be >> guaranteed to fail in the standard scenario that you are running >> "make check" before updating an existing installation. > One of us is missing something. I didn't say to run the checks using the > configured port. I had in mind something like: > port = 0xC000 | ((PG_VERSION_NUM + DEF_PGPORT) & 0x3FFF); Oh, I see, modify the DEF_PGPORT don't just use it as-is. OK, except that I think something like the above is still pretty risky for the buildfarm, because you would still have conflicts for assorted combinations of version numbers and branch_port settings. How about just this: port = 0xC000 | (DEF_PGPORT & 0x3FFF); If anyone was actually using a DEF_PGPORT above 0xC000, this would mean that they couldn't run "make check" on the same machine as their running installation (at least not without adjusting pg_regress's port choice, which I still think we need to tweak the makefiles to make easier). But for ordinary buildfarm usage, this would be guaranteed not to conflict as long as you'd chosen nonconflicting branch_ports for all your branches and animals. Or we could do something like port = 0xC000 ^ (DEF_PGPORT & 0x7FFF); which is absolutely guaranteed not to conflict with DEF_PGPORT, at the cost of possibly shifting into the 32K-48K port number range if you had set DEF_PGPORT above 48K. 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] Regression tests versus the buildfarm environment
"Kevin Grittner" writes: > Tom Lane wrote: >> BTW, I don't know why anyone would think that "a random number" >> would offer any advantage here. I'd use the postmaster PID, which >> is guaranteed to be unique across the space that you're worried >> about. > Well, in the post I cited, it was you who argued that the PID was a > bad choice, suggested a random number, and stated "That would have a > substantially lower collision probability than PID, if the number > generation process were well designed; and it wouldn't risk exposing > anything sensitive in the ping response." Hmm. I don't remember why we'd think that the postmaster PID was sensitive information ... but if you take that as true, then yeah it couldn't be included in a pg_ping response. 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] Regression tests versus the buildfarm environment
Tom Lane wrote: > BTW, I don't know why anyone would think that "a random number" > would offer any advantage here. I'd use the postmaster PID, which > is guaranteed to be unique across the space that you're worried > about. Well, in the post I cited, it was you who argued that the PID was a bad choice, suggested a random number, and stated "That would have a substantially lower collision probability than PID, if the number generation process were well designed; and it wouldn't risk exposing anything sensitive in the ping response." -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Regression tests versus the buildfarm environment
On 08/11/2010 09:43 AM, Tom Lane wrote: Andrew Dunstan writes: On 08/11/2010 12:42 AM, Tom Lane wrote: ... However, it does seem like we ought to be able to do something about two buildfarm critters defaulting to the same choice of port number. Why not just add the configured port (DEF_PGPORT) into the calculation of the port to run on? No, that would be just about the worst possible choice. It'd be guaranteed to fail in the standard scenario that you are running "make check" before updating an existing installation. One of us is missing something. I didn't say to run the checks using the configured port. I had in mind something like: port = 0xC000 | ((PG_VERSION_NUM + DEF_PGPORT) & 0x3FFF); 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] Regression tests versus the buildfarm environment
"Kevin Grittner" writes: > Tom Lane wrote: >> A look at the code shows that it is merely trying to run psql, and >> if psql reports that it can connect to the specified port, then >> pg_regress thinks the postmaster started OK. Of course, psql was >> really reporting that it could connect to the other instance's >> postmaster. > Clearly picking unique ports for `make check` is the ultimate > solution, but I'm curious whether this would have been caught sooner > with less effort if the pg_ctl TODO titled "Have the postmaster > write a random number to a file on startup that pg_ctl checks > against the contents of a pg_ping response on its initial connection > (without login)" had been implemented. It would certainly make the failure more transparent. As I mentioned, there are previous buildfarm failures that look like they might be caused by a similar conflict, but it's seldom possible to be sure. A cross-check like that would be much safer. BTW, I don't know why anyone would think that "a random number" would offer any advantage here. I'd use the postmaster PID, which is guaranteed to be unique across the space that you're worried about. In fact, you could implement this off the existing postmaster.pid, no need for any new file. What's lacking is the pg_ping protocol. 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] Regression tests versus the buildfarm environment
Tom Lane wrote: > A look at the code shows that it is merely trying to run psql, and > if psql reports that it can connect to the specified port, then > pg_regress thinks the postmaster started OK. Of course, psql was > really reporting that it could connect to the other instance's > postmaster. Clearly picking unique ports for `make check` is the ultimate solution, but I'm curious whether this would have been caught sooner with less effort if the pg_ctl TODO titled "Have the postmaster write a random number to a file on startup that pg_ctl checks against the contents of a pg_ping response on its initial connection (without login)" had been implemented. http://archives.postgresql.org/pgsql-bugs/2009-10/msg00110.php It sounds like it's related; but was curious to confirm. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Regression tests versus the buildfarm environment
Andrew Dunstan writes: > On 08/11/2010 12:42 AM, Tom Lane wrote: >> ... However, it does seem like we ought to be able to >> do something about two buildfarm critters defaulting to the same choice >> of port number. > Why not just add the configured port (DEF_PGPORT) into the calculation > of the port to run on? No, that would be just about the worst possible choice. It'd be guaranteed to fail in the standard scenario that you are running "make check" before updating an existing installation. I think what we want to do here is arrange for the buildfarm script to select the same port that it's going to use later for an "installed" postmaster, but it has to go via a different path than DEF_PGPORT. The first thought that comes to mind is to adjust the makefiles like this: ifdef REGRESSION_TEST_PORT ... add --port $(REGRESSION_TEST_PORT) to pg_regress flags ... endif and then the buildfarm script could use make REGRESSION_TEST_PORT=nnn check But I'm not sure what the cleanest way is if we have to pass that down from the top-level makefile. Make doesn't pass down variables automatically does it? Another possibility is to allow a regression test port number to be configured via configure; though that seems like a slightly larger change than I'd want to push into the back branches. 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] Regression tests versus the buildfarm environment
On 08/11/2010 12:42 AM, Tom Lane wrote: There's an interesting buildfarm failure here: http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=polecat&dt=2010-08-10%2023:46:10 It appears to me that this was caused by the concurrent run of another buildfarm animal on the same physical machine, namely: http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=colugos&dt=2010-08-11%2000:02:58 Both animals are trying to test HEAD, which means that pg_regress defaults to the same postmaster port number in both builds: if (temp_install&& !port_specified_by_user) /* * To reduce chances of interference with parallel installations, use * a port number starting in the private range (49152-65535) * calculated from the version number. */ port = 0xC000 | (PG_VERSION_NUM& 0x3FFF); We observe colugos successfully starting on that port: == starting postmaster== running on port 57332 with pid 47019 == creating database "regression" == CREATE DATABASE ALTER DATABASE ... etc etc ... polecat comes along what must be only moments later, and tries to use the same port for its temp install: == starting postmaster== running on port 57332 with pid 47022 == creating database "regression" == ERROR: duplicate key value violates unique constraint "pg_database_datname_index" DETAIL: Key (datname)=(regression) already exists. command failed: "/usr/local/src/build-farm-3.2/builds/HEAD/pgsql.15278/src/test/regress/./tmp_check/install//usr/local/src/build-farm-3.2/builds/HEAD/inst/bin/psql" -X -c "CREATE DATABASE \"regression\" TEMPLATE=template0 ENCODING='SQL_ASCII' LC_COLLATE='C' LC_CTYPE='C'" "postgres" pg_ctl: PID file "/usr/local/src/build-farm-3.2/builds/HEAD/pgsql.15278/src/test/regress/./tmp_check/data/postmaster.pid" does not exist Is server running? pg_regress: could not stop postmaster: exit code was 256 Now the postmaster log shows that the second postmaster correctly recognized that the port number was already in use, so it bailed out: == pgsql.15278/src/test/regress/log/postmaster.log === [4c61f2d2.b7ae:1] FATAL: lock file "/tmp/.s.PGSQL.57332.lock" already exists [4c61f2d2.b7ae:2] HINT: Is another postmaster (PID 47019) using socket file "/tmp/.s.PGSQL.57332"? However, pg_regress failed to have a clue about what had happened, and bulled ahead trying to run the regression tests (against the postmaster started by the other pg_regress instance). A look at the code shows that it is merely trying to run psql, and if psql reports that it can connect to the specified port, then pg_regress thinks the postmaster started OK. Of course, psql was really reporting that it could connect to the other instance's postmaster. I've seen similar multiple-postmaster-interference symptoms before in the buildfarm, but never really understood the cause. I am not sure if there's anything very good we can do about the problem of pg_regress misidentifying the postmaster it's managed to connect to. A real solution would probably be much more trouble than it's worth, anyway. However, it does seem like we ought to be able to do something about two buildfarm critters defaulting to the same choice of port number. The buildfarm infrastructure goes to great lengths to pick nonconflicting port numbers for the "installed" postmasters it runs; but we're ignoring all that effort and just using a hardwired port number for "make check". This is dumb. pg_regress does have a --port argument that can be used to override that default. I don't know whether the buildfarm script calls pg_regress directly or does "make check". If the latter, we'd need to twiddle the Makefiles to allow a port number to get passed in. But this seems well worthwhile to me. Comments? The buildfarm calls "make check". Why not just add the configured port (DEF_PGPORT) into the calculation of the port to run on? 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] Regression tests versus the buildfarm environment
On Wed, Aug 11, 2010 at 06:42, Tom Lane wrote: > I am not sure if there's anything very good we can do about the > problem of pg_regress misidentifying the postmaster it's managed to > connect to. A real solution would probably be much more trouble than > it's worth, anyway. However, it does seem like we ought to be able to > do something about two buildfarm critters defaulting to the same choice > of port number. The buildfarm infrastructure goes to great lengths to > pick nonconflicting port numbers for the "installed" postmasters it > runs; but we're ignoring all that effort and just using a hardwired > port number for "make check". This is dumb. > > pg_regress does have a --port argument that can be used to override > that default. I don't know whether the buildfarm script calls > pg_regress directly or does "make check". If the latter, we'd need to > twiddle the Makefiles to allow a port number to get passed in. But > this seems well worthwhile to me. > > Comments? > We just put in the possibility to name the client connections. Would it be interesting to be able to name the server installation itself?
[HACKERS] Regression tests versus the buildfarm environment
There's an interesting buildfarm failure here: http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=polecat&dt=2010-08-10%2023:46:10 It appears to me that this was caused by the concurrent run of another buildfarm animal on the same physical machine, namely: http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=colugos&dt=2010-08-11%2000:02:58 Both animals are trying to test HEAD, which means that pg_regress defaults to the same postmaster port number in both builds: if (temp_install && !port_specified_by_user) /* * To reduce chances of interference with parallel installations, use * a port number starting in the private range (49152-65535) * calculated from the version number. */ port = 0xC000 | (PG_VERSION_NUM & 0x3FFF); We observe colugos successfully starting on that port: == starting postmaster== running on port 57332 with pid 47019 == creating database "regression" == CREATE DATABASE ALTER DATABASE ... etc etc ... polecat comes along what must be only moments later, and tries to use the same port for its temp install: == starting postmaster== running on port 57332 with pid 47022 == creating database "regression" == ERROR: duplicate key value violates unique constraint "pg_database_datname_index" DETAIL: Key (datname)=(regression) already exists. command failed: "/usr/local/src/build-farm-3.2/builds/HEAD/pgsql.15278/src/test/regress/./tmp_check/install//usr/local/src/build-farm-3.2/builds/HEAD/inst/bin/psql" -X -c "CREATE DATABASE \"regression\" TEMPLATE=template0 ENCODING='SQL_ASCII' LC_COLLATE='C' LC_CTYPE='C'" "postgres" pg_ctl: PID file "/usr/local/src/build-farm-3.2/builds/HEAD/pgsql.15278/src/test/regress/./tmp_check/data/postmaster.pid" does not exist Is server running? pg_regress: could not stop postmaster: exit code was 256 Now the postmaster log shows that the second postmaster correctly recognized that the port number was already in use, so it bailed out: == pgsql.15278/src/test/regress/log/postmaster.log === [4c61f2d2.b7ae:1] FATAL: lock file "/tmp/.s.PGSQL.57332.lock" already exists [4c61f2d2.b7ae:2] HINT: Is another postmaster (PID 47019) using socket file "/tmp/.s.PGSQL.57332"? However, pg_regress failed to have a clue about what had happened, and bulled ahead trying to run the regression tests (against the postmaster started by the other pg_regress instance). A look at the code shows that it is merely trying to run psql, and if psql reports that it can connect to the specified port, then pg_regress thinks the postmaster started OK. Of course, psql was really reporting that it could connect to the other instance's postmaster. I've seen similar multiple-postmaster-interference symptoms before in the buildfarm, but never really understood the cause. I am not sure if there's anything very good we can do about the problem of pg_regress misidentifying the postmaster it's managed to connect to. A real solution would probably be much more trouble than it's worth, anyway. However, it does seem like we ought to be able to do something about two buildfarm critters defaulting to the same choice of port number. The buildfarm infrastructure goes to great lengths to pick nonconflicting port numbers for the "installed" postmasters it runs; but we're ignoring all that effort and just using a hardwired port number for "make check". This is dumb. pg_regress does have a --port argument that can be used to override that default. I don't know whether the buildfarm script calls pg_regress directly or does "make check". If the latter, we'd need to twiddle the Makefiles to allow a port number to get passed in. But this seems well worthwhile to me. Comments? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] regression tests of dictionaries and Windows
Correct, that fixes the problem. I've verified it on my machine, and
committed the patch.
//Magnus
On Wed, Sep 12, 2007 at 04:32:57PM +0400, Teodor Sigaev wrote:
> Do you mean something like this:
> ./src/tools/msvc/Install.pm
> *** ./src/tools/msvc/Install.pm.origWed Sep 12 16:30:25 2007
> --- ./src/tools/msvc/Install.pm Wed Sep 12 16:31:29 2007
> ***
> *** 66,71
> --- 66,72
> GenerateTimezoneFiles($target,$conf);
> GenerateTsearchFiles($target);
> CopySetOfFiles('Stopword files',
> "src\\backend\\snowball\\stopwords\\*.stop", $target .
> '/share/tsearch_data/');
> + CopySetOfFiles('Dictionaries sample files',
> "src\\backend\\tsearch\\\*_sample.*", $target . '/share/tsearch_data/');
> CopyContribFiles($config,$target);
> CopyIncludeFiles($target);
>
>
> >The MSVC build process doesn't use make and friends. The required magic
> >has to go in src/tools/msvc/Install.pm. I will look at it later today.
>
> --
> Teodor Sigaev E-mail: [EMAIL PROTECTED]
>WWW:
>http://www.sigaev.ru/
>
> ---(end of broadcast)---
> TIP 3: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/docs/faq
---(end of broadcast)---
TIP 6: explain analyze is your friend
Re: [HACKERS] regression tests of dictionaries and Windows
Do you mean something like this:
./src/tools/msvc/Install.pm
*** ./src/tools/msvc/Install.pm.origWed Sep 12 16:30:25 2007
--- ./src/tools/msvc/Install.pm Wed Sep 12 16:31:29 2007
***
*** 66,71
--- 66,72
GenerateTimezoneFiles($target,$conf);
GenerateTsearchFiles($target);
CopySetOfFiles('Stopword files',
"src\\backend\\snowball\\stopwords\\*.stop", $target . '/share/tsearch_data/');
+ CopySetOfFiles('Dictionaries sample files',
"src\\backend\\tsearch\\\*_sample.*", $target . '/share/tsearch_data/');
CopyContribFiles($config,$target);
CopyIncludeFiles($target);
The MSVC build process doesn't use make and friends. The required magic
has to go in src/tools/msvc/Install.pm. I will look at it later today.
--
Teodor Sigaev E-mail: [EMAIL PROTECTED]
WWW: http://www.sigaev.ru/
---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?
http://www.postgresql.org/docs/faq
Re: [HACKERS] regression tests of dictionaries and Windows
Teodor Sigaev wrote: All windows boxes are failed on tsdicts test: http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=mastodon&dt=2007-09-12%2007:00:00 http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=skylark&dt=2007-09-12%2003:00:01 http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=baiji&dt=2007-09-11%2022:00:01 with the same error: + ERROR: could not open dictionary file "C:/pgBuild/BuildFarm/BuildRoot/HEAD/pgsql.3204/src/test/regress/./tmp_check/install/share/tsearch_data/ispell_sample.dict": No such file or directory Does anybody know a needed magic to fix that? It's not all Windows boxes, only those building with MSVC. Mingw and Cygwin builds are working fine. The MSVC build process doesn't use make and friends. The required magic has to go in src/tools/msvc/Install.pm. I will look at it later today. cheers andrew ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
[HACKERS] regression tests of dictionaries and Windows
All windows boxes are failed on tsdicts test: http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=mastodon&dt=2007-09-12%2007:00:00 http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=skylark&dt=2007-09-12%2003:00:01 http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=baiji&dt=2007-09-11%2022:00:01 with the same error: + ERROR: could not open dictionary file "C:/pgBuild/BuildFarm/BuildRoot/HEAD/pgsql.3204/src/test/regress/./tmp_check/install/share/tsearch_data/ispell_sample.dict": No such file or directory Does anybody know a needed magic to fix that? -- Teodor Sigaev E-mail: [EMAIL PROTECTED] WWW: http://www.sigaev.ru/ ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] Regression tests
On Wednesday 04 May 2005 03:20, Christopher Kings-Lynne wrote:
> > Quite, but in the meantime, a good benchmark should stress the system
> > enough to cause crashes, lockups or at least incorrect results if a
> > bug is introduced in the shared memory or semaphore code, and will
> > definitely reveal any slowdowns introduced by new code, so my question
> > is: where can I find a good benchmark for PostgreSQL? Note that I
> > don't care about comparing PostgreSQL to other RDBMSes; I just want to
> > a) test PostgreSQL under high concurrent load and b) if possible,
> > measure the performance impact of a patch.
>
> You can use contrib/pgbench as a rather simplistic test, or you could
> try OSDB (http://osdb.sourceforge.net/)
>
Shouldn't we be getting some of this testing out of the work being done by
Mark Wong and the OSDN folks?
--
Robert Treat
Build A Brighter Lamp :: Linux Apache {middleware} PostgreSQL
---(end of broadcast)---
TIP 4: Don't 'kill -9' the postmaster
Re: [HACKERS] Regression tests
Christopher Kings-Lynne <[EMAIL PROTECTED]> writes: >> Quite, but in the meantime, a good benchmark should stress the system >> enough to cause crashes, lockups or at least incorrect results if a >> bug is introduced in the shared memory or semaphore code, and will >> definitely reveal any slowdowns introduced by new code, so my question >> is: where can I find a good benchmark for PostgreSQL? Note that I >> don't care about comparing PostgreSQL to other RDBMSes; I just want to >> a) test PostgreSQL under high concurrent load and b) if possible, >> measure the performance impact of a patch. > You can use contrib/pgbench as a rather simplistic test, or you could > try OSDB (http://osdb.sourceforge.net/) For something a little tougher, talk to Mark Wong about using OSDL's testbed. regards, tom lane ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [HACKERS] Regression tests
Quite, but in the meantime, a good benchmark should stress the system enough to cause crashes, lockups or at least incorrect results if a bug is introduced in the shared memory or semaphore code, and will definitely reveal any slowdowns introduced by new code, so my question is: where can I find a good benchmark for PostgreSQL? Note that I don't care about comparing PostgreSQL to other RDBMSes; I just want to a) test PostgreSQL under high concurrent load and b) if possible, measure the performance impact of a patch. You can use contrib/pgbench as a rather simplistic test, or you could try OSDB (http://osdb.sourceforge.net/) Chris ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] Regression tests
Neil Conway <[EMAIL PROTECTED]> writes: > Dag-Erling Smørgrav wrote: > > It doesn't stress the system anywhere near enough to reveal bugs in, > > say, the shared memory or semaphore code. > I agree -- I think we definitely need more tests for the concurrent > behavior of the system. Quite, but in the meantime, a good benchmark should stress the system enough to cause crashes, lockups or at least incorrect results if a bug is introduced in the shared memory or semaphore code, and will definitely reveal any slowdowns introduced by new code, so my question is: where can I find a good benchmark for PostgreSQL? Note that I don't care about comparing PostgreSQL to other RDBMSes; I just want to a) test PostgreSQL under high concurrent load and b) if possible, measure the performance impact of a patch. DES -- Dag-Erling Smørgrav - [EMAIL PROTECTED] ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [HACKERS] Regression tests
On Wed, 2005-05-04 at 10:56 +1000, Neil Conway wrote: > Dag-Erling SmÃrgrav wrote: > > It doesn't stress the system anywhere near enough to reveal bugs in, > > say, the shared memory or semaphore code. > > I agree -- I think we definitely need more tests for the concurrent > behavior of the system. > Yes, very much agree. Contributions in that area would be most welcome. Best Regards, Simon Riggs ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [HACKERS] Regression tests
Dag-Erling Smørgrav wrote: It doesn't stress the system anywhere near enough to reveal bugs in, say, the shared memory or semaphore code. I agree -- I think we definitely need more tests for the concurrent behavior of the system. -Neil ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] Regression tests
Christopher Kings-Lynne <[EMAIL PROTECTED]> writes: > The whole point of make check is to check correctness, not > performance. I understand that. > It has concurrent loading as well. It doesn't stress the system anywhere near enough to reveal bugs in, say, the shared memory or semaphore code. DES -- Dag-Erling Smørgrav - [EMAIL PROTECTED] ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
