Thread: Re: [PATCHES] [Fwd: Index Advisor]
I have looked over this patch, and it completes part of this TODO item:
o Add SET PERFORMANCE_TIPS option to suggest INDEX, VACUUM, VACUUM ANALYZE, and CLUSTER
Here is the foundation of it:
For an incoming EXPLAIN command, the planner generates the plan and, ifthe Index Adviser is enabled, then the query is
sentto the IndexAdviser for any suggestions it can make. The Adviser derives a set ofpotentially useful indexes (index
candidates)for this query byanalyzing the query predicates. These indexes are inserted into thesystem catalog as
virtualindexes; that is, they are not created ondisk.Then, the query is again sent to the planner, and this time the
plannermakesit's decisions taking the just-created vitual indexes into accounttoo. All index candidates used in the
finalplan represent therecommendation for the query and are inserted into the advise_indextable by the Adviser.The gain
ofthis recommendation is estimated by comparing the executioncost difference of this plan to the plan generated before
virtualindexeswere created.
It involves a patch to the backend, and a /contrib module to access it.
I think we have to decide if we want this, and whether it should be in
/contrib or fully integrated into the backend. I am thinking the API
needs to be simpified, perhaps by removing the system table and having
the recommendations just logged to the server logs.
---------------------------------------------------------------------------
Gurjeet Singh wrote:
> Hi All,
>
> Please find attached the latest version of the patch attached. It
> is based on REL8_2_STABLE.
>
> It includes a few bug fixes and an improvement to the size
> estimation function. It also includes a work-around to circumvent the
> problem we were facing earlier in xact.c; it now fakes itself to be a
> PL/xxx module by surrounding the BIST()/RARCST() calls inside an
> SPI_connect()/SPI_finish() block.
>
> Please note that the sample_*.txt files in the contrib module,
> which show a few different sample runs, may be a little out of date.
>
> Best regards,
>
>
> --
> gurjeet[.singh]@EnterpriseDB.com
> singh.gurjeet@{ gmail | hotmail | yahoo }.com
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: don't forget to increase your free space map settings
-- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
On Sat, 2007-01-06 at 16:08 -0500, Bruce Momjian wrote: > I have looked over this patch, and it completes part of this TODO item: > > o Add SET PERFORMANCE_TIPS option to suggest INDEX, VACUUM, VACUUM > ANALYZE, and CLUSTER > It involves a patch to the backend, and a /contrib module to access it. > > I think we have to decide if we want this, and whether it should be in > /contrib or fully integrated into the backend. I am thinking the API > needs to be simpified, perhaps by removing the system table and having > the recommendations just logged to the server logs. The patch to the backend is in the form of a plugin API, which does nothing when there is no plugin. IMHO there is a significant amount of code there and it is too early to try to get all of that into the backend, especially when more tested things like Tsearch2 haven't. Plugins are cool because we can update them without needing to bounce a production server, which means the code can evolve faster than it would do if it was directly in the backend. (You do need to reconnect to allow local_preload_libraries to be re-read). Tuning out the wierd recommendations will take some time/effort - I don't know there are any, but then my gut tells me there very likely are some. The output isn't a system table, its a user space table. The reason for having an output table is that we can use multiple invocations of the adviser to build up a set of new indexes for a complete workload. Reading things back out of the log would make that more difficult, since we really want this to be automated by pgAdmin et al. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com
On 1/7/07, Bruce Momjian <bruce@momjian.us> wrote:
Thanks
Well, as already said, the plugin architecture gives others a way to develop and deploy their own index advisers, or even something else that does nifty things with the generated plan!
The advise_index table not required to be a system table anymore, as required by the original patch. It can be any table/view on which the executing user has INSERT permissions. The Adviser internally builds an 'INSERT INTO advise_index ...' statement and executes it through SPI. So, it actually behaves as if the user is doing and INSERT. As a side effect, if the EXPLAIN is done in a transaction, which is later rolled back, the recommendations inserted in the advise_index will also be lost!
contrib/pg_advise_index/sample_error_messages.txt also shows an interesting usage, where advise_index is actually a VIEW with a RULE that redirects INSERTs into another advise_index_data table.
Best regards,
--
gurjeet[.singh]@EnterpriseDB.com
singh.gurjeet@{ gmail | hotmail | yahoo }.com
I have looked over this patch,
Thanks
I think we have to decide if we want this, and whether it should be in
/contrib or fully integrated into the backend.
Well, as already said, the plugin architecture gives others a way to develop and deploy their own index advisers, or even something else that does nifty things with the generated plan!
I am thinking the API
needs to be simpified, perhaps by removing the system table and having
the recommendations just logged to the server logs.
The advise_index table not required to be a system table anymore, as required by the original patch. It can be any table/view on which the executing user has INSERT permissions. The Adviser internally builds an 'INSERT INTO advise_index ...' statement and executes it through SPI. So, it actually behaves as if the user is doing and INSERT. As a side effect, if the EXPLAIN is done in a transaction, which is later rolled back, the recommendations inserted in the advise_index will also be lost!
contrib/pg_advise_index/sample_error_messages.txt also shows an interesting usage, where advise_index is actually a VIEW with a RULE that redirects INSERTs into another advise_index_data table.
Best regards,
--
gurjeet[.singh]@EnterpriseDB.com
singh.gurjeet@{ gmail | hotmail | yahoo }.com
On 1/7/07, Gurjeet Singh <singh.gurjeet@gmail.com> wrote:
Also, the DDL for the advise_index table can be found in advise_index.create.sql script in the contrib module.
Regards,
--
gurjeet[.singh]@EnterpriseDB.com
singh.gurjeet@{ gmail | hotmail | yahoo }.com
contrib/pg_advise_index/sample_error_messages.txt also shows an interesting usage, where advise_index is actually a VIEW with a RULE that redirects INSERTs into another advise_index_data table.
Also, the DDL for the advise_index table can be found in advise_index.create.sql script in the contrib module.
Regards,
--
gurjeet[.singh]@EnterpriseDB.com
singh.gurjeet@{ gmail | hotmail | yahoo }.com
One problem with only putting this information in the system logs
is that when we provide database services to a member of our
community we do not actually give them an account of the DB server
or log server. This means that this very useful information would
need to be passed through an intermediary or another tool developed
to allow access to this information. I think that having this available
from a table would be very nice. My two cents.
Ken
On Sat, Jan 06, 2007 at 04:08:24PM -0500, Bruce Momjian wrote:
>
> I have looked over this patch, and it completes part of this TODO item:
>
> o Add SET PERFORMANCE_TIPS option to suggest INDEX, VACUUM, VACUUM
> ANALYZE, and CLUSTER
>
> Here is the foundation of it:
>
> For an incoming EXPLAIN command, the planner generates the plan and, if
> the Index Adviser is enabled, then the query is sent to the Index
> Adviser for any suggestions it can make. The Adviser derives a set of
> potentially useful indexes (index candidates) for this query by
> analyzing the query predicates. These indexes are inserted into the
> system catalog as virtual indexes; that is, they are not created on
> disk.
>
> Then, the query is again sent to the planner, and this time the planner
> makes it's decisions taking the just-created vitual indexes into account
> too. All index candidates used in the final plan represent the
> recommendation for the query and are inserted into the advise_index
> table by the Adviser.
>
> The gain of this recommendation is estimated by comparing the execution
> cost difference of this plan to the plan generated before virtual
> indexes were created.
>
> It involves a patch to the backend, and a /contrib module to access it.
>
> I think we have to decide if we want this, and whether it should be in
> /contrib or fully integrated into the backend. I am thinking the API
> needs to be simpified, perhaps by removing the system table and having
> the recommendations just logged to the server logs.
>
> ---------------------------------------------------------------------------
>
> Gurjeet Singh wrote:
> > Hi All,
> >
> > Please find attached the latest version of the patch attached. It
> > is based on REL8_2_STABLE.
> >
> > It includes a few bug fixes and an improvement to the size
> > estimation function. It also includes a work-around to circumvent the
> > problem we were facing earlier in xact.c; it now fakes itself to be a
> > PL/xxx module by surrounding the BIST()/RARCST() calls inside an
> > SPI_connect()/SPI_finish() block.
> >
> > Please note that the sample_*.txt files in the contrib module,
> > which show a few different sample runs, may be a little out of date.
> >
> > Best regards,
> >
> >
> > --
> > gurjeet[.singh]@EnterpriseDB.com
> > singh.gurjeet@{ gmail | hotmail | yahoo }.com
>
> [ Attachment, skipping... ]
>
> >
> > ---------------------------(end of broadcast)---------------------------
> > TIP 5: don't forget to increase your free space map settings
>
> --
> Bruce Momjian bruce@momjian.us
> EnterpriseDB http://www.enterprisedb.com
>
> + If your life is a hard drive, Christ can be your backup. +
>
> ---------------------------(end of broadcast)---------------------------
> TIP 9: In versions below 8.0, the planner will ignore your desire to
> choose an index scan if your joining column's datatypes do not
> match
>
Simon Riggs wrote: > On Sat, 2007-01-06 at 16:08 -0500, Bruce Momjian wrote: > > > I have looked over this patch, and it completes part of this TODO item: > > > > o Add SET PERFORMANCE_TIPS option to suggest INDEX, VACUUM, VACUUM > > ANALYZE, and CLUSTER > > > It involves a patch to the backend, and a /contrib module to access it. > > > > I think we have to decide if we want this, and whether it should be in > > /contrib or fully integrated into the backend. I am thinking the API > > needs to be simpified, perhaps by removing the system table and having > > the recommendations just logged to the server logs. > > The patch to the backend is in the form of a plugin API, which does > nothing when there is no plugin. IMHO there is a significant amount of > code there and it is too early to try to get all of that into the > backend, especially when more tested things like Tsearch2 haven't. > Plugins are cool because we can update them without needing to bounce a > production server, which means the code can evolve faster than it would > do if it was directly in the backend. (You do need to reconnect to allow > local_preload_libraries to be re-read). Tuning out the wierd > recommendations will take some time/effort - I don't know there are any, > but then my gut tells me there very likely are some. > > The output isn't a system table, its a user space table. The reason for > having an output table is that we can use multiple invocations of the > adviser to build up a set of new indexes for a complete workload. > Reading things back out of the log would make that more difficult, since > we really want this to be automated by pgAdmin et al. The complex part of this is that the feature requires patches to the backend, and has a /contrib component. If it could be just in /contrib, I agree we would just keep it there until there is a clear direction, but having it in both places seems difficult. I don't think we can maintain a patch to the backend code in /contrib, so it would have to ship with our backend code. That's why I was asking about getting it integrated fully. -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
On Mon, 2007-01-08 at 11:28 -0500, Bruce Momjian wrote: > Simon Riggs wrote: > > On Sat, 2007-01-06 at 16:08 -0500, Bruce Momjian wrote: > > > > > I have looked over this patch, and it completes part of this TODO item: > > > > > > o Add SET PERFORMANCE_TIPS option to suggest INDEX, VACUUM, VACUUM > > > ANALYZE, and CLUSTER > > > > > It involves a patch to the backend, and a /contrib module to access it. > > > > > > I think we have to decide if we want this, and whether it should be in > > > /contrib or fully integrated into the backend. I am thinking the API > > > needs to be simpified, perhaps by removing the system table and having > > > the recommendations just logged to the server logs. > > > > The patch to the backend is in the form of a plugin API, which does > > nothing when there is no plugin. IMHO there is a significant amount of > > code there and it is too early to try to get all of that into the > > backend, especially when more tested things like Tsearch2 haven't. > > Plugins are cool because we can update them without needing to bounce a > > production server, which means the code can evolve faster than it would > > do if it was directly in the backend. (You do need to reconnect to allow > > local_preload_libraries to be re-read). Tuning out the wierd > > recommendations will take some time/effort - I don't know there are any, > > but then my gut tells me there very likely are some. > > > > The output isn't a system table, its a user space table. The reason for > > having an output table is that we can use multiple invocations of the > > adviser to build up a set of new indexes for a complete workload. > > Reading things back out of the log would make that more difficult, since > > we really want this to be automated by pgAdmin et al. > > The complex part of this is that the feature requires patches to the > backend, and has a /contrib component. If it could be just in /contrib, > I agree we would just keep it there until there is a clear direction, > but having it in both places seems difficult. I don't think we can > maintain a patch to the backend code in /contrib, so it would have to > ship with our backend code. That's why I was asking about getting it > integrated fully. The plugin approach is exactly what happened with the debugger. The backend has an appropriate plugin API and the debugger is a plugin. The patch to the backend shouldn't be in contrib, definitely. I would say its up to the installer to offer the opportunity to load the adviser plugin, or not. I like plugins because they encourage faster paced development, diversity and choice. e.g. Multiple java language plugins give users choice. We could include an adviser plugin with the main distribution, as happens with PL/pgSQL... -- Simon Riggs EnterpriseDB http://www.enterprisedb.com
"Simon Riggs" <simon@2ndquadrant.com> writes:
> On Mon, 2007-01-08 at 11:28 -0500, Bruce Momjian wrote:
>> The complex part of this is that the feature requires patches to the
>> backend, and has a /contrib component.
> The plugin approach is exactly what happened with the debugger. The
> backend has an appropriate plugin API and the debugger is a plugin.
> The patch to the backend shouldn't be in contrib, definitely.
> I would say its up to the installer to offer the opportunity to load the
> adviser plugin, or not. I like plugins because they encourage faster
> paced development, diversity and choice.
I would suggest that if we want to encourage faster development, we
should do the same thing we did with the plpgsql debugger support:
put the plugin hooks into the backend and keep the actual plugin(s)
as separate pgfoundry projects. That way the index advisor can have
a release every few weeks if it needs it .... and it will, for awhile.
Stuff in contrib is necessarily tied to the backend release cycle.
(This is not a statement that I approve of the specific plugin hooks
proposed --- I don't particularly. But if we can come up with something
a bit cleaner, that's how I'd approach it.)
regards, tom lane
On Mon, 2007-01-08 at 12:16 -0500, Tom Lane wrote: > "Simon Riggs" <simon@2ndquadrant.com> writes: > > On Mon, 2007-01-08 at 11:28 -0500, Bruce Momjian wrote: > >> The complex part of this is that the feature requires patches to the > >> backend, and has a /contrib component. > > > The plugin approach is exactly what happened with the debugger. The > > backend has an appropriate plugin API and the debugger is a plugin. > > > The patch to the backend shouldn't be in contrib, definitely. > > > I would say its up to the installer to offer the opportunity to load the > > adviser plugin, or not. I like plugins because they encourage faster > > paced development, diversity and choice. > > I would suggest that if we want to encourage faster development, we > should do the same thing we did with the plpgsql debugger support: > put the plugin hooks into the backend and keep the actual plugin(s) > as separate pgfoundry projects. That way the index advisor can have > a release every few weeks if it needs it .... and it will, for awhile. > Stuff in contrib is necessarily tied to the backend release cycle. > > (This is not a statement that I approve of the specific plugin hooks > proposed --- I don't particularly. But if we can come up with something > a bit cleaner, that's how I'd approach it.) Sounds good to me. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com
<span style="font-family: courier new,monospace;">On 1/8/07, </span><b class="gmail_sendername" style="font-family:
couriernew,monospace;">Tom Lane</b><span style="font-family: courier new,monospace;"> <<a
href="mailto:tgl@sss.pgh.pa.us">tgl@sss.pgh.pa.us</a>> wrote:</span><div style="font-family: courier
new,monospace;"><spanclass="gmail_quote"></span><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204,
204,204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> (This is not a statement that I approve of the specific
pluginhooks<br />proposed --- I don't particularly. But if we can come up with something<br />a bit cleaner, that's
howI'd approach it.)<br /></blockquote></div><br style="font-family: courier new,monospace;" /><span
style="font-family:courier new,monospace;">I have another idea for making the hooks a bit more cleaner; I will try that
andrun it through you guys later today.</span><br style="font-family: courier new,monospace;" /><br style="font-family:
couriernew,monospace;" /><span style="font-family: courier new,monospace;">Best regards,</span><br style="font-family:
couriernew,monospace;" /><br clear="all" style="font-family: courier new,monospace;" /><br style="font-family: courier
new,monospace;"/><span style="font-family: courier new,monospace;">-- </span><br style="font-family: courier
new,monospace;"/><span style="font-family: courier new,monospace;">gurjeet[.singh]@ EnterpriseDB.com</span><br
style="font-family:courier new,monospace;" /><span style="font-family: courier new,monospace;">singh.gurjeet@{ gmail |
hotmail| yahoo }.com </span>
"Gurjeet Singh" <singh.gurjeet@gmail.com> writes:
> Please find attached the latest version of the patch. It applies cleanly on
> REL8_2_STABLE.
The interface to the planner in this seems rather brute-force. To run
a plan involving a hypothetical index, you have to make a bunch of
catalog entries, run the planner, and then roll back the transaction
to get rid of the entries. Slow, ugly, and you still need another kluge
to keep the planner from believing the index has zero size.
It strikes me that there is a better way to do it, because 99% of the
planner does not look at the system catalog entries --- all it cares
about is the IndexOptInfo structs set up by plancat.c. So there's not
really any need to make catalog entries at all AFAICS. Rather, the
best thing would be a plugin hook at the end of get_relation_info()
that would have a chance to editorialize on the constructed IndexOptInfo
list (and maybe other properties of the RelOptInfo too). You could
remove existing index entries or insert new ones.
I'm dissatisfied with the hard-wired hook into planner() also.
That doesn't provide any extensibility nor allow the index adviser
to be implemented as a loadable plugin. I'm inclined to think it's
in the wrong place anyway; you've got thrashing around there to avoid
recursion but it's very fragile. Having to dump the results into the
postmaster log isn't a nice user API either. Perhaps what would be
better is a hook in EXPLAIN to call a plugin that can add more lines to
EXPLAIN's output, and is passed the original query and plan so that
it can re-call the planner with hypothetical indexes prepared for
insertion by the other hook.
regards, tom lane
Hi Tom,
The original patch was submitted by Kai Sattler, and we (at EDB) spent a lot of time improving it, making it as seamless and as user-friendly as possible. As is evident from the version number of the patch (v26), it has gone through a lot of iterations, and was available to the community for review and discussion (and discuss they did; they asked for a few things and those were added/improved).
<quote Bruce>
I am thinking the API needs to be simpified, perhaps by removing the system table and having the recommendations just logged to the server logs.
</quote>
<quote Kenneth>
This means that this very useful information (in log files) would need to be passed through an intermediary or another tool developed to allow access to this information. I think that having this available from a table would be very nice.
</quote>
In the initial submission, the feature was a big piece of code embedded inside the backend. It required a system table, did not show the new plan, actually created index physically before re planning, and could not advise for a running application (everything had to be manually EXPLAINed).
I read through the thread titled "Index Tuning Features" that first discussed the idea of an Index adviser for PG, and this patch also meets quite a few requirements raised there.
Here are a few of the good things about this patch as of now:
.) Loadable plugin. Develop your own plugin to do nifty things with the plan generated by the planner. Just as the debugger is implemented; if no plugin... no work to do...
.) No syntax change. Run your queries as they are and get the advice in the advise_index table (or set client_min_messages = LOG, to see the improved plan on the screen also, if any).
.) Can recommend indexes even for the generated dynamic-queries, that are hard to regenerate in a dry-run.
.) Can recommend indexes for SQL being executed through plpgsql (or any PL) (again, hard to regenerate the parameterized queries by hand), and the the advice is available in the advise_index table.
.) The adviser dumps it's advice in a table named advise_index. That can be a user table, or a view with INSERT rule, or anything else; it should just be an INSERTable object, accessible to the executing user (as opposed to a system table required by the original implementation, and hence a need for initdb).
.) No need to modify the application in any way; just set PGOPTIONS environment variable properly before executing the appln., and run it as usual... you have the advice generated for you.
.) No need for DBA (or the appln. writer) to feed anything to the planner in any way; the process of recommendation is fully automated (this may change if another plugin implimentation requires the stats in some user table).
.) Does recommend multi-column indexes. Does not make a set of each fathomable combination of table columns to develop multi-column indexes (hence avoiding a combinatorial explosion of time-space requirements); it uses the columns used in the query to generate multi-column indexes.
.) The indexes are not created on disk; the index-tuple-size calculation function does a very good job of estimating the size of the virtual index.
.) The changes to the catalog are just for the backend running under the adviser, no one else can see those virtual indexes (as opposed to the earlier implementation where the indexes were created on-disk, and available to all the backends in the planning phase).
So, with one hook (no GUC variables!), we get all these cool things. I tried very hard to eliminate that one leftover kludge, but couldn't (we have two options here, and they are enclosed in '#if GLOBAL_CAND_LIST ... #else' parts of the code; left upto the committers to decide which one we need!).
Another kludge that I had to add was the SPI_connect() and SPI_finish() frame around the savepoint handling, since the RollbackToSavepoint in xact.c assumes that only a PL/* module must be using the savepoint infrastucture (this was discussed on -hackers).
The interface etc. may not be beautiful, but it isn't ugly either! It is a lot better than manually creating pg_index records and inserting them into cache; we use index_create() API to create the index (build is deferred), and then 'rollback to savepoint' to undo those changes when the advisor is done. index_create() causes pg_depends entries too, so a 'RB to SP' is far much safer than going and deleting cache records manually.
I hope you would agree that we need two passes of planner, one without v-indexes and the other with v-indexes, for the backend to compare the costs, and recommend indexes only if the second plan turned out to be cheaper. If we implement the way you have suggested, then we will need one hook at the end of get_relation_info(), one in EXPLAIN code, and yet another, someplace after planner is finished, to do the comparison of the two plans and recommend only those indexes that were considered to be useful by the planner. (A total of three places to touch). And then we'll need some extra code in the core to generate the advisory (in some form; maybe into a user table, or as part of the EXPLAIN output; but remember, not every query can be EXPLAINed!).
Then, we also need logic in all these places to differentiate the normal run from the v-index enabled run, else we'll end up generating IndexOptInfo everytime we enter get_relation_info(). And this differentiation needs to be done in EXPLAIN code too.
Also, although the whole plan-tree is available in get_relation_info(), but it wouldn't be the right place to scan other tables, for eg., for generating JOIN-INDEXes or materializing some intermediate joins. (sometime in the future we may support them!).
If we don't run the planner twice, then the developer will have to run it manually twice, and compare the costs manually (with and without v-indexes); virtually impossible for lage applications and introduction of another human-error possibility.
(I just noticed that you quoted the line from the mail where I submitted version 23 of the patch, the plugin architecture wasn't utilized; please refer to the mail that has 'pg_post_planner_plugin-HEAD_20070116- v2.patch.gz' and 'pg_index_adviser-HEAD_20070116-v26.patch.gz' as attachments; dated 20.Jan.07)
About the right place to call the plugin... calling it immediately after the planner is done with normal planning phase seems to be right. At this point planner is done and no other part of the backend yet knows about what plan is generated; so the plugin has a chance to modify the plan in place and do it's trickery in a completely isolated time-space. (maybe we can pass a reference to the plan pointer, and let the plugin replace the whole plan itself using this reference!)
I surely agree that it is time-consuming (less efficient), but it is completely automated, with the least of human interference or application change required; hence, on the whole, it must be a million times faster than a human sitting down, extracting every query - prepending EXPLAIN to it - and executing it - twice - comparing the resulting cost!!!
Best regards,
--
gurjeet[.singh]@EnterpriseDB.com
singh.gurjeet @{ gmail | hotmail | yahoo }.com
17°29'34.37"N 78°30'59.76"E - Hyderabad
18°32'57.25"N 73°56'25.42"E - Pune *
The original patch was submitted by Kai Sattler, and we (at EDB) spent a lot of time improving it, making it as seamless and as user-friendly as possible. As is evident from the version number of the patch (v26), it has gone through a lot of iterations, and was available to the community for review and discussion (and discuss they did; they asked for a few things and those were added/improved).
<quote Bruce>
I am thinking the API needs to be simpified, perhaps by removing the system table and having the recommendations just logged to the server logs.
</quote>
<quote Kenneth>
This means that this very useful information (in log files) would need to be passed through an intermediary or another tool developed to allow access to this information. I think that having this available from a table would be very nice.
</quote>
In the initial submission, the feature was a big piece of code embedded inside the backend. It required a system table, did not show the new plan, actually created index physically before re planning, and could not advise for a running application (everything had to be manually EXPLAINed).
I read through the thread titled "Index Tuning Features" that first discussed the idea of an Index adviser for PG, and this patch also meets quite a few requirements raised there.
Here are a few of the good things about this patch as of now:
.) Loadable plugin. Develop your own plugin to do nifty things with the plan generated by the planner. Just as the debugger is implemented; if no plugin... no work to do...
.) No syntax change. Run your queries as they are and get the advice in the advise_index table (or set client_min_messages = LOG, to see the improved plan on the screen also, if any).
.) Can recommend indexes even for the generated dynamic-queries, that are hard to regenerate in a dry-run.
.) Can recommend indexes for SQL being executed through plpgsql (or any PL) (again, hard to regenerate the parameterized queries by hand), and the the advice is available in the advise_index table.
.) The adviser dumps it's advice in a table named advise_index. That can be a user table, or a view with INSERT rule, or anything else; it should just be an INSERTable object, accessible to the executing user (as opposed to a system table required by the original implementation, and hence a need for initdb).
.) No need to modify the application in any way; just set PGOPTIONS environment variable properly before executing the appln., and run it as usual... you have the advice generated for you.
.) No need for DBA (or the appln. writer) to feed anything to the planner in any way; the process of recommendation is fully automated (this may change if another plugin implimentation requires the stats in some user table).
.) Does recommend multi-column indexes. Does not make a set of each fathomable combination of table columns to develop multi-column indexes (hence avoiding a combinatorial explosion of time-space requirements); it uses the columns used in the query to generate multi-column indexes.
.) The indexes are not created on disk; the index-tuple-size calculation function does a very good job of estimating the size of the virtual index.
.) The changes to the catalog are just for the backend running under the adviser, no one else can see those virtual indexes (as opposed to the earlier implementation where the indexes were created on-disk, and available to all the backends in the planning phase).
So, with one hook (no GUC variables!), we get all these cool things. I tried very hard to eliminate that one leftover kludge, but couldn't (we have two options here, and they are enclosed in '#if GLOBAL_CAND_LIST ... #else' parts of the code; left upto the committers to decide which one we need!).
Another kludge that I had to add was the SPI_connect() and SPI_finish() frame around the savepoint handling, since the RollbackToSavepoint in xact.c assumes that only a PL/* module must be using the savepoint infrastucture (this was discussed on -hackers).
The interface etc. may not be beautiful, but it isn't ugly either! It is a lot better than manually creating pg_index records and inserting them into cache; we use index_create() API to create the index (build is deferred), and then 'rollback to savepoint' to undo those changes when the advisor is done. index_create() causes pg_depends entries too, so a 'RB to SP' is far much safer than going and deleting cache records manually.
I hope you would agree that we need two passes of planner, one without v-indexes and the other with v-indexes, for the backend to compare the costs, and recommend indexes only if the second plan turned out to be cheaper. If we implement the way you have suggested, then we will need one hook at the end of get_relation_info(), one in EXPLAIN code, and yet another, someplace after planner is finished, to do the comparison of the two plans and recommend only those indexes that were considered to be useful by the planner. (A total of three places to touch). And then we'll need some extra code in the core to generate the advisory (in some form; maybe into a user table, or as part of the EXPLAIN output; but remember, not every query can be EXPLAINed!).
Then, we also need logic in all these places to differentiate the normal run from the v-index enabled run, else we'll end up generating IndexOptInfo everytime we enter get_relation_info(). And this differentiation needs to be done in EXPLAIN code too.
Also, although the whole plan-tree is available in get_relation_info(), but it wouldn't be the right place to scan other tables, for eg., for generating JOIN-INDEXes or materializing some intermediate joins. (sometime in the future we may support them!).
If we don't run the planner twice, then the developer will have to run it manually twice, and compare the costs manually (with and without v-indexes); virtually impossible for lage applications and introduction of another human-error possibility.
(I just noticed that you quoted the line from the mail where I submitted version 23 of the patch, the plugin architecture wasn't utilized; please refer to the mail that has 'pg_post_planner_plugin-HEAD_20070116- v2.patch.gz' and 'pg_index_adviser-HEAD_20070116-v26.patch.gz' as attachments; dated 20.Jan.07)
About the right place to call the plugin... calling it immediately after the planner is done with normal planning phase seems to be right. At this point planner is done and no other part of the backend yet knows about what plan is generated; so the plugin has a chance to modify the plan in place and do it's trickery in a completely isolated time-space. (maybe we can pass a reference to the plan pointer, and let the plugin replace the whole plan itself using this reference!)
I surely agree that it is time-consuming (less efficient), but it is completely automated, with the least of human interference or application change required; hence, on the whole, it must be a million times faster than a human sitting down, extracting every query - prepending EXPLAIN to it - and executing it - twice - comparing the resulting cost!!!
Best regards,
--
gurjeet[.singh]@EnterpriseDB.com
singh.gurjeet @{ gmail | hotmail | yahoo }.com
17°29'34.37"N 78°30'59.76"E - Hyderabad
18°32'57.25"N 73°56'25.42"E - Pune *
On 4/6/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
"Gurjeet Singh" <singh.gurjeet@gmail.com> writes:
> Please find attached the latest version of the patch. It applies cleanly on
> REL8_2_STABLE.
The interface to the planner in this seems rather brute-force. To run
a plan involving a hypothetical index, you have to make a bunch of
catalog entries, run the planner, and then roll back the transaction
to get rid of the entries. Slow, ugly, and you still need another kluge
to keep the planner from believing the index has zero size.
It strikes me that there is a better way to do it, because 99% of the
planner does not look at the system catalog entries --- all it cares
about is the IndexOptInfo structs set up by plancat.c. So there's not
really any need to make catalog entries at all AFAICS. Rather, the
best thing would be a plugin hook at the end of get_relation_info()
that would have a chance to editorialize on the constructed IndexOptInfo
list (and maybe other properties of the RelOptInfo too). You could
remove existing index entries or insert new ones.
I'm dissatisfied with the hard-wired hook into planner() also.
That doesn't provide any extensibility nor allow the index adviser
to be implemented as a loadable plugin. I'm inclined to think it's
in the wrong place anyway; you've got thrashing around there to avoid
recursion but it's very fragile. Having to dump the results into the
postmaster log isn't a nice user API either. Perhaps what would be
better is a hook in EXPLAIN to call a plugin that can add more lines to
EXPLAIN's output, and is passed the original query and plan so that
it can re-call the planner with hypothetical indexes prepared for
insertion by the other hook.
regards, tom lane
Gurjeet Singh wrote: > The interface etc. may not be beautiful, but it isn't ugly either! It is > a lot better than manually creating pg_index records and inserting them into > cache; we use index_create() API to create the index (build is deferred), > and then 'rollback to savepoint' to undo those changes when the advisor is > done. index_create() causes pg_depends entries too, so a 'RB to SP' is far > much safer than going and deleting cache records manually. My complaint was not that the API used in the code was non-optimal(which I think was Tom's issue), but that the _user_ API was not very clean. Not sure what to recommend, but I will think about it later. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
On 4/12/07, Bruce Momjian <bruce@momjian.us> wrote:
Gurjeet Singh wrote:
> The interface etc. may not be beautiful, but it isn't ugly either! It is
> a lot better than manually creating pg_index records and inserting them into
> cache; we use index_create() API to create the index (build is deferred),
> and then 'rollback to savepoint' to undo those changes when the advisor is
> done. index_create() causes pg_depends entries too, so a 'RB to SP' is far
> much safer than going and deleting cache records manually.
My complaint was not that the API used in the code was non-optimal(which
I think was Tom's issue), but that the _user_ API was not very clean.
Not sure what to recommend, but I will think about it later.
That can be fixed/improved with minimal efforts, but if it is the internal API usage, or the architecture we're bothered about, then IMO just an overhaul of the code will not be sufficient, rather, it will require rework from scratch.
Best regards,
--
gurjeet[.singh]@EnterpriseDB.com
singh.gurjeet@{ gmail | hotmail | yahoo }.com
17°29'34.37"N 78°30'59.76"E - Hyderabad
18°32'57.25"N 73°56'25.42 "E - Pune *
On Tue, 2007-04-10 at 12:18 -0700, Gurjeet Singh wrote: > Also, although the whole plan-tree is available in > get_relation_info(), but it wouldn't be the right place to scan other > tables, for eg., for generating JOIN-INDEXes or materializing some > intermediate joins. (sometime in the future we may support them!). I like Tom's suggestion. We never thought actually creating the indexes was a very good thing and I'd be happy to bury that idea for good. Speed is definitely a consideration if we are to re-plan thousands of SQL statements for a real workload. > If we don't run the planner twice, then the developer will have to > run it manually twice, and compare the costs manually (with and > without v-indexes); virtually impossible for lage applications and > introduction of another human-error possibility. AFAICS Tom hasn't referred to running twice or not, so I'm not very sure what you're referring to, sorry. If you could answer Tom's suggestions one by one directly underneath them it would be easier to discuss things. ISTM that you've done a great job, the trick is now to reach agreement and finish this. If there is something still to discuss, it needs to be very clearly tied back to Tom's comments so everyone can follow it, then agree it. If there is a problem in Tom's suggestions that directly effects the operation of the tool then we need to identify what that is. But if those hooks would give us all we need, then lets agree it and fix up the adviser plug-in later. We really, really, really need this. Lots. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com