Re: [HACKERS] autovacuum, reloptions, and hardcoded pg_class tupdesc
Alvaro Herrera wrote: Alvaro Herrera wrote: I'm not sure that we have any use for the top level you propose; the attached patch just uses the two lower levels, and I think it fits autovacuum usage just fine. Thanks for the idea. Of course, there's no need to pass the relkind; it goes inside the pg_class tuple. Applied. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] autovacuum, reloptions, and hardcoded pg_class tupdesc
Hi, So I've been progressing on revising the autovacuum patch to make it work with the current reloptions. We have a number of options: 1. Call heap_open() for every relation that we're going to check, and examine the reloptions via the relcache. I'm not sure that I like this very much. Right now we just plow ahead using a pg_class seqscan, which avoids locking the relations just for the sake of verifying whether they need work. However, this is the cleanest option in terms of modularity breakage. 2. Play some dirty tricks in autovacuum and extract the reloptions from the bare pg_class tuple ourselves. I think doing this cleanly requires exporting some internal functions like GetPgClassDescriptor() from relcache.c. 3. Disallow setting reloptions for pg_class, which (I think) allows us to avoid having to use GetPgClassDescriptor, but the rest of it is still a dirty hack. In particular, if we do either (2) or (3), we'll need to keep an eye on that code whenever we modify RelationParseRelOptions. Thoughts? -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] autovacuum, reloptions, and hardcoded pg_class tupdesc
Alvaro Herrera alvhe...@commandprompt.com writes: So I've been progressing on revising the autovacuum patch to make it work with the current reloptions. We have a number of options: 1. Call heap_open() for every relation that we're going to check, and examine the reloptions via the relcache. I'm not sure that I like this very much. I don't like it at all, mainly because it implies taking locks on tables that autovacuum doesn't need to be touching. Even if it's only AccessShareLock, it could result in problems vis-a-vis clients that are holding exclusive table locks. Right now we just plow ahead using a pg_class seqscan, which avoids locking the relations just for the sake of verifying whether they need work. We should stick with that, and refactor the reloptions code as needed to be able to work from just a pg_class tuple. I'm envisioning a scheme like: bottom level: extract from pg_class tuple, return a palloc'd struct relcache: logic to cache the result of the above top level: exported function to return a cached options struct The autovac scan could use the bottom-level API. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] autovacuum, reloptions, and hardcoded pg_class tupdesc
Tom Lane wrote:
Alvaro Herrera alvhe...@commandprompt.com writes:
Right now we just plow
ahead using a pg_class seqscan, which avoids locking the relations
just for the sake of verifying whether they need work.
We should stick with that, and refactor the reloptions code as needed to
be able to work from just a pg_class tuple. I'm envisioning a scheme
like:
bottom level: extract from pg_class tuple, return a palloc'd struct
relcache: logic to cache the result of the above
top level: exported function to return a cached options struct
The autovac scan could use the bottom-level API.
I'm not sure that we have any use for the top level you propose; the
attached patch just uses the two lower levels, and I think it fits
autovacuum usage just fine. Thanks for the idea.
--
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Index: src/backend/access/common/reloptions.c
===
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/common/reloptions.c,v
retrieving revision 1.18
diff -c -p -r1.18 reloptions.c
*** src/backend/access/common/reloptions.c 12 Jan 2009 21:02:14 - 1.18
--- src/backend/access/common/reloptions.c 22 Jan 2009 23:24:04 -
*** untransformRelOptions(Datum options)
*** 558,563
--- 558,606
return result;
}
+ /*
+ * Extract reloptions from a pg_class tuple.
+ *
+ * This is a very low-level routine, expected to be used by relcache code only.
+ * For other uses, consider grabbing the pointer from the relcache entry
+ * instead.
+ *
+ * tupdesc is pg_class' tuple descriptor. amoptions is the amoptions regproc
+ * in the case of the tuple corresponding to an index, or InvalidOid otherwise.
+ */
+ bytea *
+ extractRelOptions(HeapTuple tuple, TupleDesc tupdesc, char relkind, Oid amoptions)
+ {
+ bytea *options;
+ bool isnull;
+ Datum datum;
+
+ datum = fastgetattr(tuple,
+ Anum_pg_class_reloptions,
+ tupdesc,
+ isnull);
+ if (isnull)
+ return NULL;
+
+ /* Parse into appropriate format; don't error out here */
+ switch (relkind)
+ {
+ case RELKIND_RELATION:
+ case RELKIND_TOASTVALUE:
+ case RELKIND_UNCATALOGED:
+ options = heap_reloptions(relkind, datum, false);
+ break;
+ case RELKIND_INDEX:
+ options = index_reloptions(amoptions, datum, false);
+ break;
+ default:
+ Assert(false); /* can't get here */
+ options = NULL; /* keep compiler quiet */
+ break;
+ }
+
+ return options;
+ }
/*
* Interpret reloptions that are given in text-array format.
Index: src/backend/utils/cache/relcache.c
===
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/utils/cache/relcache.c,v
retrieving revision 1.282
diff -c -p -r1.282 relcache.c
*** src/backend/utils/cache/relcache.c 22 Jan 2009 20:16:06 - 1.282
--- src/backend/utils/cache/relcache.c 22 Jan 2009 23:30:45 -
*** AllocateRelationDesc(Relation relation,
*** 351,358
static void
RelationParseRelOptions(Relation relation, HeapTuple tuple)
{
- Datum datum;
- bool isnull;
bytea *options;
relation-rd_options = NULL;
--- 351,356
*** RelationParseRelOptions(Relation relatio
*** 374,404
* we might not have any other for pg_class yet (consider executing this
* code for pg_class itself)
*/
! datum = fastgetattr(tuple,
! Anum_pg_class_reloptions,
! GetPgClassDescriptor(),
! isnull);
! if (isnull)
! return;
!
! /* Parse into appropriate format; don't error out here */
! switch (relation-rd_rel-relkind)
! {
! case RELKIND_RELATION:
! case RELKIND_TOASTVALUE:
! case RELKIND_UNCATALOGED:
! options = heap_reloptions(relation-rd_rel-relkind, datum,
! false);
! break;
! case RELKIND_INDEX:
! options = index_reloptions(relation-rd_am-amoptions, datum,
! false);
! break;
! default:
! Assert(false); /* can't get here */
! options = NULL; /* keep compiler quiet */
! break;
! }
/* Copy parsed data into CacheMemoryContext */
if (options)
--- 372,382
* we might not have any other for pg_class yet (consider executing this
* code for pg_class itself)
*/
! options = extractRelOptions(tuple,
! GetPgClassDescriptor(),
! relation-rd_rel-relkind,
! relation-rd_rel-relkind == RELKIND_INDEX ?
! relation-rd_am-amoptions : InvalidOid);
/* Copy parsed data into CacheMemoryContext */
if (options)
Index: src/include/access/reloptions.h
===
RCS file: /home/alvherre/Code/cvs/pgsql/src/include/access/reloptions.h,v
retrieving revision 1.10
diff -c -p -r1.10 reloptions.h
*** src/include/access/reloptions.h 12 Jan 2009
Re: [HACKERS] autovacuum, reloptions, and hardcoded pg_class tupdesc
Alvaro Herrera wrote:
I'm not sure that we have any use for the top level you propose; the
attached patch just uses the two lower levels, and I think it fits
autovacuum usage just fine. Thanks for the idea.
Of course, there's no need to pass the relkind; it goes inside the
pg_class tuple.
--
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
diff -u src/backend/access/common/reloptions.c src/backend/access/common/reloptions.c
--- src/backend/access/common/reloptions.c 22 Jan 2009 23:24:04 -
+++ src/backend/access/common/reloptions.c 22 Jan 2009 23:51:22 -
@@ -569,11 +569,12 @@
* in the case of the tuple corresponding to an index, or InvalidOid otherwise.
*/
bytea *
-extractRelOptions(HeapTuple tuple, TupleDesc tupdesc, char relkind, Oid amoptions)
+extractRelOptions(HeapTuple tuple, TupleDesc tupdesc, Oid amoptions)
{
bytea *options;
bool isnull;
Datum datum;
+ Form_pg_class classForm;
datum = fastgetattr(tuple,
Anum_pg_class_reloptions,
@@ -582,13 +583,15 @@
if (isnull)
return NULL;
+ classForm = (Form_pg_class) GETSTRUCT(tuple);
+
/* Parse into appropriate format; don't error out here */
- switch (relkind)
+ switch (classForm-relkind)
{
case RELKIND_RELATION:
case RELKIND_TOASTVALUE:
case RELKIND_UNCATALOGED:
- options = heap_reloptions(relkind, datum, false);
+ options = heap_reloptions(classForm-relkind, datum, false);
break;
case RELKIND_INDEX:
options = index_reloptions(amoptions, datum, false);
diff -u src/backend/utils/cache/relcache.c src/backend/utils/cache/relcache.c
--- src/backend/utils/cache/relcache.c 22 Jan 2009 23:30:45 -
+++ src/backend/utils/cache/relcache.c 22 Jan 2009 23:51:05 -
@@ -374,7 +374,6 @@
*/
options = extractRelOptions(tuple,
GetPgClassDescriptor(),
- relation-rd_rel-relkind,
relation-rd_rel-relkind == RELKIND_INDEX ?
relation-rd_am-amoptions : InvalidOid);
diff -u src/include/access/reloptions.h src/include/access/reloptions.h
--- src/include/access/reloptions.h 22 Jan 2009 23:22:58 -
+++ src/include/access/reloptions.h 22 Jan 2009 23:50:25 -
@@ -243,7 +243,7 @@
bool ignoreOids, bool isReset);
extern List *untransformRelOptions(Datum options);
extern bytea *extractRelOptions(HeapTuple tuple, TupleDesc tupdesc,
- char relkind, Oid amoptions);
+ Oid amoptions);
extern relopt_value *parseRelOptions(Datum options, bool validate,
relopt_kind kind, int *numrelopts);
extern void *allocateReloptStruct(Size base, relopt_value *options,
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
