Grouping isolationtester tests in the schedule

Started by Heikki Linnakangasover 6 years ago11 messages
#1Heikki Linnakangas
hlinnaka@iki.fi
1 attachment(s)

The list of tests in src/test/isolation/isolation_schedule has grown
over the years. Originally, they were all related to Serializable
Snapshot Isolation, but there are different kinds of concurrency tests
there now. More tests is good, but the schedule file has grown into a
big inscrutable list with zero comments.

I propose to categorize the tests and add some divider comments to the
file, see attached.

- Heikki

Attachments:

isolation_scheduletext/plain; charset=UTF-8; name=isolation_scheduleDownload
#2Thomas Munro
thomas.munro@gmail.com
In reply to: Heikki Linnakangas (#1)
Re: Grouping isolationtester tests in the schedule

On Wed, Aug 7, 2019 at 11:28 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:

The list of tests in src/test/isolation/isolation_schedule has grown
over the years. Originally, they were all related to Serializable
Snapshot Isolation, but there are different kinds of concurrency tests
there now. More tests is good, but the schedule file has grown into a
big inscrutable list with zero comments.

+1

I propose to categorize the tests and add some divider comments to the
file, see attached.

I think I'd put nowait and skip locked under a separate category "FOR
UPDATE" or "row locking" or something, but maybe that's just me... can
you call that stuff DML?

--
Thomas Munro
https://enterprisedb.com

#3Heikki Linnakangas
hlinnaka@iki.fi
In reply to: Thomas Munro (#2)
Re: Grouping isolationtester tests in the schedule

On 07/08/2019 14:42, Thomas Munro wrote:

On Wed, Aug 7, 2019 at 11:28 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:

The list of tests in src/test/isolation/isolation_schedule has grown
over the years. Originally, they were all related to Serializable
Snapshot Isolation, but there are different kinds of concurrency tests
there now. More tests is good, but the schedule file has grown into a
big inscrutable list with zero comments.

+1

I propose to categorize the tests and add some divider comments to the
file, see attached.

I think I'd put nowait and skip locked under a separate category "FOR
UPDATE" or "row locking" or something, but maybe that's just me... can
you call that stuff DML?

Yeah, I guess SELECT FOR UPDATE isn't really DML. Separate "Row locking"
category works for me. Or maybe "Concurrent DML and row locking". There
is also DML in some of those tests.

- Heikki

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Heikki Linnakangas (#1)
Re: Grouping isolationtester tests in the schedule

Heikki Linnakangas <hlinnaka@iki.fi> writes:

The list of tests in src/test/isolation/isolation_schedule has grown
over the years. Originally, they were all related to Serializable
Snapshot Isolation, but there are different kinds of concurrency tests
there now. More tests is good, but the schedule file has grown into a
big inscrutable list with zero comments.

I propose to categorize the tests and add some divider comments to the
file, see attached.

+1 for concept, didn't review your divisions.

Something related I've been wondering about is whether we could
parallelize the isolation tests. A difficulty here is that the
slowest ones tend to also be timing-sensitive, such that running
them in parallel would increase the risk of failure. But we
could likely get at least some improvement.

regards, tom lane

#5Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Tom Lane (#4)
Re: Grouping isolationtester tests in the schedule

On 2019-Aug-07, Tom Lane wrote:

Something related I've been wondering about is whether we could
parallelize the isolation tests. A difficulty here is that the
slowest ones tend to also be timing-sensitive, such that running
them in parallel would increase the risk of failure. But we
could likely get at least some improvement.

Yeah, there's some improvement to be had there. We've discussed it
previously:
/messages/by-id/20180124231006.z7spaz5gkzbdvob5@alvherre.pgsql

I'm not really happy about this grouping if we mean we're restricted in
how we can make tests run in parallel.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#6Heikki Linnakangas
hlinnaka@iki.fi
In reply to: Alvaro Herrera (#5)
Re: Grouping isolationtester tests in the schedule

On 07/08/2019 18:52, Alvaro Herrera wrote:

On 2019-Aug-07, Tom Lane wrote:

Something related I've been wondering about is whether we could
parallelize the isolation tests. A difficulty here is that the
slowest ones tend to also be timing-sensitive, such that running
them in parallel would increase the risk of failure. But we
could likely get at least some improvement.

Yeah, there's some improvement to be had there. We've discussed it
previously:
/messages/by-id/20180124231006.z7spaz5gkzbdvob5@alvherre.pgsql

I'm not really happy about this grouping if we mean we're restricted in
how we can make tests run in parallel.

The elephant in the room is the 'timeouts' test, which takes about 40
seconds, out of a total runtime of 90 seconds. So we'd really want to
run that in parallel with everything else. Or split 'timeouts' into
multiple tests that could run in parallel. I don't think grouping the
rest of the tests differently will make much difference to how easy or
hard that is.

In any case, we can scramble the list again later, if that's needed for
running the tests in parallel, and we think it's worth it. Until then, a
more logical grouping and some comments would be nice.

- Heikki

#7Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Heikki Linnakangas (#6)
Re: Grouping isolationtester tests in the schedule

On 2019-Aug-07, Heikki Linnakangas wrote:

The elephant in the room is the 'timeouts' test, which takes about 40
seconds, out of a total runtime of 90 seconds. So we'd really want to run
that in parallel with everything else. Or split 'timeouts' into multiple
tests that could run in parallel.

Hmm, that test has 8 permutations, five second each ... if we split it
in 3 and run those in parallel, we reduce the total isolation runtime
by 25 seconds even if we do *nothing else at all*. If we tweak the
other things, I think we could make the whole set run in about 30
seconds total in a normal machine.

Splitting the test never crossed my mind.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Heikki Linnakangas (#6)
Re: Grouping isolationtester tests in the schedule

Heikki Linnakangas <hlinnaka@iki.fi> writes:

The elephant in the room is the 'timeouts' test, which takes about 40
seconds, out of a total runtime of 90 seconds. So we'd really want to
run that in parallel with everything else. Or split 'timeouts' into
multiple tests that could run in parallel. I don't think grouping the
rest of the tests differently will make much difference to how easy or
hard that is.

The problem in "timeouts" is that it has to use drearily long timeouts
to be sure that the behavior will be stable even on really slow machines
(think CLOBBER_CACHE_ALWAYS or valgrind --- it can take seconds for them
to reach a waiting state that other machines reach quickly). If we run
such tests in parallel with anything else, that risks re-introducing the
instability. I'm not very sure what we can do about that. But you might
be right that unless we can solve that, there's not going to be much to be
gained from parallelizing the rest.

I wonder if there's some way to scale the timeout values based on
machine speed? But how would the test get that info?

regards, tom lane

#9Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Tom Lane (#8)
Re: Grouping isolationtester tests in the schedule

On 2019-Aug-07, Tom Lane wrote:

The problem in "timeouts" is that it has to use drearily long timeouts
to be sure that the behavior will be stable even on really slow machines
(think CLOBBER_CACHE_ALWAYS or valgrind --- it can take seconds for them
to reach a waiting state that other machines reach quickly). If we run
such tests in parallel with anything else, that risks re-introducing the
instability. I'm not very sure what we can do about that. But you might
be right that unless we can solve that, there's not going to be much to be
gained from parallelizing the rest.

It runs 8 different permutations serially. If we run the same
permutations in parallel, it would finish much quicker, and we wouldn't
run it in parallel with anything that would take up CPU time, since
they're all just sleeping.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#9)
Re: Grouping isolationtester tests in the schedule

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

On 2019-Aug-07, Tom Lane wrote:

The problem in "timeouts" is that it has to use drearily long timeouts
to be sure that the behavior will be stable even on really slow machines
(think CLOBBER_CACHE_ALWAYS or valgrind --- it can take seconds for them
to reach a waiting state that other machines reach quickly). If we run
such tests in parallel with anything else, that risks re-introducing the
instability. I'm not very sure what we can do about that. But you might
be right that unless we can solve that, there's not going to be much to be
gained from parallelizing the rest.

It runs 8 different permutations serially. If we run the same
permutations in parallel, it would finish much quicker, and we wouldn't
run it in parallel with anything that would take up CPU time, since
they're all just sleeping.

Wrong ... they're *not* just sleeping, in the problem cases. They're
eating cycles due to CLOBBER_CACHE_ALWAYS or valgrind. They're on their
way to sleeping; but they have to get there before the timeout elapses,
or the test shows unexpected results.

Admittedly, as long as you've got more CPUs than tests, it should still
be OK. But if you don't, boom.

regards, tom lane

#11Michael Paquier
michael@paquier.xyz
In reply to: Heikki Linnakangas (#3)
Re: Grouping isolationtester tests in the schedule

On Wed, Aug 07, 2019 at 03:17:02PM +0300, Heikki Linnakangas wrote:

On 07/08/2019 14:42, Thomas Munro wrote:

I think I'd put nowait and skip locked under a separate category "FOR
UPDATE" or "row locking" or something, but maybe that's just me... can
you call that stuff DML?

Yeah, I guess SELECT FOR UPDATE isn't really DML. Separate "Row locking"
category works for me. Or maybe "Concurrent DML and row locking". There is
also DML in some of those tests.

Or would it make sense to group the nowait and skip-locked portion
with the multixact group, then keep the DML-specific stuff together?
There is a test called update-locked-tuple which could enter into the
"row locking" group, and the skip-locked tests have references to
multixact locks. So I think that I would group all that into a single
group: "multixact and row locking".
--
Michael