Re: Increasing test coverage of WAL redo functions

From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgreSQL(dot)org>
Subject: Re: Increasing test coverage of WAL redo functions
Date: 2014-11-19 14:45:30
Message-ID: 20141119144530.GF17845@awork2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 2014-11-19 16:27:56 +0200, Heikki Linnakangas wrote:
> To test WAL replay, I often set up a master-standby system with streaming
> replication and run "make installcheck" on the master. However, the
> regression suite doesn't generate all WAL record types. I spent some time
> looking at the lcov report (make coverage-html), and crafted new tests to
> test those redo functions that were not otherwise covered.

That sounds good. A next step would be to have them run in some
automated fashion...

> All the new test cases are related to indexes, mostly vacuuming them. See
> attached. With this patch, all WAL record types are tested, although there
> are still a few codepaths within the redo functions (aside from "can't
> happen" error checks) that are not exercised.
>
> There are a couple of problems with these new tests:
>
> 1. Whether the vacuum tests test what they're supposed to, depends on what
> else is going on in the system. If there's another backend present that
> holds onto an snapshot, vacuum won't be able to remove any rows, so that
> code will go untested. Running those tests in parallel with other tests
> makes it quite likely that nothing can be vacuumed.

Yes, that's "annoying", but not prohibitive imo. Even having the
routines only triggered every couple runs is better than them not being
triggered at all.

> 2. These make the regression database larger. The following tables and
> indexes are added:
>
> postgres=# \d+
> List of relations
> Schema | Name | Type | Owner | Size | Description
> --------+------------------+-------+--------+---------+-------------
> public | btree_tall_tbl | table | heikki | 24 kB |
> public | btree_test_tbl | table | heikki | 392 kB |
> public | gin_test_tbl | table | heikki | 588 MB |
> public | gist_point_tbl | table | heikki | 1056 kB |
> public | spgist_point_tbl | table | heikki | 1056 kB |
> public | spgist_text_tbl | table | heikki | 1472 kB |
> (6 rows)
>
> postgres=# \di+
> List of relations
> Schema | Name | Type | Owner | Table | Size |
> Descri
> ption
> --------+------------------+-------+--------+------------------+---------+-------
> ------
> public | btree_tall_idx | index | heikki | btree_tall_tbl | 1176 kB |
> public | btree_test_idx | index | heikki | btree_test_tbl | 472 kB |
> public | gin_test_idx | index | heikki | gin_test_tbl | 220 MB |
> public | gist_pointidx | index | heikki | gist_point_tbl | 1744 kB |
> public | spgist_point_idx | index | heikki | spgist_point_tbl | 1120 kB |
> public | spgist_text_idx | index | heikki | spgist_text_tbl | 440 kB |
> (6 rows)

> The GIN test needs to create a huge table, to cover the case of splitting an
> internal posting tree page. That 588MB table plus index is obviously too
> much to include in the regular regression suite. I'm not sure how much
> smaller it could be made, but it's going to be in that ballpark anyway. It
> also takes a long time to run.

How long?

> I think the rest are tolerable, they make the regression database about 9 MB
> larger, from 45 MB to 53 MB, and only take a few seconds to run, on my
> laptop.

> My plan is to leave out that large GIN test for now, and commit the
> rest.

How about putting the gin test in a separate schedule?
expensive_parallel_schedule or something? I'd be more comfortable if the
tests were actually there, even if not run by default.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Marko Tiikkaja 2014-11-19 14:46:13 Re: Functions used in index definitions shouldn't be changed
Previous Message Albe Laurenz 2014-11-19 14:38:05 Functions used in index definitions shouldn't be changed