Index build temp files

On Tue, Jan 8, 2013 at 05:09:47PM -0500, Stephen Frost wrote:

Greetings,

We were surprised recently to note that the temp files that are
created during a CREATE INDEX don't go into either a temp tablespace
set for the user or into the tablespace which the CREATE INDEX
specifies. Instead, they go only into base/pgsql_tmp/. This doesn't
allow for any flexibility in defining where to create these
potentially quite large sets of files.

Shouldn't these temp files be going into the temp tablespace for the
user creating the index instead..? Or perhaps into the tablespace
which the index is being created in?

Well, our docs for temp_tablespaces says:

This variable specifies tablespaces in which to create temporary
objects (temp tables and indexes on temp tables) when a
<command>CREATE</> command does not explicitly specify a tablespace.
Temporary files for purposes such as sorting large data sets
are also created in these tablespaces.

Are you saying this is inaccorate?

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

* Bruce Momjian (bruce@momjian.us) wrote:

On Tue, Jan 8, 2013 at 05:09:47PM -0500, Stephen Frost wrote:

Greetings,

We were surprised recently to note that the temp files that are
created during a CREATE INDEX don't go into either a temp tablespace
set for the user or into the tablespace which the CREATE INDEX
specifies. Instead, they go only into base/pgsql_tmp/. This doesn't
allow for any flexibility in defining where to create these
potentially quite large sets of files.

Shouldn't these temp files be going into the temp tablespace for the
user creating the index instead..? Or perhaps into the tablespace
which the index is being created in?

Well, our docs for temp_tablespaces says:

This variable specifies tablespaces in which to create temporary
objects (temp tables and indexes on temp tables) when a
<command>CREATE</> command does not explicitly specify a tablespace.
Temporary files for purposes such as sorting large data sets
are also created in these tablespaces.

Are you saying this is inaccorate?

Yes and no? Are the temporary files created during a CREATE INDEX
considered "Temporary files for purposes such as sorting large data
sets"? My thinking is 'yes', but others may disagree. Also,
considering this a bug would imply that it's back-patchable and I'm not
convinced it's worth the risk of breaking things which depend on the
current behavior.

Thanks,

Stephen

Stephen Frost <sfrost@snowman.net> writes:

* Bruce Momjian (bruce@momjian.us) wrote:

Well, our docs for temp_tablespaces says:
This variable specifies tablespaces in which to create temporary
objects (temp tables and indexes on temp tables) when a
<command>CREATE</> command does not explicitly specify a tablespace.
Temporary files for purposes such as sorting large data sets
are also created in these tablespaces.

Are you saying this is inaccorate?

Yes and no? Are the temporary files created during a CREATE INDEX
considered "Temporary files for purposes such as sorting large data
sets"? My thinking is 'yes', but others may disagree. Also,
considering this a bug would imply that it's back-patchable and I'm not
convinced it's worth the risk of breaking things which depend on the
current behavior.

I don't think it's a bug. What you seem to be proposing is that CREATE
INDEX ought to ignore temp_tablespaces and instead always put its temp
files in the tablespace where the finished index will reside. This
would not be a good idea IMO --- allowing the temp files to be spread to
other tablespaces is better both from the standpoint of space usage and
the ability to overlap I/O. (Admittedly, both of those concerns are
often theoretical, but if they are then I don't see why you'd care which
tablespace is used.)

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

On 9 January 2013 00:04, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I don't think it's a bug. What you seem to be proposing is that CREATE
INDEX ought to ignore temp_tablespaces and instead always put its temp
files in the tablespace where the finished index will reside.

I don't think that's what he said.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Simon Riggs <simon@2ndQuadrant.com> writes:

On 9 January 2013 00:04, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I don't think it's a bug. What you seem to be proposing is that CREATE
INDEX ought to ignore temp_tablespaces and instead always put its temp
files in the tablespace where the finished index will reside.

I don't think that's what he said.

Then I misunderstood. Stephen, what was your thought exactly about
what should happen?

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

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Simon Riggs <simon@2ndQuadrant.com> writes:

On 9 January 2013 00:04, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I don't think it's a bug. What you seem to be proposing is that CREATE
INDEX ought to ignore temp_tablespaces and instead always put its temp
files in the tablespace where the finished index will reside.

I don't think that's what he said.

Then I misunderstood. Stephen, what was your thought exactly about
what should happen?

Perhaps this is more of a bug than I originally thought, given the
confusion and general expectation.

Here's what we're seeing:

postgresql.conf:
temp_tablespaces = 'temp_01,temp_02'

I have temp file logging on in postgresql.conf, so here's what we see:

LOG: temporary file: path "base/pgsql_tmp/pgsql_tmp9221.4", size 204521472
CONTEXT: SQL statement "create index table_key_idx on table (key) tablespace data_n04"

We observe the files being created under $DATA/base/pgsql_tmp/, ignoring
the temp tablespaces and not using the tablespace where the index is
ultimately created either. I was speculating earlier that it should use
one or the other. I'd be perfectly happy if it used one of the temp
tablespaces, but currently it's using pg_default regardless of the other
settings.

Thanks,

Stephen

Stephen Frost <sfrost@snowman.net> writes:

Here's what we're seeing:

postgresql.conf:
temp_tablespaces = 'temp_01,temp_02'

I have temp file logging on in postgresql.conf, so here's what we see:

LOG: temporary file: path "base/pgsql_tmp/pgsql_tmp9221.4", size 204521472
CONTEXT: SQL statement "create index table_key_idx on table (key) tablespace data_n04"

We observe the files being created under $DATA/base/pgsql_tmp/, ignoring
the temp tablespaces and not using the tablespace where the index is
ultimately created either.

Does the user running CREATE INDEX have CREATE permission on those
tablespaces? (A quick way to double check is to try to SET
temp_tablespaces to that value explicitly.) The code will silently
ignore any temp tablespaces you don't have such permission for.

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

Tom,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Does the user running CREATE INDEX have CREATE permission on those
tablespaces? (A quick way to double check is to try to SET
temp_tablespaces to that value explicitly.) The code will silently
ignore any temp tablespaces you don't have such permission for.

Great point, thanks for that. While working up a test case, I was able
to get it to use the temp tablespace. In looking back at the actual
system, it turns out that the load process (which creates the indexes)
doesn't have rights on those temp tablespaces.

If I can come up with a doc or similar patch, I'll post it, but I have
to admit that the docs are pretty clear that you need create rights.
Still, it seems a little odd that we don't complain in any way in this
case. Perhaps a NOTICE when the temp_tablespaces value is set but won't
be used due to permissions..?

Thanks,

Stephen

On Tue, 2013-01-08 at 23:38 -0500, Stephen Frost wrote:

If I can come up with a doc or similar patch, I'll post it, but I have
to admit that the docs are pretty clear that you need create rights.
Still, it seems a little odd that we don't complain in any way in this
case. Perhaps a NOTICE when the temp_tablespaces value is set but
won't be used due to permissions..?

It should be a warning in my opinion.

It should actually be an error, but there are probably reasons why that
wouldn't work.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Stephen Frost <sfrost@snowman.net> writes:

If I can come up with a doc or similar patch, I'll post it, but I have
to admit that the docs are pretty clear that you need create rights.
Still, it seems a little odd that we don't complain in any way in this
case. Perhaps a NOTICE when the temp_tablespaces value is set but won't
be used due to permissions..?

The code definitely will complain if you try to interactively SET
temp_tablespaces to a space you lack permissions for. However, there
has never been a case in which people would hold still for warnings
emitted as a consequence of values read from postgresql.conf or other
background sources, and I doubt that the response would be different
if we made this variable act like that. See for example past
discussions about what to do with invalid entries in search_path.

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

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

The code definitely will complain if you try to interactively SET
temp_tablespaces to a space you lack permissions for. However, there
has never been a case in which people would hold still for warnings
emitted as a consequence of values read from postgresql.conf or other
background sources, and I doubt that the response would be different
if we made this variable act like that. See for example past
discussions about what to do with invalid entries in search_path.

Indeed, I fully expected the comparison argument to search_path, but I
have to admit that search_path feels a great deal more like CWD, while
the temp tablespaces is more like trying to write to /tmp and getting an
error.

That is to say, tablespaces and in particular temp tablespaces are much
more 'system' level than search paths. I don't expect regular users to
change their temp tablepace, while I expect them to change their search
path on a regular basis.

In any case, I was suggesting a NOTICE rather than a WARNING, though I
appreciate that both could make noise for users who don't expect it.
Still, I don't expect many users would complain about this, while they
would complain about a similar thing for search_path. Perhaps that's
not how they *should* act, but humans aren't always logical. :)

Thanks,

Stephen

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Does the user running CREATE INDEX have CREATE permission on those
tablespaces? (A quick way to double check is to try to SET
temp_tablespaces to that value explicitly.) The code will silently
ignore any temp tablespaces you don't have such permission for.

Alright, this isn't quite as open-and-shut as it may have originally
seemed. We're apparently cacheing the temp tablespaces which should be
used, even across set role's and security definer functions, which I
would argue isn't correct.

Attached are two test scripts and results from them. The gist of it is
this:

With the first script, a privileged user creates a temp table and this
table ends up in a tablespace which that user has access to. In the
same session, the user drops privileges using a 'set role' to a less
privileged user (who doesn't have access to the same tablespaces) and
then tries to create a temporary table- boom: permission denied error.

In the second script, an unprivileged user creates a temp table, which
goes into the default tablespace (this is fine- there are other temp
tablespaces, but this user doesn't have access to them), but then calls
a SECURITY DEFINER function, which runs as a privileged user who DOES
have access to the temp tablespaces defined by default. What ends up
happening, however, is that the temporary table created during the
security definer function ends up going into the default tablespace
instead. If the security definer function calls 'set temp_tablespaces',
before creating the temp table, it'll go into the correct tablespace but
after the security definer function ends, if the regular user tries to
create a temporary table they'll get a permission denied error.

I've not gone and looked at any of the code behind this yet (hopefully
will be able to soon). It seems like we need to modify the cacheing
behavior of the temp tablespace code to account for the role that is
currently active. This can't work quite like search_path since we want
to reuse the same tablespace, if possible, for the duration of a
session, per the docs.

Thoughts?

Thanks,

Stephen

* Stephen Frost (sfrost@snowman.net) wrote:

Attached are two test scripts and results from them. The gist of it is
this:

Now with the attachements. Apologies about that. Note that you'll need
to create the temp tablespace first.

Thanks,

Stephen

On 9 January 2013 02:42, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Stephen Frost <sfrost@snowman.net> writes:

Here's what we're seeing:

postgresql.conf:
temp_tablespaces = 'temp_01,temp_02'

I have temp file logging on in postgresql.conf, so here's what we see:

LOG: temporary file: path "base/pgsql_tmp/pgsql_tmp9221.4", size 204521472
CONTEXT: SQL statement "create index table_key_idx on table (key) tablespace data_n04"

We observe the files being created under $DATA/base/pgsql_tmp/, ignoring
the temp tablespaces and not using the tablespace where the index is
ultimately created either.

Does the user running CREATE INDEX have CREATE permission on those
tablespaces? (A quick way to double check is to try to SET
temp_tablespaces to that value explicitly.) The code will silently
ignore any temp tablespaces you don't have such permission for.

I think we need to allow a TEMP permission on tablespaces, so that you
aren't allowed to create normal objects but you can create TEMP
objects and sort files there.

I think SHOW temp_tablespaces should only show the valid tablespaces,
not all of the ones actually listed in the parameter, otherwise you
have no clue that the parameter setting is ineffectual for you.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Stephen Frost <sfrost@snowman.net> writes:

Alright, this isn't quite as open-and-shut as it may have originally
seemed. We're apparently cacheing the temp tablespaces which should be
used, even across set role's and security definer functions, which I
would argue isn't correct.

Ah. Yeah, that would be true.

We do have mechanism that forces search_path to be recomputed across
changes of active role, but it's expensive to do that, and it seems
of rather debatable value to do it here --- it certainly wouldn't
improve Stephen's original problem, much less the other issues he
raises here.

What would people think of just eliminating the access-permissions
checks involved in temp_tablespaces? It would likely be appropriate to
change temp_tablespaces from USERSET to SUSET if we did so. So
essentially the worldview would become that the DBA is responsible for
the temp_tablespaces setting, not individual users.

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

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

We do have mechanism that forces search_path to be recomputed across
changes of active role, but it's expensive to do that, and it seems
of rather debatable value to do it here --- it certainly wouldn't
improve Stephen's original problem, much less the other issues he
raises here.

I agree that we would need something more for users to be able to be
able to tell what temp tablespace their temporary tables will end up in.
Generally, I think it'd be acceptable from a performance perspective to
figure out where to create the temporary objects at the time that they
are requested- at that point, you're going to be creating a new table,
index, or doing a large sort that spills to disk, all of which are
relatively heavy-weight operations.

What would people think of just eliminating the access-permissions
checks involved in temp_tablespaces? It would likely be appropriate to
change temp_tablespaces from USERSET to SUSET if we did so. So
essentially the worldview would become that the DBA is responsible for
the temp_tablespaces setting, not individual users.

I believe it's important to be able to control what temp tablespaces are
used on a per-user basis and that 'set role;' or security definer
functions respect the tablespace which has been set for the current
role. Temp tablespaces are extremely valuable for managing users who
unintentionally write run-away queries that fill up the tablespace.
Keeping those seperate from the temp tablespace used for SECURITY
DEFINER functions (which are written with care) or other ongoing system
activity is necessary. Perhaps there would be a way to still do that
with your approach, but it didn't sound like it initially.

Perhaps we can simply keep track of what the current role was when we
cache'd which temp tablespace was selected for a given session and, if
the current role has changed, reconsider the temp tablespace selected?
We would need to update the documentation to reflect that you might not
*always* have the same tablespace for a session, if there are security
definer and/or set role's happening, but that seems reasonable to me.

Thanks,

Stephen

* Simon Riggs (simon@2ndQuadrant.com) wrote:

I think we need to allow a TEMP permission on tablespaces, so that you
aren't allowed to create normal objects but you can create TEMP
objects and sort files there.

I agree that this would be valuable. Would we end up needing to burn
another bit off the aclmask though? That's certainly been a concern in
the past and I don't recall that we ever improved that situation.

I think SHOW temp_tablespaces should only show the valid tablespaces,
not all of the ones actually listed in the parameter, otherwise you
have no clue that the parameter setting is ineffectual for you.

I like this as well.

Thanks,

Stephen

Stephen Frost <sfrost@snowman.net> writes:

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

We do have mechanism that forces search_path to be recomputed across
changes of active role, but it's expensive to do that, and it seems
of rather debatable value to do it here --- it certainly wouldn't
improve Stephen's original problem, much less the other issues he
raises here.

Generally, I think it'd be acceptable from a performance perspective to
figure out where to create the temporary objects at the time that they
are requested- at that point, you're going to be creating a new table,
index, or doing a large sort that spills to disk, all of which are
relatively heavy-weight operations.

It's not so much performance I'm worried about here as
modularity/layering issues. Actual use of the temp-tablespaces list to
create temp files is in fd.c, which has no business invoking catalog
accesses, so we can't just say "we'll recheck the permissions when we're
going to create some temp files". In any case this wouldn't improve the
behavior you were originally on about, which was surprising (to you)
failure to use a DBA-specified temp_tablespaces list.

What would people think of just eliminating the access-permissions
checks involved in temp_tablespaces? It would likely be appropriate to
change temp_tablespaces from USERSET to SUSET if we did so. So
essentially the worldview would become that the DBA is responsible for
the temp_tablespaces setting, not individual users.

I believe it's important to be able to control what temp tablespaces are
used on a per-user basis and that 'set role;' or security definer
functions respect the tablespace which has been set for the current
role. Temp tablespaces are extremely valuable for managing users who
unintentionally write run-away queries that fill up the tablespace.

IIRC, we do have mechanism now whereby a superuser can establish settings
for SUSET variables via ALTER ROLE SET/ALTER DATABASE SET, and
everything works as expected. So the only loss of flexibility here
would be if you want unprivileged code to be able to set
temp_tablespaces for itself. However, if you want that then it's hard
to argue that there shouldn't be a permissions check, and then we're
back into the surprises mentioned previously.

It might be possible to compromise on an arrangement whereby tablespace
permissions checking only occurs if the active value of the variable was
set by a non-superuser. But TBH that idea fills me with dread --- we
don't have any other GUCs that work like that, and it seems too likely
that there would be conceptual or implementation bugs in it.

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

On 9 January 2013 20:53, Stephen Frost <sfrost@snowman.net> wrote:

* Simon Riggs (simon@2ndQuadrant.com) wrote:

I think we need to allow a TEMP permission on tablespaces, so that you
aren't allowed to create normal objects but you can create TEMP
objects and sort files there.

I agree that this would be valuable. Would we end up needing to burn
another bit off the aclmask though? That's certainly been a concern in
the past and I don't recall that we ever improved that situation.

There already is a TEMP permission, it just only applies to databases
at present.

I think SHOW temp_tablespaces should only show the valid tablespaces,
not all of the ones actually listed in the parameter, otherwise you
have no clue that the parameter setting is ineffectual for you.

I like this as well.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers