Re: [HACKERS] PATCH: default_index_tablespace
J.L., * jltal...@adv-solutions.net (jltal...@adv-solutions.net) wrote: Any suggestions on how to do it properly? Does Greg Stark's suggestion (at CAM-w4HPOASwsQMdGZqjyFHNubbUnWrUAo8ibci-97UKU=po...@mail.gmail.com) make sense to you? This approach might suffer from the same problem as mine, though. Well, Greg's suggestion was intended to specifically avoid breaking pg_dump by having two new GUCs and having default_tablespace take precedence, if set. It seems to me, IMVHO, a limitation in pg_dump ---since 8.0 when tablespace support for CREATE INDEX was implemented--- that we should fix. Keeping backwards compatibility is indeed required; I just did not think about pg_dump at all :( Limitation strikes me as not quite the right term, but I certainly agree that it's unfortunate that pg_dump uses that GUC instead of adding the TABLESPACE clause to the CREATE INDEX, then again, there are likely to be historical reasons for that. Unfortunately, not break existing pg_dump-generated files is pretty tough. I don't mind reworking the patch or redoing it completely once there is a viable solution. Having three GUCs in the end might work but it seems kind of grotty to have the more-specific GUCs be overridden by the less-specific GUC. We could throw a warning if the more-specific GUC is attempted to be set while the less-specific GUC is set, and vis-versa, and essentially make them either/or. That'd cause additional warnings to be thrown when restoring an older dump, but if pg_dump was modified to use the TABLESPACE clause for CREATE INDEX for new dump files then that's only a temporary situation. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] PATCH: default_index_tablespace
On 15 Apr 2015 19:12, Tom Lane t...@sss.pgh.pa.us wrote: I'm afraid this idea is a nonstarter, because it will break existing applications, and in particular existing pg_dump output files, which expect to be able to determine an index's tablespace by setting default_tablespace. (It is *not* adequate that the code falls back to default_tablespace if the new GUC is unset; if it is set, you've still broken pg_dump.) The incremental value, if indeed there is any, of being able to control index positioning this way seems unlikely to justify a backwards-compatibility break of such magnitude. Just brainstorming here but that just means default_tablespace needs to take precedence. We could have a default_table_tablespace and default_index_tablespace which default_tablespace overrides. Or we could allow a mini config language in default_tablespace like table=space1,index=space2.
Re: [HACKERS] PATCH: default_index_tablespace
On 4/15/15 11:33 PM, Amit Kapila wrote: On Thu, Apr 16, 2015 at 8:01 AM, Bruce Momjian br...@momjian.us mailto:br...@momjian.us wrote: On Wed, Apr 15, 2015 at 07:12:11PM -0400, Tom Lane wrote: jltal...@adv-solutions.net mailto:jltal...@adv-solutions.net writes: This small patch implements a new GUC (default_index_tablespace) plus supporting code. Originated from a customer request, the feature intends to make creation of indexes on SSD-backed tablespaces easy and convenient (almost transparent) for users: the DBA can just set it and indexes will be placed in the specified tablespace --as opposed to the same tablespace where the referenced table is-- without having to specify it every time. Another way to provide different default tablespace for index could be to provide it at Database level. Have a new option INDEX_TABLESPACE in Create Database command which can be used to create indexes when not specified during Create Index command. This would also need changes in pg_dump (like while dumping info about database) but on initial look, it seems it can be done without much changes. That's same idea that Stephen and I have discussed in the past. Something like: CREATE DATABASE name SET TABLESPACE table_volume SET INDEX TABLESPACE index_volume; This has some real usability advantages. In the past I've written code to move tables to where they need to be once the db update is complete. The tables tend to be small or empty so this is not usually a big deal - but sometimes it is. Trying to get a tablespace clause on every index in the build scripts is a real PITA. -- - David Steele da...@pgmasters.net signature.asc Description: OpenPGP digital signature
Re: [HACKERS] PATCH: default_index_tablespace
I'm afraid this idea is a nonstarter, because it will break existing applications, and in particular existing pg_dump output files, which expect to be able to determine an index's tablespace by setting default_tablespace. (It is *not* adequate that the code falls back to default_tablespace if the new GUC is unset; if it is set, you've still broken pg_dump.) Got it. Thank you for the feedback. The incremental value, if indeed there is any, of being able to control index positioning this way seems unlikely to justify a backwards-compatibility break of such magnitude. I can see why someone would want this because random I/O, which is frequent for indexes, is much faster on SSD than magnetic disks. (Sequential I/O is more similar for the two.) The general idea is something I've brought up previously also (I believe it was even discussed at the Dev meeting in, uh, 2013?) so I'm not anxious to simply dismiss it, but it'd certainly have to be done correctly.. Any suggestions on how to do it properly? Does Greg Stark's suggestion (at CAM-w4HPOASwsQMdGZqjyFHNubbUnWrUAo8ibci-97UKU=po...@mail.gmail.com) make sense to you? This approach might suffer from the same problem as mine, though. It seems to me, IMVHO, a limitation in pg_dump ---since 8.0 when tablespace support for CREATE INDEX was implemented--- that we should fix. Keeping backwards compatibility is indeed required; I just did not think about pg_dump at all :( I don't mind reworking the patch or redoing it completely once there is a viable solution. Thanks, / J.L. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] PATCH: default_index_tablespace
Hi,
This small patch implements a new GUC (default_index_tablespace) plus
supporting code.
Originated from a customer request, the feature intends to make
creation of indexes on SSD-backed tablespaces easy and convenient
(almost transparent) for users: the DBA can just set it and indexes will
be placed in the specified tablespace --as opposed to the same
tablespace where the referenced table is-- without having to specify it
every time.
Feedback appreciated.
Thanks,
/ J.L.
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
***
*** 5622,5627 COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
--- 5622,5664
/listitem
/varlistentry
+ varlistentry id=guc-default-index-tablespace
xreflabel=default_index_tablespace
+ termvarnamedefault_index_tablespace/varname (typestring/type)
+ indexterm
+primaryvarnamedefault_index_tablespace/ configuration
parameter/primary
+ /indexterm
+ indextermprimarytablespace/secondarydefault//
+ /term
+ listitem
+para
+ This variable specifies the default tablespace in which to create
+ indexes when a commandCREATE INDEX/ command does
+ not explicitly specify a tablespace.
+/para
+
+para
+ The value is either the name of a tablespace, or an empty string
+ to specify using the default tablespace of the current database
+ unless the xref linkend=guc-default-tablespace is defined,
+ in which case the rules for that parameter apply.
+ If the value does not match the name of any existing tablespace,
+ productnamePostgreSQL/ will automatically use the default
+ tablespace of the current database; the user must have
+ literalCREATE/ privilege for it, or creation attempts will fail.
+/para
+
+para
+ This variable is not used for indexes on temporary tables; for them,
+ xref linkend=guc-temp-tablespaces is consulted instead.
+/para
+
+para
+ For more information on tablespaces,
+ see xref linkend=manage-ag-tablespaces.
+/para
+ /listitem
+ /varlistentry
+
varlistentry id=guc-temp-tablespaces xreflabel=temp_tablespaces
termvarnametemp_tablespaces/varname (typestring/type)
indexterm
*** a/src/backend/commands/indexcmds.c
--- b/src/backend/commands/indexcmds.c
***
*** 417,423 DefineIndex(Oid relationId,
}
else
{
! tablespaceId =
GetDefaultTablespace(rel-rd_rel-relpersistence);
/* note InvalidOid is OK in this case */
}
--- 417,423
}
else
{
! tablespaceId = GetIndexTablespace(rel-rd_rel-relpersistence);
/* note InvalidOid is OK in this case */
}
*** a/src/backend/commands/tablespace.c
--- b/src/backend/commands/tablespace.c
***
*** 86,91
--- 86,92
/* GUC variables */
char *default_tablespace = NULL;
+ char *default_index_tblspc = NULL;
char *temp_tablespaces = NULL;
***
*** 1085,1090 GetDefaultTablespace(char relpersistence)
--- 1086,1149
/*
+ * GetIndexTablespace -- get the OID of the tablespace for an index
+ *
+ * Temporary objects have different default tablespaces, hence the
+ * relpersistence parameter must be specified.
+ *
+ * May return InvalidOid to indicate use the database's default tablespace.
+ *
+ * Note that caller is expected to check appropriate permissions for any
+ * result other than InvalidOid.
+ *
+ * This exists to hide (and possibly optimize the use of) the
+ * default_index_tablespace GUC variable.
+ */
+ Oid
+ GetIndexTablespace(char relpersistence)
+ {
+ Oid result;
+ const char *tablespace;
+
+ /* The temp-table case is handled elsewhere */
+ if (relpersistence == RELPERSISTENCE_TEMP)
+ {
+ PrepareTempTablespaces();
+ return GetNextTempTableSpace();
+ }
+
+ /* Fast path for empty GUC: check defaults */
+ if (default_index_tblspc == NULL || default_index_tblspc[0] == '\0')
+ {
+ /* if default_tablespace is also empty, return immediately */
+ if (default_tablespace == NULL || default_tablespace[0] == '\0')
+ return InvalidOid;
+ else
+ tablespace = default_tablespace;
+ }
+ else
+ tablespace = default_index_tblspc;
+
+ /*
+* It is tempting to cache this lookup for more speed, but then we would
+* fail to detect the case where the tablespace was dropped since the
GUC
+* variable was set. Note also that we don't complain if the value
fails
+* to refer to an existing tablespace; we just silently return
InvalidOid,
+
Re: [HACKERS] PATCH: default_index_tablespace
jltal...@adv-solutions.net writes: This small patch implements a new GUC (default_index_tablespace) plus supporting code. Originated from a customer request, the feature intends to make creation of indexes on SSD-backed tablespaces easy and convenient (almost transparent) for users: the DBA can just set it and indexes will be placed in the specified tablespace --as opposed to the same tablespace where the referenced table is-- without having to specify it every time. I'm afraid this idea is a nonstarter, because it will break existing applications, and in particular existing pg_dump output files, which expect to be able to determine an index's tablespace by setting default_tablespace. (It is *not* adequate that the code falls back to default_tablespace if the new GUC is unset; if it is set, you've still broken pg_dump.) The incremental value, if indeed there is any, of being able to control index positioning this way seems unlikely to justify a backwards-compatibility break of such magnitude. 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] PATCH: default_index_tablespace
* Bruce Momjian (br...@momjian.us) wrote: On Wed, Apr 15, 2015 at 07:12:11PM -0400, Tom Lane wrote: jltal...@adv-solutions.net writes: This small patch implements a new GUC (default_index_tablespace) plus supporting code. Originated from a customer request, the feature intends to make creation of indexes on SSD-backed tablespaces easy and convenient (almost transparent) for users: the DBA can just set it and indexes will be placed in the specified tablespace --as opposed to the same tablespace where the referenced table is-- without having to specify it every time. I'm afraid this idea is a nonstarter, because it will break existing applications, and in particular existing pg_dump output files, which expect to be able to determine an index's tablespace by setting default_tablespace. (It is *not* adequate that the code falls back to default_tablespace if the new GUC is unset; if it is set, you've still broken pg_dump.) The incremental value, if indeed there is any, of being able to control index positioning this way seems unlikely to justify a backwards-compatibility break of such magnitude. I can see why someone would want this because random I/O, which is frequent for indexes, is much faster on SSD than magnetic disks. (Sequential I/O is more similar for the two.) The general idea is something I've brought up previously also (I believe it was even discussed at the Dev meeting in, uh, 2013?) so I'm not anxious to simply dismiss it, but it'd certainly have to be done correctly.. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] PATCH: default_index_tablespace
On Wed, Apr 15, 2015 at 07:12:11PM -0400, Tom Lane wrote: jltal...@adv-solutions.net writes: This small patch implements a new GUC (default_index_tablespace) plus supporting code. Originated from a customer request, the feature intends to make creation of indexes on SSD-backed tablespaces easy and convenient (almost transparent) for users: the DBA can just set it and indexes will be placed in the specified tablespace --as opposed to the same tablespace where the referenced table is-- without having to specify it every time. I'm afraid this idea is a nonstarter, because it will break existing applications, and in particular existing pg_dump output files, which expect to be able to determine an index's tablespace by setting default_tablespace. (It is *not* adequate that the code falls back to default_tablespace if the new GUC is unset; if it is set, you've still broken pg_dump.) The incremental value, if indeed there is any, of being able to control index positioning this way seems unlikely to justify a backwards-compatibility break of such magnitude. I can see why someone would want this because random I/O, which is frequent for indexes, is much faster on SSD than magnetic disks. (Sequential I/O is more similar for the two.) -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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] PATCH: default_index_tablespace
On Thu, Apr 16, 2015 at 8:01 AM, Bruce Momjian br...@momjian.us wrote: On Wed, Apr 15, 2015 at 07:12:11PM -0400, Tom Lane wrote: jltal...@adv-solutions.net writes: This small patch implements a new GUC (default_index_tablespace) plus supporting code. Originated from a customer request, the feature intends to make creation of indexes on SSD-backed tablespaces easy and convenient (almost transparent) for users: the DBA can just set it and indexes will be placed in the specified tablespace --as opposed to the same tablespace where the referenced table is-- without having to specify it every time. I'm afraid this idea is a nonstarter, because it will break existing applications, and in particular existing pg_dump output files, which expect to be able to determine an index's tablespace by setting default_tablespace. (It is *not* adequate that the code falls back to default_tablespace if the new GUC is unset; if it is set, you've still broken pg_dump.) The incremental value, if indeed there is any, of being able to control index positioning this way seems unlikely to justify a backwards-compatibility break of such magnitude. I can see why someone would want this because random I/O, which is frequent for indexes, is much faster on SSD than magnetic disks. (Sequential I/O is more similar for the two.) Another way to provide different default tablespace for index could be to provide it at Database level. Have a new option INDEX_TABLESPACE in Create Database command which can be used to create indexes when not specified during Create Index command. This would also need changes in pg_dump (like while dumping info about database) but on initial look, it seems it can be done without much changes. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
