Обсуждение: PATCH: Update autovacuum to use reloptions, instead of a system catalog

On Wed, Feb 18, 2009 at 7:45 AM, Ashesh Vashi
<ashesh.vashi@enterprisedb.com> wrote:
> Hi All,
>
> Please find the attached patch for "Update autovacuum to use reloptions,
> instead of a system catalog".

Thanks - patch applied.

> In "CREATE/ALTER TABLE", it also supports for toast.autovacuum* settings (as
> storage parameters),
> which I have not implemented currently.
> Do we need to give support for that?
> And if yes,

If you can add it in the next few days, then I think yes.

> 1. Which documents I need to read for that?

http://developer.postgresql.org/pgdocs/postgres/sql-createtable.html#SQL-CREATETABLE-STORAGE-PARAMETERS
?

It all looks pretty much the same as the work you've done though.

> 2. From which catalog tables, I can get the toast.autovacuum* settings.

I assume they go in pg_class.reloptions for the owner table. It should
be easy to test that.

> 3. How UI should look like?

Good question. What about having a tabset under the 'Custom
Autovacuum' checkbox, and have one page that shows the existing
options for the table, and one that shows the TOAST options?


--
Dave Page
EnterpriseDB UK:   http://www.enterprisedb.com

In "CREATE/ALTER TABLE", it also supports for toast.autovacuum* settings (as
storage parameters),
which I have not implemented currently.
Do we need to give support for that?
And if yes,

If you can add it in the next few days, then I think yes.

1. Which documents I need to read for that?

http://developer.postgresql.org/pgdocs/postgres/sql-createtable.html#SQL-CREATETABLE-STORAGE-PARAMETERS
It all looks pretty much the same as the work you've done though.

2. From which catalog tables, I can get the toast.autovacuum* settings.

I assume they go in pg_class.reloptions for the owner table. It should
be easy to test that.

3. How UI should look like?

Good question. What about having a tabset under the 'Custom
Autovacuum' checkbox, and have one page that shows the existing
options for the table, and one that shows the TOAST options?

On Wed, Feb 18, 2009 at 1:24 PM, Ashesh Vashi
<ashesh.vashi@enterprisedb.com> wrote:

> Shouldn't we have two 'Custom Autovacuum' checkbox - one for the table and
> the
> other for the toast table?
> Tabset under the 'Vacuum Settings' should do.

Do we even need that checkbox any more? I think it's only really there
for the old AV interface to give a convenient way to remove all
settings.


--
Dave Page
EnterpriseDB UK:   http://www.enterprisedb.com

On Wed, Feb 18, 2009 at 1:24 PM, Ashesh Vashi
<ashesh.vashi@enterprisedb.com> wrote:

Shouldn't we have two 'Custom Autovacuum' checkbox - one for the table and
the
other for the toast table?
Tabset under the 'Vacuum Settings' should do.

Do we even need that checkbox any more? I think it's only really there
for the old AV interface to give a convenient way to remove all
settings.

On Wed, Feb 18, 2009 at 2:04 PM, Ashesh Vashi
<ashesh.vashi@enterprisedb.com> wrote:

> I think - we need.
> Otherwise, we will not be able to reset the autovacuum settings to the
> system(postgres - GUC) settings.

OK - let's include that on the tabset as well then.

--
Dave Page
EnterpriseDB UK:   http://www.enterprisedb.com

On Mon, Feb 23, 2009 at 2:01 PM, Ashesh Vashi
<ashesh.vashi@enterprisedb.com> wrote:
> Hi All,
>
> Please find the patch for the "Update autovacuum (for TOAST-TABLE) to use
> reloptions, instead of a system catalog.

Thanks Ashesh - I've committed what's there as it works as is,
however, the custom AV settings for toast tables are not reflected in
the output from pgTable::GetSql(). Could you fix that please?

We should also add the AV settings to the properties listview (in
pgTable::ShowTreeDetail), like we do with variables on databases and
roles. Can you also look at that please?

Thanks!

--
Dave Page
EnterpriseDB UK:   http://www.enterprisedb.com

On Mon, Feb 23, 2009 at 2:01 PM, Ashesh Vashi
<ashesh.vashi@enterprisedb.com> wrote:

Hi All,

Please find the patch for the "Update autovacuum (for TOAST-TABLE) to use
reloptions, instead of a system catalog.

Thanks Ashesh - I've committed what's there as it works as is,
however, the custom AV settings for toast tables are not reflected in
the output from pgTable::GetSql(). Could you fix that please?

We should also add the AV settings to the properties listview (in
pgTable::ShowTreeDetail), like we do with variables on databases and
roles. Can you also look at that please?

Thanks!

On Tue, Feb 24, 2009 at 9:00 AM, Ashesh Vashi
<ashesh.vashi@enterprisedb.com> wrote:
> Hi Dave,
>
> Please find the patch for (TOAST-TABLE) autovacuum settings.
>
> Patch contains changes for:
> * Moved the code related to fetching the (Toast-Table) AutoVaccum settings
> from dlgTable to pgTable.
> * Showing AV settings (also the Toast-Table AV settings, if present) in the
> properties listview.
> * pgTable::GetSql() will now show custom AV settings for toast tables.
>
Hi Ashesh,

I think there is a little misunderstanding.

With variables on roles and databases, if one is set, we add it to the
properties list as an item on it's own, e.g.

add_missing_from    on
log_statement      all

With tables, I think we should isplay the AV reloptions similarly *if*
they are set, eg.

autovacuum_enabled    Yes
autovacuum_analyze_threshold   150
toast.autovacuum_enabled    Yes
toast.autovacuum_vacuum_cost_limit     25

Can you tweak the patch accordingly please?

--
Dave Page
EnterpriseDB UK:   http://www.enterprisedb.com

On Tue, Feb 24, 2009 at 9:00 AM, Ashesh Vashi
<ashesh.vashi@enterprisedb.com> wrote: 

Hi Dave,

Please find the patch for (TOAST-TABLE) autovacuum settings.

Patch contains changes for:
* Moved the code related to fetching the (Toast-Table) AutoVaccum settings
from dlgTable to pgTable.
* Showing AV settings (also the Toast-Table AV settings, if present) in the
properties listview.
* pgTable::GetSql() will now show custom AV settings for toast tables.
   

Hi Ashesh,

I think there is a little misunderstanding.

With variables on roles and databases, if one is set, we add it to the
properties list as an item on it's own, e.g.

add_missing_from    on
log_statement      all

With tables, I think we should isplay the AV reloptions similarly *if*
they are set, eg.

autovacuum_enabled    Yes
autovacuum_analyze_threshold   150
toast.autovacuum_enabled    Yes
toast.autovacuum_vacuum_cost_limit     25

Can you tweak the patch accordingly please?
 

On Tue, Feb 24, 2009 at 10:00 AM, Ashesh D Vashi
<ashesh.vashi@enterprisedb.com> wrote:
> sure.
> I will update you soon.
> Should we include the toast-table AutoVacuum Settings too?

Yes, I think so.

thanks!

--
Dave Page
EnterpriseDB UK:   http://www.enterprisedb.com

On Tue, Feb 24, 2009 at 10:00 AM, Ashesh D Vashi
<ashesh.vashi@enterprisedb.com> wrote: 

sure.
I will update you soon.
Should we include the toast-table AutoVacuum Settings too?   

Yes, I think so.

thanks

On Tue, Feb 24, 2009 at 11:36 AM, Ashesh Vashi
<ashesh.vashi@enterprisedb.com> wrote:
> Hi Dave,
>
> Please find the updated patch.

Thanks, patch applied (I did change the text displayed slightly - most
notably, _("Yes") instead of wxT("true") and _("No") instead of
wxT("false")).

--
Dave Page
EnterpriseDB UK:   http://www.enterprisedb.com