pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
SKIP_LOCKED test causes random buildfarm failures
11+ messages / 3 participants
[nested] [flat]

Commits from this thread:
  2019-11-16 3db0598d90 bbaa38e824  Improve stability of tests for VACUUM (SKIP_LOCKED)
* SKIP_LOCKED test causes random buildfarm failures
@ 2019-11-06 21:54 Tom Lane <tgl@sss.pgh.pa.us>
  2019-11-06 23:01 ` Re: SKIP_LOCKED test causes random buildfarm failures Andres Freund <andres@anarazel.de>
  0 siblings, 1 reply; 11+ messages in thread

From: Tom Lane @ 2019-11-06 21:54 UTC (permalink / raw)
  To: pgsql-hackers@lists.postgresql.org

Every once in awhile we get failures like this one:

https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=gull&dt=2019-11-05%2008%3A27%3A27

inline patch 1: /home/pgsql/build-farm/buildroot-clang/HEAD/pgsql.build/src/test/regress/results/vacuum.out (+1/-0 lines) download inline-1.patchdiff -U3 /home/pgsql/build-farm/buildroot-clang/HEAD/pgsql.build/../pgsql/src/test/regress/expected/vacuum.out /home/pgsql/build-farm/buildroot-clang/HEAD/pgsql.build/src/test/regress/results/vacuum.out --- /home/pgsql/build-farm/buildroot-clang/HEAD/pgsql.build/../pgsql/src/test/regress/expected/vacuum.out 2019-08-11 03:02:18.921535948 -0700 +++ /home/pgsql/build-farm/buildroot-clang/HEAD/pgsql.build/src/test/regress/results/vacuum.out 2019-11-05 00:50:42.381244885 -0800 @@ -204,6 +204,7 @@ -- SKIP_LOCKED option VACUUM (SKIP_LOCKED) vactst; VACUUM (SKIP_LOCKED, FULL) vactst; +WARNING: skipping vacuum of "vactst" --- lock not available ANALYZE (SKIP_LOCKED) vactst; -- ensure VACUUM and ANALYZE don't have a problem with serializable SET default_transaction_isolation = serializable;
No doubt this is a conflict with autovacuum. There are two reasonable ways to remove the test instability: * Crank up client_min_messages to more than WARNING for this test stanza. * Downgrade the "skipping" messages to DEBUG1 or less. I kind of wonder why we are issuing a "WARNING" when the statement does exactly what you asked it to, anyway. At most I'd expect that to be a NOTICE condition. regards, tom lane ^ permalink raw reply [nested|flat] 11+ messages in thread

* Re: SKIP_LOCKED test causes random buildfarm failures
  2019-11-06 21:54 SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
@ 2019-11-06 23:01 ` Andres Freund <andres@anarazel.de>
  2019-11-07 01:39   ` Re: SKIP_LOCKED test causes random buildfarm failures Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 11+ messages in thread

From: Andres Freund @ 2019-11-06 23:01 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers@lists.postgresql.org

Hi,

On 2019-11-06 16:54:38 -0500, Tom Lane wrote:
> Every once in awhile we get failures like this one:
> 
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=gull&dt=2019-11-05%2008%3A27%3A27
> 
> diff -U3 /home/pgsql/build-farm/buildroot-clang/HEAD/pgsql.build/../pgsql/src/test/regress/expected/vacuum.out /home/pgsql/build-farm/buildroot-clang/HEAD/pgsql.build/src/test/regress/results/vacuum.out
> --- /home/pgsql/build-farm/buildroot-clang/HEAD/pgsql.build/../pgsql/src/test/regress/expected/vacuum.out	2019-08-11 03:02:18.921535948 -0700
> +++ /home/pgsql/build-farm/buildroot-clang/HEAD/pgsql.build/src/test/regress/results/vacuum.out	2019-11-05 00:50:42.381244885 -0800
> @@ -204,6 +204,7 @@
>  -- SKIP_LOCKED option
>  VACUUM (SKIP_LOCKED) vactst;
>  VACUUM (SKIP_LOCKED, FULL) vactst;
> +WARNING:  skipping vacuum of "vactst" --- lock not available
>  ANALYZE (SKIP_LOCKED) vactst;
>  -- ensure VACUUM and ANALYZE don't have a problem with serializable
>  SET default_transaction_isolation = serializable;
> 
> 
> No doubt this is a conflict with autovacuum.  There are two reasonable
> ways to remove the test instability:

I assume you consider disabling autovacuum for that table not a
reasonable approach? Due to the danger that it could end up still
running, e.g. due to anti-wraparound or such?


> * Crank up client_min_messages to more than WARNING for this test
> stanza.

Hm.


> * Downgrade the "skipping" messages to DEBUG1 or less.
> 
> I kind of wonder why we are issuing a "WARNING" when the statement
> does exactly what you asked it to, anyway.  At most I'd expect
> that to be a NOTICE condition.

I don't know what lead us to doing so, but it doesn't seem reasonable to
allow the user to see whether the table has actually been vacuumed. I
would assume that one uses SKIP_LOCKED partially to avoid unnecessary
impacts in production due to other tasks starting to block on e.g. a
VACUUM FULL, even though without the "ordered queueing" everything could
just go on working fine. I'm not sure that indicates whether WARNING or
NOTICE is the best choice.

So I'd be inclined to go with the client_min_messages approach?


Greetings,

Andres Freund





^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: SKIP_LOCKED test causes random buildfarm failures
  2019-11-06 21:54 SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
  2019-11-06 23:01 ` Re: SKIP_LOCKED test causes random buildfarm failures Andres Freund <andres@anarazel.de>
@ 2019-11-07 01:39   ` Michael Paquier <michael@paquier.xyz>
  2019-11-07 16:50     ` Re: SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 11+ messages in thread

From: Michael Paquier @ 2019-11-07 01:39 UTC (permalink / raw)
  To: Andres Freund <andres@anarazel.de>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-hackers@lists.postgresql.org

On Wed, Nov 06, 2019 at 03:01:11PM -0800, Andres Freund wrote:
> I don't know what lead us to doing so, but it doesn't seem reasonable to
> allow the user to see whether the table has actually been vacuumed. I
> would assume that one uses SKIP_LOCKED partially to avoid unnecessary
> impacts in production due to other tasks starting to block on e.g. a
> VACUUM FULL, even though without the "ordered queueing" everything could
> just go on working fine. I'm not sure that indicates whether WARNING or
> NOTICE is the best choice.

Good question.  That's a historical choice, still I have seen cases
where those warnings are helpful while not making the logs too
verbose to see some congestion in the jobs.

> So I'd be inclined to go with the client_min_messages approach?

The main purpose of the tests in regress/ is to check after the
grammar, so using client_min_messages sounds like a plan.  We have
a second set of tests in isolation/ where I would actually like to
disable autovacuum by default on a subset of tables.  Thoughts about
the attached?
--
Michael

Attachments:

vacuum-lock-tests.patch (text/x-diff, 2.5K) download diff --git a/src/test/isolation/specs/vacuum-skip-locked.spec b/src/test/isolation/specs/vacuum-skip-locked.spec index 4d59b294ca..4f749e74b6 100644 --- a/src/test/isolation/specs/vacuum-skip-locked.spec +++ b/src/test/isolation/specs/vacuum-skip-locked.spec @@ -7,7 +7,9 @@ setup { CREATE TABLE parted (a INT) PARTITION BY LIST (a); CREATE TABLE part1 PARTITION OF parted FOR VALUES IN (1); + ALTER TABLE part1 SET (autovacuum_enabled = false); CREATE TABLE part2 PARTITION OF parted FOR VALUES IN (2); + ALTER TABLE part2 SET (autovacuum_enabled = false); } teardown diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out index aff0b10a93..9996d882d1 100644 --- a/src/test/regress/expected/vacuum.out +++ b/src/test/regress/expected/vacuum.out @@ -196,7 +196,9 @@ ERROR: unrecognized ANALYZE option "nonexistentarg" LINE 1: ANALYZE (nonexistentarg) does_not_exit; ^ -- ensure argument order independence, and that SKIP_LOCKED on non-existing --- relation still errors out. +-- relation still errors out. Suppress WARNING messages caused by concurrent +-- autovacuums. +SET client_min_messages TO 'ERROR'; ANALYZE (SKIP_LOCKED, VERBOSE) does_not_exist; ERROR: relation "does_not_exist" does not exist ANALYZE (VERBOSE, SKIP_LOCKED) does_not_exist; @@ -205,6 +207,7 @@ ERROR: relation "does_not_exist" does not exist VACUUM (SKIP_LOCKED) vactst; VACUUM (SKIP_LOCKED, FULL) vactst; ANALYZE (SKIP_LOCKED) vactst; +RESET client_min_messages; -- ensure VACUUM and ANALYZE don't have a problem with serializable SET default_transaction_isolation = serializable; VACUUM vactst; diff --git a/src/test/regress/sql/vacuum.sql b/src/test/regress/sql/vacuum.sql index f0fee3af2b..69987f75e9 100644 --- a/src/test/regress/sql/vacuum.sql +++ b/src/test/regress/sql/vacuum.sql @@ -158,7 +158,9 @@ ANALYZE (nonexistent-arg) does_not_exist; ANALYZE (nonexistentarg) does_not_exit; -- ensure argument order independence, and that SKIP_LOCKED on non-existing --- relation still errors out. +-- relation still errors out. Suppress WARNING messages caused by concurrent +-- autovacuums. +SET client_min_messages TO 'ERROR'; ANALYZE (SKIP_LOCKED, VERBOSE) does_not_exist; ANALYZE (VERBOSE, SKIP_LOCKED) does_not_exist; @@ -166,6 +168,7 @@ ANALYZE (VERBOSE, SKIP_LOCKED) does_not_exist; VACUUM (SKIP_LOCKED) vactst; VACUUM (SKIP_LOCKED, FULL) vactst; ANALYZE (SKIP_LOCKED) vactst; +RESET client_min_messages; -- ensure VACUUM and ANALYZE don't have a problem with serializable SET default_transaction_isolation = serializable;
signature.asc (application/pgp-signature, 832B) download ^ permalink raw reply [nested|flat] 11+ messages in thread

* Re: SKIP_LOCKED test causes random buildfarm failures
  2019-11-06 21:54 SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
  2019-11-06 23:01 ` Re: SKIP_LOCKED test causes random buildfarm failures Andres Freund <andres@anarazel.de>
  2019-11-07 01:39   ` Re: SKIP_LOCKED test causes random buildfarm failures Michael Paquier <michael@paquier.xyz>
@ 2019-11-07 16:50     ` Tom Lane <tgl@sss.pgh.pa.us>
  2019-11-08 00:01       ` Re: SKIP_LOCKED test causes random buildfarm failures Michael Paquier <michael@paquier.xyz>
  2019-11-14 18:37       ` Re: SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 2 replies; 11+ messages in thread

From: Tom Lane @ 2019-11-07 16:50 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-hackers@lists.postgresql.org

Michael Paquier <michael@paquier.xyz> writes:
> On Wed, Nov 06, 2019 at 03:01:11PM -0800, Andres Freund wrote:
>> I don't know what lead us to doing so, but it doesn't seem reasonable to
>> allow the user to see whether the table has actually been vacuumed. I
>> would assume that one uses SKIP_LOCKED partially to avoid unnecessary
>> impacts in production due to other tasks starting to block on e.g. a
>> VACUUM FULL, even though without the "ordered queueing" everything could
>> just go on working fine. I'm not sure that indicates whether WARNING or
>> NOTICE is the best choice.

> Good question.  That's a historical choice, still I have seen cases
> where those warnings are helpful while not making the logs too
> verbose to see some congestion in the jobs.

I kind of feel that NOTICE is more semantically appropriate, but
perhaps there's an argument for keeping it at WARNING.

>> So I'd be inclined to go with the client_min_messages approach?

> The main purpose of the tests in regress/ is to check after the
> grammar, so using client_min_messages sounds like a plan.  We have
> a second set of tests in isolation/ where I would actually like to
> disable autovacuum by default on a subset of tables.  Thoughts about
> the attached?

I do not want to fix this in the main tests by disabling autovacuum,
because that'd actually reduce the tests' cross-section.  The fact
that this happens occasionally is a Good Thing for verifying that the
code paths actually work.  So it seems that there's a consensus on
adjusting client_min_messages to fix the test output instability ---
but we need to agree on whether to do s/WARNING/NOTICE/ first, so we
can know what to set client_min_messages to.

As for the case in the isolation test, shouldn't we also use
client_min_messages there, rather than prevent the conflict
from arising?  Or would that test fail in some larger way if
autovacuum gets in the way?

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: SKIP_LOCKED test causes random buildfarm failures
  2019-11-06 21:54 SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
  2019-11-06 23:01 ` Re: SKIP_LOCKED test causes random buildfarm failures Andres Freund <andres@anarazel.de>
  2019-11-07 01:39   ` Re: SKIP_LOCKED test causes random buildfarm failures Michael Paquier <michael@paquier.xyz>
  2019-11-07 16:50     ` Re: SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
@ 2019-11-08 00:01       ` Michael Paquier <michael@paquier.xyz>
  1 sibling, 0 replies; 11+ messages in thread

From: Michael Paquier @ 2019-11-08 00:01 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-hackers@lists.postgresql.org

On Thu, Nov 07, 2019 at 11:50:25AM -0500, Tom Lane wrote:
> Michael Paquier <michael@paquier.xyz> writes:
>> Good question.  That's a historical choice, still I have seen cases
>> where those warnings are helpful while not making the logs too
>> verbose to see some congestion in the jobs.
> 
> I kind of feel that NOTICE is more semantically appropriate, but
> perhaps there's an argument for keeping it at WARNING.

Perhaps.  Well, that's the same level as the one used after the
permission checks on the relation vacuumed.

> I do not want to fix this in the main tests by disabling autovacuum,
> because that'd actually reduce the tests' cross-section.  The fact
> that this happens occasionally is a Good Thing for verifying that the
> code paths actually work.  So it seems that there's a consensus on
> adjusting client_min_messages to fix the test output instability ---
> but we need to agree on whether to do s/WARNING/NOTICE/ first, so we
> can know what to set client_min_messages to.

Makes sense. 

> As for the case in the isolation test, shouldn't we also use
> client_min_messages there, rather than prevent the conflict
> from arising?  Or would that test fail in some larger way if
> autovacuum gets in the way?

I think that there is no risk regarding the stability of the output
because we use LOCK before from a first session on the relation to
vacuum in a second session.  So if autovacuum runs in parallel, the
consequence would be a small slow down while waiting on the lock to be
taken.  And per the way the test is ordered, it seems to me that it
makes the most sense to disable autovacuum as it would just get in the
way.  In this case I think that it is actually better to show the
messages as that makes the tests more verbose and we make sure to test
their format, even if we could just rely on the fact that VACUUM
should just be blocking or non-blocking.
--
Michael

Attachments:

  signature.asc (application/pgp-signature, 832B) download

^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: SKIP_LOCKED test causes random buildfarm failures
  2019-11-06 21:54 SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
  2019-11-06 23:01 ` Re: SKIP_LOCKED test causes random buildfarm failures Andres Freund <andres@anarazel.de>
  2019-11-07 01:39   ` Re: SKIP_LOCKED test causes random buildfarm failures Michael Paquier <michael@paquier.xyz>
  2019-11-07 16:50     ` Re: SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
@ 2019-11-14 18:37       ` Tom Lane <tgl@sss.pgh.pa.us>
  2019-11-14 20:16         ` Re: SKIP_LOCKED test causes random buildfarm failures Andres Freund <andres@anarazel.de>
  1 sibling, 1 reply; 11+ messages in thread

From: Tom Lane @ 2019-11-14 18:37 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-hackers@lists.postgresql.org

I wrote:
> Michael Paquier <michael@paquier.xyz> writes:
>> Good question.  That's a historical choice, still I have seen cases
>> where those warnings are helpful while not making the logs too
>> verbose to see some congestion in the jobs.

> I kind of feel that NOTICE is more semantically appropriate, but
> perhaps there's an argument for keeping it at WARNING.

Well, I'm not hearing any groundswell of support for changing the
message level, so let's leave that as-is and just see about
stabilizing the tests.

>> The main purpose of the tests in regress/ is to check after the
>> grammar, so using client_min_messages sounds like a plan.  We have
>> a second set of tests in isolation/ where I would actually like to
>> disable autovacuum by default on a subset of tables.  Thoughts about
>> the attached?

After looking more closely at the isolation test, I agree that adding
the "ALTER TABLE ... SET (autovacuum_enabled = false)" bits to it is
a good idea.  The LOCK operations should make that irrelevant for
part1, but there's at least a theoretical hazard for part2.
(Actually, is "autovacuum_enabled = false" really sufficient to
keep autovacuum away?  It'd probably lock the table for long enough
to examine its reloptions, so it seems like all we're doing here is
reducing the window for trouble a little bit.  Still, maybe that's
worthwhile.)

As for the SKIP_LOCKED tests in vacuum.sql, what I now propose is that
we just remove them.  I do not see that they're offering any coverage
that's not completely redundant with this isolation test.  We don't
need to spend cycles every day on that.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: SKIP_LOCKED test causes random buildfarm failures
  2019-11-06 21:54 SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
  2019-11-06 23:01 ` Re: SKIP_LOCKED test causes random buildfarm failures Andres Freund <andres@anarazel.de>
  2019-11-07 01:39   ` Re: SKIP_LOCKED test causes random buildfarm failures Michael Paquier <michael@paquier.xyz>
  2019-11-07 16:50     ` Re: SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
  2019-11-14 18:37       ` Re: SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
@ 2019-11-14 20:16         ` Andres Freund <andres@anarazel.de>
  2019-11-14 20:20           ` Re: SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 11+ messages in thread

From: Andres Freund @ 2019-11-14 20:16 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Michael Paquier <michael@paquier.xyz>; pgsql-hackers@lists.postgresql.org

Hi,

On 2019-11-14 13:37:44 -0500, Tom Lane wrote:
> I wrote:
> > Michael Paquier <michael@paquier.xyz> writes:
> >> Good question.  That's a historical choice, still I have seen cases
> >> where those warnings are helpful while not making the logs too
> >> verbose to see some congestion in the jobs.
> 
> > I kind of feel that NOTICE is more semantically appropriate, but
> > perhaps there's an argument for keeping it at WARNING.
> 
> Well, I'm not hearing any groundswell of support for changing the
> message level, so let's leave that as-is and just see about
> stabilizing the tests.

Ok.


> >> The main purpose of the tests in regress/ is to check after the
> >> grammar, so using client_min_messages sounds like a plan.  We have
> >> a second set of tests in isolation/ where I would actually like to
> >> disable autovacuum by default on a subset of tables.  Thoughts about
> >> the attached?
> 
> After looking more closely at the isolation test, I agree that adding
> the "ALTER TABLE ... SET (autovacuum_enabled = false)" bits to it is
> a good idea.  The LOCK operations should make that irrelevant for
> part1, but there's at least a theoretical hazard for part2.
> (Actually, is "autovacuum_enabled = false" really sufficient to
> keep autovacuum away?  It'd probably lock the table for long enough
> to examine its reloptions, so it seems like all we're doing here is
> reducing the window for trouble a little bit.  Still, maybe that's
> worthwhile.)

+1


> As for the SKIP_LOCKED tests in vacuum.sql, what I now propose is that
> we just remove them.  I do not see that they're offering any coverage
> that's not completely redundant with this isolation test.  We don't
> need to spend cycles every day on that.

-0 on that, I'd rather just put a autovacuum_enabled = false for
them. They're quick enough, and it's nice to have decent coverage of
various options within the plain regression tests when possible.

Greetings,

Andres Freund





^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: SKIP_LOCKED test causes random buildfarm failures
  2019-11-06 21:54 SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
  2019-11-06 23:01 ` Re: SKIP_LOCKED test causes random buildfarm failures Andres Freund <andres@anarazel.de>
  2019-11-07 01:39   ` Re: SKIP_LOCKED test causes random buildfarm failures Michael Paquier <michael@paquier.xyz>
  2019-11-07 16:50     ` Re: SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
  2019-11-14 18:37       ` Re: SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
  2019-11-14 20:16         ` Re: SKIP_LOCKED test causes random buildfarm failures Andres Freund <andres@anarazel.de>
@ 2019-11-14 20:20           ` Tom Lane <tgl@sss.pgh.pa.us>
  2019-11-15 01:19             ` Re: SKIP_LOCKED test causes random buildfarm failures Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 11+ messages in thread

From: Tom Lane @ 2019-11-14 20:20 UTC (permalink / raw)
  To: Andres Freund <andres@anarazel.de>; +Cc: Michael Paquier <michael@paquier.xyz>; pgsql-hackers@lists.postgresql.org

Andres Freund <andres@anarazel.de> writes:
> On 2019-11-14 13:37:44 -0500, Tom Lane wrote:
>> As for the SKIP_LOCKED tests in vacuum.sql, what I now propose is that
>> we just remove them.  I do not see that they're offering any coverage
>> that's not completely redundant with this isolation test.  We don't
>> need to spend cycles every day on that.

> -0 on that, I'd rather just put a autovacuum_enabled = false for
> them. They're quick enough, and it's nice to have decent coverage of
> various options within the plain regression tests when possible.

If we're going to keep them in vacuum.sql, we should use the
client_min_messages fix there, as that's a full solution not just
reducing the window.  But I don't agree that these tests are worth
the cycles, given the coverage elsewhere.  The probability of breaking
this option is just not high enough to justify core-regression-test
coverage.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: SKIP_LOCKED test causes random buildfarm failures
  2019-11-06 21:54 SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
  2019-11-06 23:01 ` Re: SKIP_LOCKED test causes random buildfarm failures Andres Freund <andres@anarazel.de>
  2019-11-07 01:39   ` Re: SKIP_LOCKED test causes random buildfarm failures Michael Paquier <michael@paquier.xyz>
  2019-11-07 16:50     ` Re: SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
  2019-11-14 18:37       ` Re: SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
  2019-11-14 20:16         ` Re: SKIP_LOCKED test causes random buildfarm failures Andres Freund <andres@anarazel.de>
  2019-11-14 20:20           ` Re: SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
@ 2019-11-15 01:19             ` Michael Paquier <michael@paquier.xyz>
  2019-11-15 16:22               ` Re: SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 11+ messages in thread

From: Michael Paquier @ 2019-11-15 01:19 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-hackers@lists.postgresql.org

On Thu, Nov 14, 2019 at 03:20:09PM -0500, Tom Lane wrote:
> If we're going to keep them in vacuum.sql, we should use the
> client_min_messages fix there, as that's a full solution not just
> reducing the window.  But I don't agree that these tests are worth
> the cycles, given the coverage elsewhere.  The probability of breaking
> this option is just not high enough to justify core-regression-test
> coverage.

I would rather keep the solution with client_min_messages, and the
tests in vacuum.sql to keep those checks for the grammar parsing.  So
this basically brings us back to use the patch I proposed here:
https://www.postgresql.org/message-id/20191107013942.GA1768@paquier.xyz

Any objections?
--
Michael

Attachments:

  signature.asc (application/pgp-signature, 832B) download

^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: SKIP_LOCKED test causes random buildfarm failures
  2019-11-06 21:54 SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
  2019-11-06 23:01 ` Re: SKIP_LOCKED test causes random buildfarm failures Andres Freund <andres@anarazel.de>
  2019-11-07 01:39   ` Re: SKIP_LOCKED test causes random buildfarm failures Michael Paquier <michael@paquier.xyz>
  2019-11-07 16:50     ` Re: SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
  2019-11-14 18:37       ` Re: SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
  2019-11-14 20:16         ` Re: SKIP_LOCKED test causes random buildfarm failures Andres Freund <andres@anarazel.de>
  2019-11-14 20:20           ` Re: SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
  2019-11-15 01:19             ` Re: SKIP_LOCKED test causes random buildfarm failures Michael Paquier <michael@paquier.xyz>
@ 2019-11-15 16:22               ` Tom Lane <tgl@sss.pgh.pa.us>
  2019-11-16 06:24                 ` Re: SKIP_LOCKED test causes random buildfarm failures Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 11+ messages in thread

From: Tom Lane @ 2019-11-15 16:22 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-hackers@lists.postgresql.org

Michael Paquier <michael@paquier.xyz> writes:
> I would rather keep the solution with client_min_messages, and the
> tests in vacuum.sql to keep those checks for the grammar parsing.  So
> this basically brings us back to use the patch I proposed here:
> https://www.postgresql.org/message-id/20191107013942.GA1768@paquier.xyz

OK.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: SKIP_LOCKED test causes random buildfarm failures
  2019-11-06 21:54 SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
  2019-11-06 23:01 ` Re: SKIP_LOCKED test causes random buildfarm failures Andres Freund <andres@anarazel.de>
  2019-11-07 01:39   ` Re: SKIP_LOCKED test causes random buildfarm failures Michael Paquier <michael@paquier.xyz>
  2019-11-07 16:50     ` Re: SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
  2019-11-14 18:37       ` Re: SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
  2019-11-14 20:16         ` Re: SKIP_LOCKED test causes random buildfarm failures Andres Freund <andres@anarazel.de>
  2019-11-14 20:20           ` Re: SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
  2019-11-15 01:19             ` Re: SKIP_LOCKED test causes random buildfarm failures Michael Paquier <michael@paquier.xyz>
  2019-11-15 16:22               ` Re: SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
@ 2019-11-16 06:24                 ` Michael Paquier <michael@paquier.xyz>
  0 siblings, 0 replies; 11+ messages in thread

From: Michael Paquier @ 2019-11-16 06:24 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-hackers@lists.postgresql.org

On Fri, Nov 15, 2019 at 11:22:20AM -0500, Tom Lane wrote:
> Michael Paquier <michael@paquier.xyz> writes:
>> I would rather keep the solution with client_min_messages, and the
>> tests in vacuum.sql to keep those checks for the grammar parsing.  So
>> this basically brings us back to use the patch I proposed here:
>> https://www.postgresql.org/message-id/20191107013942.GA1768@paquier.xyz
> 
> OK.

Thanks, applied and back-patched.
--
Michael

Attachments:

  signature.asc (application/pgp-signature, 832B) download

^ permalink  raw  reply  [nested|flat] 11+ messages in thread


end of thread, other threads:[~2019-11-16 06:24 UTC | newest]

Thread overview: 11+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-11-06 21:54 SKIP_LOCKED test causes random buildfarm failures Tom Lane <tgl@sss.pgh.pa.us>
2019-11-06 23:01 ` Andres Freund <andres@anarazel.de>
2019-11-07 01:39   ` Michael Paquier <michael@paquier.xyz>
2019-11-07 16:50     ` Tom Lane <tgl@sss.pgh.pa.us>
2019-11-08 00:01       ` Michael Paquier <michael@paquier.xyz>
2019-11-14 18:37       ` Tom Lane <tgl@sss.pgh.pa.us>
2019-11-14 20:16         ` Andres Freund <andres@anarazel.de>
2019-11-14 20:20           ` Tom Lane <tgl@sss.pgh.pa.us>
2019-11-15 01:19             ` Michael Paquier <michael@paquier.xyz>
2019-11-15 16:22               ` Tom Lane <tgl@sss.pgh.pa.us>
2019-11-16 06:24                 ` Michael Paquier <michael@paquier.xyz>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox