Skip site navigation (1)
Skip section navigation (2)
Peripheral Links
Re: Measuring relation free space
Attached patch adds a new function to the pageinspect extension for
measuring total free space, in either tables or indexes. It returns the
free space as a percentage, so higher numbers mean more bloat. After
trying a couple of ways to quantify it, I've found this particular
measure correlates well with the nastiest bloat issues I've ran into in
production recently. For example, an index that had swelled to over 5X
its original size due to autovacuum issues registered at 0.86 on this
scale. I could easily see people putting an alert at something like
0.40 and picking candidates to reindex based on it triggering. That
would be about a million times smarter than how I've been muddling
through this class of problems so far.
Code by Jaime Casanova, based on a prototype by me. Thanks to attendees
and sponsors of the PgWest conference for helping to fund some deeper
exploration of this idea.
Here's a test case showing it in action:
create extension pageinspect;
create table t (k serial,v integer);
insert into t(v) (select generate_series(1,100000));
create index t_idx on t(k);
delete from t where k<50000;
vacuum t;
gsmith=# select relation_free_space('t');
relation_free_space
---------------------
0.445466
gsmith=# select relation_free_space('t_idx');
relation_free_space
---------------------
0.550946
Some open questions in my mind:
-Given this is doing a full table scan, should it hook into a ring
buffer to keep from trashing the buffer cache? Or might it loop over
the relation in a different way all together? I was thinking about
eyeing the FSM instead at one point, didn't explore that yet. There's
certainly a few ways to approach this, we just aimed at the easiest way
to get a working starter implementation, and associated results to
compare others against.
-Should there be a non-superuser version of this? We'd certainly need
to get a less cache demolishing version before that would seem wise.
-There were related things in the pageinspect module, but a case could
be made for this being a core function instead. It's a bit more likely
to be used in production than the rest of that extension.
-What if anything related to TOAST should this handle?
We're also planning to do a sampling version of this, using the same
approach ANALYZE does. Grab a number of blocks, extrapolate from
there. It shouldn't take many samples before the accuracy is better
than how people are estimated this now. That work is just waiting on
some better thinking about how to handle the full relation version first.
And, yes, the explanation in the docs and code should be clear that it's
returning a percentage, which I just realized when writing this. At
least I remembered to document something; still ahead of the average new
patch...
--
Greg Smith 2ndQuadrant US greg(at)2ndQuadrant(dot)com Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us
Attachment: relation_free_space-v2.patch
Description: text/x-patch (7.2 KB)
Description: text/x-patch (7.2 KB)
On Sun, Nov 6, 2011 at 04:08, Greg Smith <greg(at)2ndquadrant(dot)com> wrote: > Attached patch adds a new function to the pageinspect extension for > measuring total free space, in either tables or indexes. It returns the > free space as a percentage, so higher numbers mean more bloat. After trying > a couple of ways to quantify it, I've found this particular measure > correlates well with the nastiest bloat issues I've ran into in production > recently. For example, an index that had swelled to over 5X its original > size due to autovacuum issues registered at 0.86 on this scale. I could > easily see people putting an alert at something like 0.40 and picking > candidates to reindex based on it triggering. That would be about a million > times smarter than how I've been muddling through this class of problems so > far. > > Code by Jaime Casanova, based on a prototype by me. Thanks to attendees and > sponsors of the PgWest conference for helping to fund some deeper > exploration of this idea. Looks pretty useful. One quick stylistic comment - we don't generally use "* 1.0" to turn an int into a double - just use a cast. > -Given this is doing a full table scan, should it hook into a ring buffer to > keep from trashing the buffer cache? Or might it loop over the relation in > a different way all together? I was thinking about eyeing the FSM instead > at one point, didn't explore that yet. There's certainly a few ways to > approach this, we just aimed at the easiest way to get a working starter > implementation, and associated results to compare others against. Hooking into a ring buffer seems like an almost requirement before you can run this on a larger production system, wouldn't it? I don't know how hard that is code-wise, but it certainly seems worthwhile. > -Should there be a non-superuser version of this? We'd certainly need to > get a less cache demolishing version before that would seem wise. Not sure that's necessary - at least not for now. Many other diagnostics functions are already superuser only... > -There were related things in the pageinspect module, but a case could be > made for this being a core function instead. It's a bit more likely to be > used in production than the rest of that extension. A case can be made for a lot of things in contrib to be in core ;) I say let's keep it in pageinspect, but then also have you finish off that "split up the contrib" patch :-) > -What if anything related to TOAST should this handle? Similar data for TOAST relations would be intersting, no? But that's easily done from userspace by just querying to the toast table specifically, I assume? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
--On 6. November 2011 01:08:11 -0200 Greg Smith <greg(at)2ndQuadrant(dot)com> wrote: > Attached patch adds a new function to the pageinspect extension for measuring > total free space, in either tables or indexes. I wonder if that should be done in the pgstattuple module, which output some similar numbers. -- Thanks Bernd
On 07/11/11 10:20, Bernd Helmle wrote: > > > --On 6. November 2011 01:08:11 -0200 Greg Smith <greg(at)2ndQuadrant(dot)com> > wrote: > >> Attached patch adds a new function to the pageinspect extension for >> measuring >> total free space, in either tables or indexes. > > I wonder if that should be done in the pgstattuple module, which > output some similar numbers. > Not meaning to disparage Greg's effort in any way, but I was thinking the same thing about pg_freespacemap. I have not checked what - if any differences there are in output, but it would be interesting to compare which of the various (3 at present) extensions with slightly overlapping areas of functionality should perhaps be merged. I am guessing (at this point very much guessing) that pg_freespace map may return its data faster, as pageinspect is gonna have to grovel through all the pages for itself (whereas pg_freespacemap relies on using info from the ... free space map fork). regards Mark
On 11/06/2011 11:55 PM, Mark Kirkwood wrote: > I am guessing (at this point very much guessing) that pg_freespace map > may return its data faster, as pageinspect is gonna have to grovel > through all the pages for itself (whereas pg_freespacemap relies on > using info from the ... free space map fork). I started with pageinspect because I wasn't sure if other methods would be as accurate. For example, I wasn't sure until just before submission that only the free space and size of the relation are needed to get a useful measure here; at one point I was considering some other things too. I've ruled them out as unnecessary as far as I can tell, but I can't claim my tests are exhaustive. Some review confirmation that this measure is useful for other people is one thing I was hoping for feedback on, as one thing to consider in addition to the actual implementation. If this measurement is the only one needed, than as I said at the start of the thread here it might easily be implemented to run just against the free space map instead. I'm thinking of what's been sent so far as a UI with matching output it should produce. If it's possible to get the same numbers faster, exactly how to implement the function under the hood is easy enough to change. Jaime already has a new version in development that adds a ring buffer for example. -- Greg Smith 2ndQuadrant US greg(at)2ndQuadrant(dot)com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us
On Sun, Nov 6, 2011 at 5:38 AM, Magnus Hagander <magnus(at)hagander(dot)net> wrote: > > Looks pretty useful. > thanks for the review, attached is a new version of it > One quick stylistic comment - we don't generally use "* 1.0" to turn > an int into a double - just use a cast. > ok > > Hooking into a ring buffer seems like an almost requirement before you > can run this on a larger production system, wouldn't it? I don't know > how hard that is code-wise, but it certainly seems worthwhile. > seems it wasn't too difficult... i just have to indicate the right buffer access strategy so it's using a ring buffer now -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación
Attachment: relation_free_space-v3.patch
Description: text/x-patch (8.1 KB)
Description: text/x-patch (8.1 KB)
On Tue, Nov 8, 2011 at 1:07 PM, Greg Smith <greg(at)2ndquadrant(dot)com> wrote: > On 11/06/2011 11:55 PM, Mark Kirkwood wrote: > > I am guessing (at this point very much guessing) that pg_freespace map may > return its data faster, as pageinspect is gonna have to grovel through all > the pages for itself (whereas pg_freespacemap relies on using info from the > ... free space map fork). > > I started with pageinspect because I wasn't sure if other methods would be > as accurate. For example, I wasn't sure until just before submission that > only the free space and size of the relation are needed to get a useful > measure here; at one point I was considering some other things too. I've > ruled them out as unnecessary as far as I can tell, but I can't claim my > tests are exhaustive. Some review confirmation that this measure is useful > for other people is one thing I was hoping for feedback on, as one thing to > consider in addition to the actual implementation. > > If this measurement is the only one needed, than as I said at the start of > the thread here it might easily be implemented to run just against the free > space map instead. I'm thinking of what's been sent so far as a UI with > matching output it should produce. If it's possible to get the same numbers > faster, exactly how to implement the function under the hood is easy enough > to change. Jaime already has a new version in development that adds a ring > buffer for example. It's already easy to get "good enough" numbers based on user space tools with very little overhead, so I think it's more important that the server side tool be accurate rather than fast. Of course, if we can get both, that's a bonus, but I'd rather not go that route at the expense of accuracy. Just my .02. Robert Treat conjecture: xzilla.net consulting: omniti.com
On 11/08/2011 05:07 PM, Robert Treat wrote: > It's already easy to get "good enough" numbers based on user space > tools with very little overhead, so I think it's more important that > the server side tool be accurate rather than fast. What user space method do you consider good enough here? I haven't found any approximation that I was really happy with; wouldn't have bothered with this otherwise. -- Greg Smith 2ndQuadrant US greg(at)2ndQuadrant(dot)com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us
Excerpts from Jaime Casanova's message of mar nov 08 18:12:25 -0300 2011: > On Sun, Nov 6, 2011 at 5:38 AM, Magnus Hagander <magnus(at)hagander(dot)net> wrote: > > > > Looks pretty useful. > > thanks for the review, attached is a new version of it Note that AFAIK you shouldn't update the 1.0 extension script ... you have to create a 1.1 version (or whatever), update the default version in the control file, and create an 1.0--1.1 script to upgrade from the original version to 1.1. -- Álvaro Herrera <alvherre(at)commandprompt(dot)com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Tue, Nov 8, 2011 at 7:19 PM, Greg Smith <greg(at)2ndquadrant(dot)com> wrote: > On 11/08/2011 05:07 PM, Robert Treat wrote: >> >> It's already easy to get "good enough" numbers based on user space >> tools with very little overhead, so I think it's more important that >> the server side tool be accurate rather than fast. > > What user space method do you consider good enough here? I haven't found > any approximation that I was really happy with; wouldn't have bothered with > this otherwise. > check_postgres and the pg_bloat_report both use a method of comparing on disk size vs estimated size based on table structure (or index info). Run regularly, it's certainly possible to keep bloat under control. That said, I'd still like to see something more accurate. Robert Treat conjecture: xzilla.net consulting: omniti.com
On Wed, Nov 9, 2011 at 7:58 AM, Alvaro Herrera <alvherre(at)commandprompt(dot)com> wrote: > > Excerpts from Jaime Casanova's message of mar nov 08 18:12:25 -0300 2011: >> On Sun, Nov 6, 2011 at 5:38 AM, Magnus Hagander <magnus(at)hagander(dot)net> wrote: >> > >> > Looks pretty useful. >> >> thanks for the review, attached is a new version of it > > Note that AFAIK you shouldn't update the 1.0 extension script ... you > have to create a 1.1 version (or whatever), update the default version > in the control file, and create an 1.0--1.1 script to upgrade from the > original version to 1.1. > good point... fixed that... a question i have is: are we supposed to let the old script (1.0) around? -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación
Attachment: relation_free_space-v4.patch
Description: text/x-patch (11.8 KB)
Description: text/x-patch (11.8 KB)
On Mon, Nov 14, 2011 at 2:02 PM, Jaime Casanova <jaime(at)2ndquadrant(dot)com> wrote: > On Wed, Nov 9, 2011 at 7:58 AM, Alvaro Herrera > <alvherre(at)commandprompt(dot)com> wrote: >> >> Excerpts from Jaime Casanova's message of mar nov 08 18:12:25 -0300 2011: >>> On Sun, Nov 6, 2011 at 5:38 AM, Magnus Hagander <magnus(at)hagander(dot)net> wrote: >>> > >>> > Looks pretty useful. >>> >>> thanks for the review, attached is a new version of it >> >> Note that AFAIK you shouldn't update the 1.0 extension script ... you >> have to create a 1.1 version (or whatever), update the default version >> in the control file, and create an 1.0--1.1 script to upgrade from the >> original version to 1.1. >> > > good point... fixed that... > a question i have is: are we supposed to let the old script (1.0) around? Since the syntax to install a non-default version is supported, I would argue the old script should be kept. CREATE extension pageinspect with version "1.0" This patch applies and builds cleanly. It works either for "CREATE EXTENSION" from scratch, or for updating from the prior version with "ALTER EXTENSION..UPDATE". It seems to be using the buffer ring strategy as advertised. It reports space that is free exclusively for updates as being free. In other words, it considers space free even if it is reserved against inserts in deference to fillfactor. This is in contrast to pg_freespace, which only reports space available for inserts as being available. I think this is reasonable behavior, but it is subtle and should perhaps be documented. (Is it common to use fill factors other than the default in the first place? Do we assume that people using fillfactor are sophisticated enough not to shot themselves in the foot?) As noted by Greg, the documentation calls it "total amount of free free [sic] space" when that is not what is reported. However, it also is not reporting a percentage, but rather a decimal fraction. The reported value should be multiplied by 100, especially if the docs are going to be changed to call it a percentage. Unless I am missing something, all indexes are handled via a procedure designed for BTree indices, "GetBTRelationFreeSpace". I don't know that the ultimate behavior of this is wrong, but it seems unusual. If I get some more time, I will try to explore what is actually going on when called on other types of indexes. I have no insight into how to handle toast tables, or non-superusers. I had thought that toast tables had names of their own which could be used, but I could not figure out how to do that. Even if there are other ways to get approximately the same information, this functionality seems to be a natural thing to have in the pageinspect extension. Cheers, Jeff
On 11/25/2011 04:42 PM, Jeff Janes wrote: > It reports space that is free exclusively for updates as being free. > In other words, it considers space free even if it is reserved against > inserts in deference to fillfactor. This is in contrast to > pg_freespace, which only reports space available for inserts as being > available. I think this is reasonable behavior, but it is subtle and > should perhaps be documented. Ah, that's right, this is why I first wandered this specific path. Ignoring fillfactor seems to have even more downsides as I see it. Certainly deserves a doc improvement, as well as fixing the description of the value so it's clearly a ratio rather than a true percentage. > (Is it common to use fill factors other > than the default in the first place? Do we assume that people using > fillfactor are sophisticated enough not to shot themselves in the > foot?) It's not common, and I think anyone who sets fillfactor themselves would understand the downside. The bigger risk are people who inherit designs from others that use this feature, but the new person doesn't understand it. If using this feature calls attention to a problem there that prompts an investigation, I'd see that as a good thing, rather than a foot shot. > Unless I am missing something, all indexes are handled via a procedure > designed for BTree indices, "GetBTRelationFreeSpace". I don't know > that the ultimate behavior of this is wrong, but it seems unusual. If > I get some more time, I will try to explore what is actually going on > when called on other types of indexes. This I think I'll punt back toward Jaime, as well as asking "did you have a plan for TOAST here?"
On Mon, Nov 28, 2011 at 5:40 AM, Greg Smith <greg(at)2ndquadrant(dot)com> wrote: > >> Unless I am missing something, all indexes are handled via a procedure >> designed for BTree indices, "GetBTRelationFreeSpace". I don't know >> that the ultimate behavior of this is wrong, but it seems unusual. If >> I get some more time, I will try to explore what is actually going on >> when called on other types of indexes. > > > This I think I'll punt back toward Jaime, as well as asking "did you have a > plan for TOAST here?" > for indexes. it seems pageinspect only deals with btree indexes and i neglected to put a similar limitation on this function... now, because the free space is calculated using PageGetFreeSpace() for indexes it should be doing the right thing for all kind of indexes, i only put the function there because i was trying to avoid to create a new file. But if the function is right for all kind of indexes that's maybe enough to create a new file and rename the helper function so is obvious that it can manage all kind of indexes for toast tables. a simple test here seems to show that is as easy as to add toast tables in the supported objects and treat them as normal pages... or there is something i'm missing? -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación
On 11/28/2011 05:40 AM, Greg Smith wrote: > Ignoring fillfactor seems to have even more downsides as I see it. > Certainly deserves a doc improvement, as well as fixing the > description of the value so it's clearly a ratio rather than a true > percentage. So: I'm very clear on what to do here now: -Make the computation be in units that match it documetnation -Take a look at other index types, as well as TOAST, at least to get the easy ones right. -Fully confirm the extension upgrade logic works as hoped That's the must do stuff. Then there's two more features to consider and do something with if sensible: -Double check whether there is really a useful role in using pg_freespace here. I don't think the numbers will be as good, but maybe we don't care. -Once the above is all sorted, add a second UI that samples some pages and extrapolates, ANALYZE-style, rather than hitting everything. This ones leaves as returned with feedback, feeling pretty good it will be whipped into good shape for the last 9.2 CommitFest. -- Greg Smith 2ndQuadrant US greg(at)2ndQuadrant(dot)com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us
On Sun, Nov 06, 2011 at 10:20:49PM +0100, Bernd Helmle wrote:
> --On 6. November 2011 01:08:11 -0200 Greg Smith <greg(at)2ndQuadrant(dot)com> wrote:
>
>> Attached patch adds a new function to the pageinspect extension for measuring
>> total free space, in either tables or indexes.
>
> I wonder if that should be done in the pgstattuple module, which output
> some similar numbers.
Indeed, pgstattuple already claims to show precisely the same measure. Its
reckoning is right in line for heaps, but the proposed pageinspect function
finds more free space in indexes:
[local] test=# SELECT t.free_percent, relation_free_space('pg_proc'), i.free_percent, relation_free_space('pg_proc_proname_args_nsp_index') FROM pgstattuple('pg_proc') t, pgstattuple('pg_proc_proname_args_nsp_index') i;
free_percent | relation_free_space | free_percent | relation_free_space
--------------+---------------------+--------------+---------------------
2.53 | 0.0253346 | 8.61 | 0.128041
(1 row)
Is one of those index figures simply wrong, or do they measure two senses of
free space, both of which are interesting to DBAs?
Thanks,
nm
On 12/15/2011 04:11 PM, Noah Misch wrote: > Is one of those index figures simply wrong, or do they measure two senses of > free space, both of which are interesting to DBAs? > I think the bigger one--the one I was aiming to measure--also includes fill-factor space. It should be possible to isolate whether that's true by running the function against a fresh index, or by trying tests with a table where there's no useful fill. I need to add some of those to the test example suite. While in theory both measures of free space might be interesting to DBAs, I'd prefer to have the one that reflects the lost space to fill-factor if I'm checking an index. But as Robert Treat was pointing out, even the very rough estimates being made with existing user-space tools not using the contrib module features are helpful enough for a lot of users. So long as it's easy and accuracy is good enough to find bloated indexes, either implementation is probably good enough. Shaking out the alternate implementation ideas was really my goal for this CF here. The major goal of the next revision is to present the options with a measure of their respective accuracy and runtime. If I have to give up just a of bit of accuracy and make it much faster, that's probably what most people want as an option. When Jaime and I come back with an update, it really needs to have benchmarks and accuracy numbers for each option. That may be complicated a bit depending on how much of the table or index is cached, so isolating that out will be a pain. -- Greg Smith 2ndQuadrant US greg(at)2ndQuadrant(dot)com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us
On Fri, Dec 16, 2011 at 02:02:03AM -0500, Greg Smith wrote:
> On 12/15/2011 04:11 PM, Noah Misch wrote:
>> Is one of those index figures simply wrong, or do they measure two senses of
>> free space, both of which are interesting to DBAs?
>
> I think the bigger one--the one I was aiming to measure--also includes
> fill-factor space. It should be possible to isolate whether that's true
> by running the function against a fresh index, or by trying tests with a
> table where there's no useful fill. I need to add some of those to the
> test example suite.
No, both measures include fillfactor space. From a brief look at the code, the
proposed function counts space in non-leaf pages, while pgstattuple does not.
Also, the proposed function counts half-dead pages like live pages, while
pgstattuple counts them like dead pages.
One could perhaps justify those choices either way, but they seem too esoteric
for DBA exposure. I recommend choosing a policy on each and making both
pgstattuple() and any new code respect that policy.
> Shaking out the alternate implementation ideas was really my goal for
> this CF here. The major goal of the next revision is to present the
> options with a measure of their respective accuracy and runtime. If I
> have to give up just a of bit of accuracy and make it much faster,
> that's probably what most people want as an option. When Jaime and I
> come back with an update, it really needs to have benchmarks and
> accuracy numbers for each option. That may be complicated a bit
> depending on how much of the table or index is cached, so isolating that
> out will be a pain.
The previous submission seemed to boil down to a speedier version of "SELECT
free_percent FROM pgstattuple('foo')". (Some of the other statistics aren't
cheap.) Considering that, the code does belong in the pgstattuple module.
The sampling approach you have mentioned sounds promising, especially for
indexes. For heap bloat, it may be hard to improve on pg_freespacemap-based and
check_postgres-style estimates with anything less than a full heap scan.
Thanks,
nm
On Thu, Dec 15, 2011 at 4:11 PM, Noah Misch <noah(at)leadboat(dot)com> wrote:
> On Sun, Nov 06, 2011 at 10:20:49PM +0100, Bernd Helmle wrote:
>> --On 6. November 2011 01:08:11 -0200 Greg Smith <greg(at)2ndQuadrant(dot)com> wrote:
>>
>>> Attached patch adds a new function to the pageinspect extension for measuring
>>> total free space, in either tables or indexes.
>>
>> I wonder if that should be done in the pgstattuple module, which output
>> some similar numbers.
>
> Indeed, pgstattuple already claims to show precisely the same measure. Its
> reckoning is right in line for heaps, but the proposed pageinspect function
> finds more free space in indexes:
>
> [local] test=# SELECT t.free_percent, relation_free_space('pg_proc'), i.free_percent, relation_free_space('pg_proc_proname_args_nsp_index') FROM pgstattuple('pg_proc') t, pgstattuple('pg_proc_proname_args_nsp_index') i;
> free_percent | relation_free_space | free_percent | relation_free_space
> --------------+---------------------+--------------+---------------------
> 2.53 | 0.0253346 | 8.61 | 0.128041
> (1 row)
>
> Is one of those index figures simply wrong, or do they measure two senses of
> free space, both of which are interesting to DBAs?
>
i created a test env using pgbench -s 20 -F 90, i then create a new
table (that keep tracks actions that happens the the pgbench tables,
so insert only) and changed a few fillfactors:
"""
relname | reltuples | reloptions
-------------------------------------+---- -------+------------------
audit_log | 804977 |
pgbench_accounts | 1529890 | {fillfactor=90}
pgbench_accounts_pkey | 1529890 | {fillfactor=50}
pgbench_branches | 20 | {fillfactor=100}
pgbench_branches_pkey | 20 |
pgbench_history | 94062 |
pgbench_tellers | 200 | {fillfactor=100}
pgbench_tellers_pkey | 200 |
(8 rows)
"""
and after running "pgbench -n -c 4 -j 2 -T 300" a few times, i used
attached free_space.sql to see what pg_freespacemap, pgstattuple and
relation_free_space had to say about these tables. the result is
attached in result_free_space.out
my first conclusion is that pg_freespacemap is unreliable when indexes
are involved (and looking at the documentation of that module confirms
that), also the fact that FSM is not designed for accuracy make me
think is not an option.
pgstattuple and relation_free_space are very close in all the numbers
except for 2 indexes pgbench_branches_pkey and pgbench_tellers_pkey;
after a VACUUM FULL and a REINDEX (and the difference persistence) i
checked pgbench_tellers_pkey contents (it has only one page besides
the metapage) and the numbers that i look at where:
page size: 8192
free size: 4148
which in good romance means 50% of free space... so, answering Noah's
question: if that difference has some meaning i can't see it but
looking at the evidence the measure relation_free_space() is giving is
the good one
so, tomorrow (or ...looking at the clock... later today) i will update
the relation_free_space() patch to accept toast tables and other kind
of indexes and add it to the commitfest unless someone says that my
math is wrong and somehow there is a more accurate way of getting the
free space (which is entirely possible)
--
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Attachment: result_free_space.out
Description: application/octet-stream (2.9 KB)
Description: application/octet-stream (2.9 KB)
Attachment: free_space.sql
Description: text/x-sql (588 bytes)
Description: text/x-sql (588 bytes)
On Sat, Jan 14, 2012 at 04:41:57AM -0500, Jaime Casanova wrote: > pgstattuple and relation_free_space are very close in all the numbers > except for 2 indexes pgbench_branches_pkey and pgbench_tellers_pkey; > after a VACUUM FULL and a REINDEX (and the difference persistence) i > checked pgbench_tellers_pkey contents (it has only one page besides > the metapage) and the numbers that i look at where: > > page size: 8192 > free size: 4148 > > which in good romance means 50% of free space... so, answering Noah's > question: if that difference has some meaning i can't see it but > looking at the evidence the measure relation_free_space() is giving is > the good one > > so, tomorrow (or ...looking at the clock... later today) i will update > the relation_free_space() patch to accept toast tables and other kind > of indexes and add it to the commitfest unless someone says that my > math is wrong and somehow there is a more accurate way of getting the > free space (which is entirely possible) Did you see this followup[1]? To summarize: - pgstattuple() and relation_free_space() should emit the same number, even if that means improving pgstattuple() at the same time. - relation_free_space() belongs in the pgstattuple extension, because its role is cheaper access to a single pgstattuple() metric. Thanks, nm [1] http://archives.postgresql.org/message-id/20111218165625.GB6393@tornado.leadboat.com
On Sat, Jan 14, 2012 at 6:26 AM, Noah Misch <noah(at)leadboat(dot)com> wrote: > > - pgstattuple() and relation_free_space() should emit the same number, even if > that means improving pgstattuple() at the same time. yes, i just wanted to understand which one was more accurate and why... and give the opportunity for anyone to point my error if any > - relation_free_space() belongs in the pgstattuple extension, because its role > is cheaper access to a single pgstattuple() metric. > oh! right! so, what about just fixing the free_percent that pgstattuple is providing -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación
On Sat, Jan 14, 2012 at 02:40:46PM -0500, Jaime Casanova wrote: > On Sat, Jan 14, 2012 at 6:26 AM, Noah Misch <noah(at)leadboat(dot)com> wrote: > > > > - pgstattuple() and relation_free_space() should emit the same number, even if > > ?that means improving pgstattuple() at the same time. > > yes, i just wanted to understand which one was more accurate and > why... and give the opportunity for anyone to point my error if any pgstattuple()'s decision to treat half-dead pages like deleted pages is better. That transient state can only end in the page's deletion. I don't know about counting non-leaf pages, but I personally wouldn't revisit pgstattuple()'s decision there. In the indexes I've briefly surveyed, the ratio of leaf pages to non-leaf pages is 100:1 or better. No amount of bloat in that 1% will matter. Feel free to make the argument if you think otherwise, though; I've only taken a brief look at the topic. > > - relation_free_space() belongs in the pgstattuple extension, because its role > > ?is cheaper access to a single pgstattuple() metric. > > oh! right! so, what about just fixing the free_percent that > pgstattuple is providing If pgstattuple() meets your needs, you might indeed no longer need any patch.
On Mon, Jan 16, 2012 at 5:09 AM, Noah Misch <noah(at)leadboat(dot)com> wrote:
>
> pgstattuple()'s decision to treat half-dead pages like deleted pages is
> better. That transient state can only end in the page's deletion.
>
the only page in that index has 200 records (all live 0 dead) using
half the page size (which is a leaf page and is not half dead, btw).
so, how do you justify that pgstattuple say we have just 25% of free
space?
postgres=# SELECT * from bt_page_stats('pgbench_tellers_pkey', 1);
-[ RECORD 1 ]-+-----
blkno | 1
type | l
live_items | 200
dead_items | 0
avg_item_size | 16
page_size | 8192
free_size | 4148
btpo_prev | 0
btpo_next | 0
btpo | 0
btpo_flags | 3
> I don't know about counting non-leaf pages
ignoring all non-leaf pages still gives a considerable difference
between pgstattuple and relation_free_space()
--
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
On Wed, Jan 18, 2012 at 09:46:20AM -0500, Jaime Casanova wrote:
> On Mon, Jan 16, 2012 at 5:09 AM, Noah Misch <noah(at)leadboat(dot)com> wrote:
> >
> > pgstattuple()'s decision to treat half-dead pages like deleted pages is
> > better. ?That transient state can only end in the page's deletion.
> >
>
> the only page in that index has 200 records (all live 0 dead) using
> half the page size (which is a leaf page and is not half dead, btw).
> so, how do you justify that pgstattuple say we have just 25% of free
> space?
>
> postgres=# SELECT * from bt_page_stats('pgbench_tellers_pkey', 1);
> -[ RECORD 1 ]-+-----
> blkno | 1
> type | l
> live_items | 200
> dead_items | 0
> avg_item_size | 16
> page_size | 8192
> free_size | 4148
> btpo_prev | 0
> btpo_next | 0
> btpo | 0
> btpo_flags | 3
>
> > I don't know about counting non-leaf pages
>
> ignoring all non-leaf pages still gives a considerable difference
> between pgstattuple and relation_free_space()
pgstattuple() counts the single B-tree meta page as always-full, while
relation_free_space() skips it for all purposes. For tiny indexes, that can
shift the percentage dramatically.
On Wed, Jan 18, 2012 at 7:01 PM, Noah Misch <noah(at)leadboat(dot)com> wrote: > On Wed, Jan 18, 2012 at 09:46:20AM -0500, Jaime Casanova wrote: >> >> ignoring all non-leaf pages still gives a considerable difference >> between pgstattuple and relation_free_space() > > pgstattuple() counts the single B-tree meta page as always-full, while > relation_free_space() skips it for all purposes. For tiny indexes, that can > shift the percentage dramatically. > ok, i will reformulate the question. why is fine ignoring non-leaf pages but is not fine to ignore the meta page? -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación
On Fri, Jan 20, 2012 at 07:03:22PM -0500, Jaime Casanova wrote: > On Wed, Jan 18, 2012 at 7:01 PM, Noah Misch <noah(at)leadboat(dot)com> wrote: > > On Wed, Jan 18, 2012 at 09:46:20AM -0500, Jaime Casanova wrote: > >> > >> ignoring all non-leaf pages still gives a considerable difference > >> between pgstattuple and relation_free_space() > > > > pgstattuple() counts the single B-tree meta page as always-full, while > > relation_free_space() skips it for all purposes. ?For tiny indexes, that can > > shift the percentage dramatically. > > > > ok, i will reformulate the question. why is fine ignoring non-leaf > pages but is not fine to ignore the meta page? pgstattuple() figures the free_percent by adding up all space available to hold tuples and dividing that by the simple size of the relation. Non-leaf pages and the meta page get identical treatment: both never hold tuples, so they do not contribute to the free space.
Excerpts from Noah Misch's message of vie ene 20 22:33:30 -0300 2012: > On Fri, Jan 20, 2012 at 07:03:22PM -0500, Jaime Casanova wrote: > > On Wed, Jan 18, 2012 at 7:01 PM, Noah Misch <noah(at)leadboat(dot)com> wrote: > > > On Wed, Jan 18, 2012 at 09:46:20AM -0500, Jaime Casanova wrote: > > >> > > >> ignoring all non-leaf pages still gives a considerable difference > > >> between pgstattuple and relation_free_space() > > > > > > pgstattuple() counts the single B-tree meta page as always-full, while > > > relation_free_space() skips it for all purposes. ?For tiny indexes, that can > > > shift the percentage dramatically. > > > > > > > ok, i will reformulate the question. why is fine ignoring non-leaf > > pages but is not fine to ignore the meta page? > > pgstattuple() figures the free_percent by adding up all space available to > hold tuples and dividing that by the simple size of the relation. Non-leaf > pages and the meta page get identical treatment: both never hold tuples, so > they do not contribute to the free space. Hm. Leaf pages hold as much tuples as non-leaf pages, no? I mean for each page element there's a value and a CTID. In non-leaf those CTIDs point to other index pages, one level down the tree; in leaf pages they point to the heap. The metapage is special in that it is not used to store any user data, just a pointer to the root page. -- Álvaro Herrera <alvherre(at)commandprompt(dot)com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Mon, Jan 23, 2012 at 04:56:24PM -0300, Alvaro Herrera wrote: > Excerpts from Noah Misch's message of vie ene 20 22:33:30 -0300 2012: > > pgstattuple() figures the free_percent by adding up all space available to > > hold tuples and dividing that by the simple size of the relation. Non-leaf > > pages and the meta page get identical treatment: both never hold tuples, so > > they do not contribute to the free space. > > Hm. Leaf pages hold as much tuples as non-leaf pages, no? I mean > for each page element there's a value and a CTID. In non-leaf those > CTIDs point to other index pages, one level down the tree; in leaf pages > they point to the heap. That distinction seemed important when I sent my last message, but now I agree that it's largely irrelevant for free space purposes. If someone feels like doing it, +1 for making pgstattuple() count non-leaf free space. Thanks, nm
On Mon, Jan 23, 2012 at 7:18 PM, Noah Misch <noah(at)leadboat(dot)com> wrote: > On Mon, Jan 23, 2012 at 04:56:24PM -0300, Alvaro Herrera wrote: >> >> Hm. Leaf pages hold as much tuples as non-leaf pages, no? I mean >> for each page element there's a value and a CTID. In non-leaf those >> CTIDs point to other index pages, one level down the tree; in leaf pages >> they point to the heap. > > That distinction seemed important when I sent my last message, but now I agree > that it's largely irrelevant for free space purposes. If someone feels like > doing it, +1 for making pgstattuple() count non-leaf free space. > actually i agreed that non-leaf pages are irrelevant... i just confirmed that in a production system with 300GB none of the indexes in an 84M rows table nor in a heavily updated one has more than 1 root page, all the rest are deleted, half_dead or leaf. so the posibility of bloat coming from non-leaf pages seems very odd but the possibility of bloat coming from the meta page doesn't exist, AFAIUI at least we need the most accurate value about usable free space, because the idea is to add a sampler mode to the function so we don't scan the whole relation. that's why we still need the function. btw... pgstattuple also has the problem that it's not using a ring buffer attached are two patches: - v5: is the same original patch but only track space in leaf, deleted and half_dead pages - v5.1: adds the same for all kind of indexes (problem is that this is inconsistent with the fact that pageinspect only manages btree indexes for everything else) -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación
Attachment: relation_free_space-v5.1.patch
Description: text/x-patch (13.8 KB)
Description: text/x-patch (13.8 KB)
Attachment: relation_free_space-v5.patch
Description: text/x-patch (12.1 KB)
Description: text/x-patch (12.1 KB)
On Tue, Jan 24, 2012 at 11:24:08AM -0500, Jaime Casanova wrote:
> On Mon, Jan 23, 2012 at 7:18 PM, Noah Misch <noah(at)leadboat(dot)com> wrote:
> > If someone feels like
> > doing it, +1 for making pgstattuple() count non-leaf free space.
>
> actually i agreed that non-leaf pages are irrelevant... i just
> confirmed that in a production system with 300GB none of the indexes
> in an 84M rows table nor in a heavily updated one has more than 1 root
> page, all the rest are deleted, half_dead or leaf. so the posibility
> of bloat coming from non-leaf pages seems very odd
FWIW, the number to look at is internal_pages from pgstatindex():
[local] test=# create table t4 (c) as select * from generate_series(1,1000000);
SELECT 1000000
[local] test=# alter table t4 add primary key(c);
NOTICE: ALTER TABLE / ADD PRIMARY KEY will create implicit index "t4_pkey" for table "t4"
ALTER TABLE
[local] test=# select * from pgstatindex('t4_pkey');
-[ RECORD 1 ]------+---------
version | 2
tree_level | 2
index_size | 22478848
root_block_no | 290
internal_pages | 10
leaf_pages | 2733
empty_pages | 0
deleted_pages | 0
avg_leaf_density | 90.06
leaf_fragmentation | 0
So, 0.4% of this index. They appear in proportion to the logarithm of the
total index size. I agree that bloat centered on them is unlikely. Counting
them would be justified, but that is a question of formal accuracy rather than
practical importance.
> but the possibility of bloat coming from the meta page doesn't exist,
> AFAIUI at least
>
> we need the most accurate value about usable free space, because the
> idea is to add a sampler mode to the function so we don't scan the
> whole relation. that's why we still need the function.
I doubt we'd add this function solely on the basis that a future improvement
will make it useful. For the patch to go in now, it needs to be useful now.
(This is not a universal principle, but it mostly holds for low-complexity
patches like this one.)
All my comments below would also apply to such a broader patch.
> btw... pgstattuple also has the problem that it's not using a ring buffer
>
>
> attached are two patches:
> - v5: is the same original patch but only track space in leaf, deleted
> and half_dead pages
> - v5.1: adds the same for all kind of indexes (problem is that this is
> inconsistent with the fact that pageinspect only manages btree indexes
> for everything else)
Let's take a step back. Again, what you're proposing is essentially a faster
implementation of "SELECT free_percent FROM pgstattuple(rel)". If this code
belongs in core at all, it belongs in the pgstattuple module. Share as much
infrastructure as is reasonable between the user-visible functions of that
module. For example, I'm suspecting that the pgstat_index() call tree should
be shared, with pgstat_index_page() checking a flag to decide whether to
gather per-tuple stats.
Next, compare the bits of code that differ between pgstattuple() and
relation_free_space(), convincing yourself that the differences are justified.
Each difference will yield one of the following conclusions:
1. Your code contains an innovation that would apply to both functions. Where
not too difficult, merge these improvements into pgstattuple(). In order for
a demonstration of your new code's better performance to be interesting, we
must fix the same low-hanging fruit in its competitor. One example is the use
of the bulk read strategy. Another is the support for SP-GiST.
2. Your code is missing an essential behavior of pgstattuple(). Add it to
your code. One example is the presence of CHECK_FOR_INTERRUPTS() calls.
3. Your code behaves differently from pgstattuple() due to a fundamental
difference in their tasks. These are the only functional differences that
ought to remain in your finished patch; please point them out in comments.
For example, pgstat_heap() visits every tuple in the heap. You'll have no
reason to do that; pgstattuple() only needs it to calculate statistics other
than free_percent.
In particular, I call your attention to the fact that pgstattuple() takes
shared buffer content locks before examining pages. Your proposed patch does
not do so. I do not know with certainty whether that falls under #1 or #2.
The broad convention is to take such locks, because we elsewhere want an exact
answer. These functions are already inexact; they make no effort to observe a
consistent snapshot of the table. If you convince yourself that the error
arising from not locking buffers is reasonably bounded, we can lose the locks
(in both functions -- conclusion #1). Comments would then be in order.
With all that done, run some quick benchmarks: see how "SELECT free_percent
FROM pgstattuple(rel)" fares compared to "SELECT relation_free_space(rel)" for
a large heap and for a large B-tree index. If the timing difference is too
small to be interesting to you, remove relation_free_space() and submit your
pgstattuple() improvements alone. Otherwise, submit as written.
I suppose I didn't make all of this clear enough earlier; sorry for that.
Thanks,
nm
On Wed, Jan 25, 2012 at 9:47 PM, Noah Misch <noah(at)leadboat(dot)com> wrote: > > With all that done, run some quick benchmarks: see how "SELECT free_percent > FROM pgstattuple(rel)" fares compared to "SELECT relation_free_space(rel)" for > a large heap and for a large B-tree index. If the timing difference is too > small to be interesting to you, remove relation_free_space() and submit your > pgstattuple() improvements alone. Otherwise, submit as written. > Ok. I split this in three patches. 1) pgstattuple-gin_spgist.patch This first patch adds gin and spgist support to pgstattuple, also makes pgstattuple use a ring buffer when reading tables or indexes. 2) pgstattuple-relation_free_space.patch This patch adds the relation_free_space function to pgstattuple. the function relation_free_space() is faster than pgstattuple(), to test that i initialize pgbench with a scale of 40. In that context pgstattuple() tooks 1.4s to process pgbench_account table and relation_free_space() tooks 730ms (half the time!) In the index the difference is less notorious, 170ms the former and 150ms the latter. 3) pgstattuple-stats_target.patch This patch adds a stats_target parameter to the relation_free_space() function, it mimics the way analyze choose the blocks to read and is faster than plain relation_free_space() but of course could be inexact if the pages that we don't read are the ones with more free space -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación
Attachment: pgstattuple-stats_target.patch
Description: text/x-patch (8.2 KB)
Description: text/x-patch (8.2 KB)
Attachment: pgstattuple-relation_free_space.patch
Description: text/x-patch (19.7 KB)
Description: text/x-patch (19.7 KB)
Attachment: pgstattuple-gin_spgist.patch
Description: text/x-patch (8.2 KB)
Description: text/x-patch (8.2 KB)
On Tue, Feb 14, 2012 at 02:04:26AM -0500, Jaime Casanova wrote:
> On Wed, Jan 25, 2012 at 9:47 PM, Noah Misch <noah(at)leadboat(dot)com> wrote:
> >
> > With all that done, run some quick benchmarks: see how "SELECT free_percent
> > FROM pgstattuple(rel)" fares compared to "SELECT relation_free_space(rel)" for
> > a large heap and for a large B-tree index. ?If the timing difference is too
> > small to be interesting to you, remove relation_free_space() and submit your
> > pgstattuple() improvements alone. ?Otherwise, submit as written.
> >
>
> Ok. I split this in three patches.
>
> 1) pgstattuple-gin_spgist.patch
> This first patch adds gin and spgist support to pgstattuple, also
> makes pgstattuple use a ring buffer when reading tables or indexes.
The buffer access strategy usage bits look fine to commit. The gin and spgist
support has problems, detailed below.
> 2) pgstattuple-relation_free_space.patch
> This patch adds the relation_free_space function to pgstattuple.
>
> the function relation_free_space() is faster than pgstattuple(), to
> test that i initialize pgbench with a scale of 40.
> In that context pgstattuple() tooks 1.4s to process pgbench_account
> table and relation_free_space() tooks 730ms (half the time!)
> In the index the difference is less notorious, 170ms the former and
> 150ms the latter.
Benchmarks lasting on the order of one second are far too short. I tried the
first two patches on this 6914 MiB table and 4284 MiB index:
create table t(n) as select * from generate_series(1,200000000);
create index on t(n);
This machine has about 1 GiB of memory available for disk cache, and I used
shared_buffers = 128MB. I used a regular assert-enabled build with
debug_assertions = off. Timings:
pgstattuple.free_percent, heap: runtime 166.2s; answer 0.34
pgstattuple.free_percent, index: runtime 110.9s; answer 9.83
relation_free_space, heap: runtime 165.1s; answer 0.00838721
relation_free_space, index: runtime 108.7s; answer 2.23692
Note the disagreement in answers and the nonsensical answer from the last
test. The numbers do line up for smaller tables and indexes that I tried.
During the pgstattuple() runs on the heap, CPU usage divided evenly between
user and iowait time. With relation_free_space(), iowait kept above 80%. For
the index, pgstattuple() managed 60-80% iowait and relation_free_space() again
kept above 80%. Even so, that did not produce any significant change in
runtime. I'm guessing that readahead was highly effective here, so the I/O
bound dictated elapsed time.
Bottom line, this patch can probably achieve 50% speedups on already-in-memory
relations. It can reduce the contribution to CPU load, but not the elapsed
runtime, for relations we largely pull from disk. Do those benefits justify
the additional user-visible interface? I suppose the sort of installation
that would benefit most is one just short of the tipping point of the database
size exceeding memory size. Larger databases could not afford either
function, and smaller databases don't need to watch bloat so closely.
Offhand, I think that the I/O savings of sampling will be the real win, and
it's not worth an extra user-visible function to get the CPU usage savings
this patch offers. Other opinions welcome.
> 3) pgstattuple-stats_target.patch
> This patch adds a stats_target parameter to the relation_free_space()
> function, it mimics the way analyze choose the blocks to read and is
> faster than plain relation_free_space() but of course could be inexact
> if the pages that we don't read are the ones with more free space
This part is a fresh submission. It is simple enough that I have reviewed it.
It gives the expected speedup. However, the results are wrong:
3 runs of pgstattuple('t', 5): 0.000171412, 0.000171876, 0.000169326
3 runs of pgstattuple('t', 10): 0.000336525, 0.000344404, 0.000341314
I can get an apparent infinite loop:
create table t0(n) as select * from generate_series(1,4000000);
create index on t0(n);
[local] test=# select relation_free_space('t0_n_idx', 100);
relation_free_space
---------------------
0.0982675
Time: 133.788 ms
[local] test=# select relation_free_space('t0_n_idx', 5);
Cancel request sent
ERROR: canceling statement due to user request
Time: 24655.481 ms
As a way forward, I suggest abandoning relation_free_space() entirely and
adding a sampling capability to pgstattuple(). There are clear gains to be
had from that method. The gains of splitting out the free percent calculation
from the other pgstattuple() calculations seem meager.
If you would like, submit the buffer strategy bits as a separate patch with
its own CF entry, noting that it arose from here. That part can be Ready for
Committer. I'm marking the original CF entry Returned with Feedback.
Patch 1:
> *** a/contrib/pgstattuple/pgstatindex.c
> --- b/contrib/pgstattuple/pgstatindex.c
> + /*
> + * Buffer access strategy for reading relations, it's simpler to keep it
> + * global because pgstat_*_page() functions read one buffer at a time.
> + * pgstat_heap() and pgstat_index() should initialize it before use.
> + */
> + BufferAccessStrategy bstrategy;
This variable should be static.
> *** 450,455 ****
> --- 471,538 ----
> }
>
> /*
> + * pgstat_gin_page -- check tuples in a gin page
> + */
> + static void
> + pgstat_gin_page(pgstattuple_type *stat, Relation rel, BlockNumber blkno)
> + {
> + Buffer buf;
> + Page page;
> +
> + buf = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL, bstrategy);
> + LockBuffer(buf, GIN_SHARE);
> + page = BufferGetPage(buf);
> +
> + if (GinPageIsDeleted(page))
> + {
> + /* recyclable page */
> + stat->free_space += BLCKSZ;
> + }
> + else if (GinPageIsLeaf(page) || GinPageIsData(page))
> + {
> + pgstat_index_page(stat, page, FirstOffsetNumber,
> + PageGetMaxOffsetNumber(page));
> + }
> + else
> + {
> + /* root or node */
> + }
> +
> + UnlockReleaseBuffer(buf);
> + }
Would you discuss, preferably in comments, the various page types found in GIN
indexes and why this code is correct for all of them? At a minimum, the
comment "root or node" is incorrect.
Another thing to consider and document is how you wish to count tuples. Your
implementation counts every key as a tuple. I think it would be more useful
to count every posting tree entry as a tuple, but I haven't given it a great
deal of thought.
> +
> + /*
> + * pgstat_spgist_page -- check tuples in a spgist page
> + */
> + static void
> + pgstat_spgist_page(pgstattuple_type *stat, Relation rel, BlockNumber blkno)
> + {
> + Buffer buf;
> + Page page;
> +
> + buf = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL, bstrategy);
> + LockBuffer(buf, BUFFER_LOCK_SHARE);
> + page = BufferGetPage(buf);
> +
> + if (SpGistPageIsDeleted(page))
> + {
> + /* recyclable page */
> + stat->free_space += BLCKSZ;
> + }
> + else if (SpGistPageIsLeaf(page))
> + {
> + pgstat_index_page(stat, page, FirstOffsetNumber,
> + PageGetMaxOffsetNumber(page));
> + }
> + else
> + {
> + /* root or node */
> + }
> +
> + UnlockReleaseBuffer(buf);
> + }
pgstat_index_page will not do the right thing for SP-GiST indexes. Only
SPGIST_LIVE tuples should count as live; the other tupstates should count as
dead. Also, pgstattuple gives me a tuple count of 119288 for an SP-GiST index
of a table containing only 40000 tuples. (Disclaimer: I took my first look at
the SP-GiST code today.)
Patch 3:
> *** a/contrib/pgstattuple/pgstattuple.c
> --- b/contrib/pgstattuple/pgstattuple.c
> *************** GetHeapRelationFreeSpace(Relation rel)
> *** 691,716 ****
> {
> /* Get the current relation length */
> LockRelationForExtension(rel, ExclusiveLock);
> ! nblocks = RelationGetNumberOfBlocks(rel);
> UnlockRelationForExtension(rel, ExclusiveLock);
>
> /* Quit if we've scanned the whole relation */
> if (blkno >= nblocks)
> {
> break;
> }
>
> ! for (; blkno < nblocks; blkno++)
> {
> CHECK_FOR_INTERRUPTS();
>
> buffer = ReadBufferExtended(rel, MAIN_FORKNUM, blkno, RBM_NORMAL, bstrategy);
> LockBuffer(buffer, BUFFER_LOCK_SHARE);
>
> page = BufferGetPage(buffer);
> free_space += PageGetHeapFreeSpace(page);
>
> UnlockReleaseBuffer(buffer);
> }
> }
>
> --- 700,733 ----
> {
> /* Get the current relation length */
> LockRelationForExtension(rel, ExclusiveLock);
> ! totalBlocksInRelation = RelationGetNumberOfBlocks(rel);
> UnlockRelationForExtension(rel, ExclusiveLock);
>
> + nblocks = totalBlocksInRelation * stats_target / 100;
You have defined stats_target as a percentage of the relation to sample.
That's a poor basis for sample size. Use a constant number of pages, just as
a given default_statistics_target leads ANALYZE to take a constant number of
tuples from each table. Further background:
http://en.wikipedia.org/wiki/Sample_size_determination#Estimating_proportions_and_means
Excepting those few copied from analyze.c, this patch adds zero comments. You
even omit comments present in the corresponding analyze.c code.
nm
On Wed, Feb 22, 2012 at 12:27 AM, Noah Misch <noah(at)leadboat(dot)com> wrote: > On Tue, Feb 14, 2012 at 02:04:26AM -0500, Jaime Casanova wrote: >> >> 1) pgstattuple-gin_spgist.patch >> This first patch adds gin and spgist support to pgstattuple, also >> makes pgstattuple use a ring buffer when reading tables or indexes. > > The buffer access strategy usage bits look fine to commit. > ok. i extracted that part. which basically makes pgstattuple usable in production (i mean, by not bloating shared buffers when using the function) -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación
Attachment: pgstattuple_bas.patch
Description: text/x-patch (5.6 KB)
Description: text/x-patch (5.6 KB)
On Fri, Mar 09, 2012 at 02:18:02AM -0500, Jaime Casanova wrote: > On Wed, Feb 22, 2012 at 12:27 AM, Noah Misch <noah(at)leadboat(dot)com> wrote: > > On Tue, Feb 14, 2012 at 02:04:26AM -0500, Jaime Casanova wrote: > >> > >> 1) pgstattuple-gin_spgist.patch > >> This first patch adds gin and spgist support to pgstattuple, also > >> makes pgstattuple use a ring buffer when reading tables or indexes. > > > > The buffer access strategy usage bits look fine to commit. > > > > ok. i extracted that part. which basically makes pgstattuple usable in > production (i mean, by not bloating shared buffers when using the > function) I created a CF entry for this and marked it Ready for Committer. You left the bstrategy variable non-static, but that didn't seem important enough to justify another round trip.
On Mon, Mar 12, 2012 at 9:41 PM, Noah Misch <noah(at)leadboat(dot)com> wrote: > > I created a CF entry for this and marked it Ready for Committer. i wasn't sure if create an entry this late was a good idea or not... but now i feel better because is less probable that it will fall out on the cracks, thanks > You left the > bstrategy variable non-static, but that didn't seem important enough to > justify another round trip. > ah! i forgot that... -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación
On Mon, Mar 12, 2012 at 11:10 PM, Jaime Casanova <jaime(at)2ndquadrant(dot)com> wrote: > On Mon, Mar 12, 2012 at 9:41 PM, Noah Misch <noah(at)leadboat(dot)com> wrote: >> >> I created a CF entry for this and marked it Ready for Committer. > > i wasn't sure if create an entry this late was a good idea or not... > but now i feel better because is less probable that it will fall out > on the cracks, thanks > >> You left the >> bstrategy variable non-static, but that didn't seem important enough to >> justify another round trip. >> > > ah! i forgot that... I committed this, but I didn't like the global variable, so I adjusted it to pass bstrategy as a parameter where needed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company