PATCH: default_index_tablespace

Lists: pgsql-hackers
From: jltallon(at)adv-solutions(dot)net
To: <pgsql-hackers(at)postgresql(dot)org>
Subject: PATCH: default_index_tablespace
Date: 2015-04-15 21:53:35
Message-ID: b040657607ba76fc55a311bbcec69da4@adv-solutions.net
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-hackers

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.

Attachment Content-Type Size
DITv1.git.patch text/plain 6.1 KB

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: jltallon(at)adv-solutions(dot)net
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: PATCH: default_index_tablespace
Date: 2015-04-15 23:12:11
Message-ID: 3600.1429139531@sss.pgh.pa.us
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-hackers

jltallon(at)adv-solutions(dot)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


From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: jltallon(at)adv-solutions(dot)net, pgsql-hackers(at)postgresql(dot)org
Subject: Re: PATCH: default_index_tablespace
Date: 2015-04-16 02:31:46
Message-ID: 20150416023146.GD1672@momjian.us
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-hackers

On Wed, Apr 15, 2015 at 07:12:11PM -0400, Tom Lane wrote:
> jltallon(at)adv-solutions(dot)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 <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +


From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, jltallon(at)adv-solutions(dot)net, pgsql-hackers(at)postgresql(dot)org
Subject: Re: PATCH: default_index_tablespace
Date: 2015-04-16 02:44:48
Message-ID: 20150416024448.GS3663@tamriel.snowman.net
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-hackers

* Bruce Momjian (bruce(at)momjian(dot)us) wrote:
> On Wed, Apr 15, 2015 at 07:12:11PM -0400, Tom Lane wrote:
> > jltallon(at)adv-solutions(dot)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


From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, José Luis Tallón <jltallon(at)adv-solutions(dot)net>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PATCH: default_index_tablespace
Date: 2015-04-16 03:33:25
Message-ID: CAA4eK1L9VwivbZGOA_uZrk0ci6cQ_JJR8ShJxf52LmWuTgd5LQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-hackers

On Thu, Apr 16, 2015 at 8:01 AM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>
> On Wed, Apr 15, 2015 at 07:12:11PM -0400, Tom Lane wrote:
> > jltallon(at)adv-solutions(dot)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


From: Greg Stark <stark(at)mit(dot)edu>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: jltallon(at)adv-solutions(dot)net, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PATCH: default_index_tablespace
Date: 2015-04-16 09:17:57
Message-ID: CAM-w4HPOASwsQMdGZqjyFHNubbUnWrUAo8ibci-97UKU=poDbg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-hackers

On 15 Apr 2015 19:12, "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)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".


From: David Steele <david(at)pgmasters(dot)net>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, José Luis Tallón <jltallon(at)adv-solutions(dot)net>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PATCH: default_index_tablespace
Date: 2015-04-16 13:51:46
Message-ID: 552FBE72.2000207@pgmasters.net
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-hackers

On 4/15/15 11:33 PM, Amit Kapila wrote:
> On Thu, Apr 16, 2015 at 8:01 AM, Bruce Momjian <bruce(at)momjian(dot)us
> <mailto:bruce(at)momjian(dot)us>> wrote:
>>
>> On Wed, Apr 15, 2015 at 07:12:11PM -0400, Tom Lane wrote:
>> > jltallon(at)adv-solutions(dot)net <mailto:jltallon(at)adv-solutions(dot)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
david(at)pgmasters(dot)net


From: jltallon(at)adv-solutions(dot)net
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: Bruce Momjian <bruce(at)momjian(dot)us>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Greg Stark <stark(at)mit(dot)edu>, <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PATCH: default_index_tablespace
Date: 2015-04-16 16:50:36
Message-ID: e8dc762395ac3adc661c96a39021ed18@adv-solutions.net
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-hackers

>> >
>> > 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=poDbg(at)mail(dot)gmail(dot)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.


From: Stephen Frost <sfrost(at)snowman(dot)net>
To: jltallon(at)adv-solutions(dot)net
Cc: Bruce Momjian <bruce(at)momjian(dot)us>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Greg Stark <stark(at)mit(dot)edu>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: PATCH: default_index_tablespace
Date: 2015-04-26 18:28:51
Message-ID: 20150426182851.GE30322@tamriel.snowman.net
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-hackers

J.L.,

* jltallon(at)adv-solutions(dot)net (jltallon(at)adv-solutions(dot)net) wrote:
> Any suggestions on how to do it "properly"?
> Does Greg Stark's suggestion (at
> <CAM-w4HPOASwsQMdGZqjyFHNubbUnWrUAo8ibci-97UKU=poDbg(at)mail(dot)gmail(dot)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