How about a proper TEMPORARY TABLESPACE?
Hi Hackers,
I was facing a situation were we wanted to set temp_tablespaces to a
tablespace on a ephemeral disk (yes, it is AWS ephemeral disk), and I know
many users have faced the same situation. Although it seems safe to create
a tablespace on ephemeral disks if you use it to store only temporary files
(either created by queries or temp tables), PostgreSQL does not have such
information, and can't safely prevent a careless user of creating a
non-temporary relation on this tablespace.
Also, in case of a lost of this tablespace, its PG_TEMP_FILES_DIR should be
created by hand after recovering. That is fine actually, but many users may
not even noticed that.
So, what you guys think about letting PG know somehow that a tablespace is
temporary?
I have took some small time to make a PoC just to see if that is doable.
And so I did a new syntax like:
CREATE TABLESPACE spcname [TEMP | TEMPORARY] LOCATION ...
So, if TEMP or TEMPORARY is present, I mark a new column at pg_tablespace
as true. On every table creation or moving to a new tablespace, I just
check this, and fails if the tablespace is "temporary" but the
"relpersistence" says the table is not.
The other part is, every time some query or relation is asked to be stored
on this tablespace, I create (on-demand) the PG_TEMP_FILES_DIR inside of it
(also if it is temporary).
The attached patch (and also on my Github, [1]https://github.com/matheusoliveira/postgres/compare/my_temptablespace), shows the PoC. For now,
I'm not worried about the code quality, there are yet a lot of work to be
done there (like ALTER TABLESPACE, better testing, use relcache, etc...),
and it is my first hacking on PG (so I'm a newbie). But I'd like to hear
from you guys if such feature is desirable and if I could starting working
on that for real. Also some thoughts about better way of implementing it.
[1]: https://github.com/matheusoliveira/postgres/compare/my_temptablespace
Waiting for thoughts on that.
Best regards,
--
Matheus de Oliveira
Analista de Banco de Dados
Dextra Sistemas - MPS.Br nível F!
www.dextra.com.br/postgres
Attachments:
temptablespace.patchtext/x-patch; charset=US-ASCII; name=temptablespace.patchDownload
*** a/src/backend/commands/dbcommands.c
--- b/src/backend/commands/dbcommands.c
***************
*** 388,399 **** createdb(const CreatedbStmt *stmt)
--- 388,406 ----
aclcheck_error(aclresult, ACL_KIND_TABLESPACE,
tablespacename);
+
/* pg_global must never be the default tablespace */
if (dst_deftablespace == GLOBALTABLESPACE_OID)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("pg_global cannot be used as default tablespace")));
+ /* can't create a database on temporary tablespace */
+ if (is_tablespace_storage_temporary(dst_deftablespace))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("cannot create a database on a tablespace in temporary storage")));
+
/*
* If we are trying to change the default tablespace of the template,
* we require that the template not have any files in the new default
***************
*** 1083,1088 **** movedb(const char *dbname, const char *tblspcname)
--- 1090,1101 ----
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("pg_global cannot be used as default tablespace")));
+ /* can't create a database on temporary tablespace */
+ if (is_tablespace_storage_temporary(dst_tblspcoid))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("cannot create a database on a tablespace in temporary storage")));
+
/*
* No-op if same tablespace
*/
*** a/src/backend/commands/indexcmds.c
--- b/src/backend/commands/indexcmds.c
***************
*** 432,437 **** DefineIndex(Oid relationId,
--- 432,446 ----
get_tablespace_name(tablespaceId));
}
+ /* Can't save relations on temporary tablespace */
+ if (rel->rd_rel->relpersistence != RELPERSISTENCE_TEMP &&
+ is_tablespace_storage_temporary(OidIsValid(tablespaceId) ? tablespaceId : MyDatabaseTableSpace))
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("cannot save relation on a tablespace in temporary storage")));
+ }
+
/*
* Force shared indexes into the pg_global tablespace. This is a bit of a
* hack but seems simpler than marking them in the BKI commands. On the
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
***************
*** 523,528 **** DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId)
--- 523,537 ----
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("only shared relations can be placed in pg_global tablespace")));
+ /* Can't save relations on temporary tablespace */
+ if (stmt->relation->relpersistence != RELPERSISTENCE_TEMP &&
+ is_tablespace_storage_temporary(OidIsValid(tablespaceId) ? tablespaceId : MyDatabaseTableSpace))
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("cannot save relation on a tablespace in temporary storage")));
+ }
+
/* Identify user ID that will own the table */
if (!OidIsValid(ownerId))
ownerId = GetUserId();
***************
*** 8825,8830 **** ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel, char *tablespacename, L
--- 8834,8848 ----
aclcheck_error(aclresult, ACL_KIND_TABLESPACE, tablespacename);
}
+ /* Can't save relations on temporary tablespace */
+ if (rel->rd_rel->relpersistence != RELPERSISTENCE_TEMP &&
+ is_tablespace_storage_temporary(OidIsValid(tablespaceId) ? tablespaceId : MyDatabaseTableSpace))
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("cannot save relation on a tablespace in temporary storage")));
+ }
+
/* Save info for Phase 3 to do the real work */
if (OidIsValid(tab->newTableSpace))
ereport(ERROR,
*** a/src/backend/commands/tablespace.c
--- b/src/backend/commands/tablespace.c
***************
*** 154,170 **** TablespaceCreateDbspace(Oid spcNode, Oid dbNode, bool isRedo)
{
char *parentdir;
! /* Failure other than not exists or not in WAL replay? */
! if (errno != ENOENT || !isRedo)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not create directory \"%s\": %m",
dir)));
/*
! * Parent directories are missing during WAL replay, so
! * continue by creating simple parent directories rather
! * than a symlink.
*/
/* create two parents up if not exist */
--- 154,171 ----
{
char *parentdir;
! /* Failure other than not exists or not in WAL replay with a non-temp tablespace? */
! if (errno != ENOENT || !( isRedo || is_tablespace_storage_temporary(spcNode) ) )
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not create directory \"%s\": %m",
dir)));
/*
! * Parent directories are missing during WAL replay, and
! * they can be missing for temp tablespaces, so continue
! * by creating simple parent directories rather than a
! * symlink.
*/
/* create two parents up if not exist */
***************
*** 322,327 **** CreateTableSpace(CreateTableSpaceStmt *stmt)
--- 323,330 ----
DirectFunctionCall1(namein, CStringGetDatum(stmt->tablespacename));
values[Anum_pg_tablespace_spcowner - 1] =
ObjectIdGetDatum(ownerId);
+ values[Anum_pg_tablespace_spcistempstorage - 1] =
+ stmt->temporary;
nulls[Anum_pg_tablespace_spcacl - 1] = true;
/* Generate new proposed spcoptions (text array) */
***************
*** 1573,1578 **** get_tablespace_name(Oid spc_oid)
--- 1576,1621 ----
return result;
}
+ /*
+ * is_tablespace_storage_temporary - given a tablespace OID, check if the
+ * underline storage is temporary
+ */
+ bool
+ is_tablespace_storage_temporary(Oid spc_oid)
+ {
+ char *result;
+ Relation rel;
+ HeapScanDesc scandesc;
+ HeapTuple tuple;
+ ScanKeyData entry[1];
+
+ /*
+ * Search pg_tablespace. We use a heapscan here even though there is an
+ * index on oid, on the theory that pg_tablespace will usually have just a
+ * few entries and so an indexed lookup is a waste of effort.
+ */
+ rel = heap_open(TableSpaceRelationId, AccessShareLock);
+
+ ScanKeyInit(&entry[0],
+ ObjectIdAttributeNumber,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(spc_oid));
+ scandesc = heap_beginscan_catalog(rel, 1, entry);
+ tuple = heap_getnext(scandesc, ForwardScanDirection);
+
+ /* We assume that there can be at most one matching tuple */
+ /* TODO: Should we check this? If not valid, what is the best assumption? */
+ if (HeapTupleIsValid(tuple))
+ result = ((Form_pg_tablespace) GETSTRUCT(tuple))->spcistempstorage;
+ else
+ result = true;
+
+ heap_endscan(scandesc);
+ heap_close(rel, AccessShareLock);
+
+ return result;
+ }
+
/*
* TABLESPACE resource manager's routines
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***************
*** 476,482 **** static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <str> ExistingIndex
%type <list> constraints_set_list
! %type <boolean> constraints_set_mode
%type <str> OptTableSpace OptConsTableSpace OptTableSpaceOwner
%type <ival> opt_check_option
--- 476,482 ----
%type <str> ExistingIndex
%type <list> constraints_set_list
! %type <boolean> constraints_set_mode OptTableSpaceTemporary
%type <str> OptTableSpace OptConsTableSpace OptTableSpaceOwner
%type <ival> opt_check_option
***************
*** 3594,3606 **** opt_procedural:
*
*****************************************************************************/
! CreateTableSpaceStmt: CREATE TABLESPACE name OptTableSpaceOwner LOCATION Sconst opt_reloptions
{
CreateTableSpaceStmt *n = makeNode(CreateTableSpaceStmt);
n->tablespacename = $3;
n->owner = $4;
! n->location = $6;
! n->options = $7;
$$ = (Node *) n;
}
;
--- 3594,3607 ----
*
*****************************************************************************/
! CreateTableSpaceStmt: CREATE TABLESPACE name OptTableSpaceOwner OptTableSpaceTemporary LOCATION Sconst opt_reloptions
{
CreateTableSpaceStmt *n = makeNode(CreateTableSpaceStmt);
n->tablespacename = $3;
n->owner = $4;
! n->temporary = $5;
! n->location = $7;
! n->options = $8;
$$ = (Node *) n;
}
;
***************
*** 3609,3614 **** OptTableSpaceOwner: OWNER name { $$ = $2; }
--- 3610,3621 ----
| /*EMPTY */ { $$ = NULL; }
;
+ OptTableSpaceTemporary:
+ TEMPORARY { $$ = true; }
+ | TEMP { $$ = true; }
+ | /*EMPTY*/ { $$ = false; }
+ ;
+
/*****************************************************************************
*
* QUERY :
*** a/src/backend/storage/file/fd.c
--- b/src/backend/storage/file/fd.c
***************
*** 1083,1088 **** OpenTemporaryFileInTablespace(Oid tblspcOid, bool rejectError)
--- 1083,1089 ----
char tempdirpath[MAXPGPATH];
char tempfilepath[MAXPGPATH];
File file;
+ int parentlevel;
/*
* Identify the tempfile directory for this tablespace.
***************
*** 1092,1097 **** OpenTemporaryFileInTablespace(Oid tblspcOid, bool rejectError)
--- 1093,1099 ----
if (tblspcOid == DEFAULTTABLESPACE_OID ||
tblspcOid == GLOBALTABLESPACE_OID)
{
+ parentlevel = 0;
/* The default tablespace is {datadir}/base */
snprintf(tempdirpath, sizeof(tempdirpath), "base/%s",
PG_TEMP_FILES_DIR);
***************
*** 1099,1106 **** OpenTemporaryFileInTablespace(Oid tblspcOid, bool rejectError)
else
{
/* All other tablespaces are accessed via symlinks */
! snprintf(tempdirpath, sizeof(tempdirpath), "pg_tblspc/%u/%s/%s",
! tblspcOid, TABLESPACE_VERSION_DIRECTORY, PG_TEMP_FILES_DIR);
}
/*
--- 1101,1111 ----
else
{
/* All other tablespaces are accessed via symlinks */
! parentlevel = snprintf(tempdirpath, sizeof(tempdirpath), "pg_tblspc/%u/%s",
! tblspcOid, TABLESPACE_VERSION_DIRECTORY);
! tempdirpath[parentlevel] = '/';
! snprintf(tempdirpath + parentlevel + 1, sizeof(tempdirpath) - (parentlevel + 1), "%s",
! PG_TEMP_FILES_DIR);
}
/*
***************
*** 1127,1132 **** OpenTemporaryFileInTablespace(Oid tblspcOid, bool rejectError)
--- 1132,1143 ----
* just did the same thing. If it doesn't work then we'll bomb out on
* the second create attempt, instead.
*/
+ if (parentlevel > 0)
+ {
+ tempdirpath[parentlevel] = '\0';
+ mkdir(tempdirpath, S_IRWXU);
+ tempdirpath[parentlevel] = '/';
+ }
mkdir(tempdirpath, S_IRWXU);
file = PathNameOpenFile(tempfilepath,
*** a/src/include/catalog/pg_tablespace.h
--- b/src/include/catalog/pg_tablespace.h
***************
*** 32,37 **** CATALOG(pg_tablespace,1213) BKI_SHARED_RELATION
--- 32,38 ----
{
NameData spcname; /* tablespace name */
Oid spcowner; /* owner of tablespace */
+ bool spcistempstorage; /* T if the tablespace only store temporary files */
#ifdef CATALOG_VARLEN /* variable-length fields start here */
aclitem spcacl[1]; /* access permissions */
***************
*** 51,64 **** typedef FormData_pg_tablespace *Form_pg_tablespace;
* ----------------
*/
! #define Natts_pg_tablespace 4
! #define Anum_pg_tablespace_spcname 1
! #define Anum_pg_tablespace_spcowner 2
! #define Anum_pg_tablespace_spcacl 3
! #define Anum_pg_tablespace_spcoptions 4
! DATA(insert OID = 1663 ( pg_default PGUID _null_ _null_ ));
! DATA(insert OID = 1664 ( pg_global PGUID _null_ _null_ ));
#define DEFAULTTABLESPACE_OID 1663
#define GLOBALTABLESPACE_OID 1664
--- 52,66 ----
* ----------------
*/
! #define Natts_pg_tablespace 5
! #define Anum_pg_tablespace_spcname 1
! #define Anum_pg_tablespace_spcowner 2
! #define Anum_pg_tablespace_spcistempstorage 3
! #define Anum_pg_tablespace_spcacl 4
! #define Anum_pg_tablespace_spcoptions 5
! DATA(insert OID = 1663 ( pg_default PGUID false _null_ _null_ ));
! DATA(insert OID = 1664 ( pg_global PGUID false _null_ _null_ ));
#define DEFAULTTABLESPACE_OID 1663
#define GLOBALTABLESPACE_OID 1664
*** a/src/include/commands/tablespace.h
--- b/src/include/commands/tablespace.h
***************
*** 53,58 **** extern void PrepareTempTablespaces(void);
--- 53,59 ----
extern Oid get_tablespace_oid(const char *tablespacename, bool missing_ok);
extern char *get_tablespace_name(Oid spc_oid);
+ extern bool is_tablespace_storage_temporary(Oid spc_oid);
extern bool directory_is_empty(const char *path);
*** a/src/include/nodes/parsenodes.h
--- b/src/include/nodes/parsenodes.h
***************
*** 1667,1672 **** typedef struct CreateTableSpaceStmt
--- 1667,1673 ----
NodeTag type;
char *tablespacename;
char *owner;
+ bool temporary;
char *location;
List *options;
} CreateTableSpaceStmt;
* Matheus de Oliveira (matioli.matheus@gmail.com) wrote:
Hi Hackers,
Having read only the subject- +1 from me on the idea. Maybe +1000.
I was facing a situation were we wanted to set temp_tablespaces to a
tablespace on a ephemeral disk (yes, it is AWS ephemeral disk), and I know
many users have faced the same situation. Although it seems safe to create
a tablespace on ephemeral disks if you use it to store only temporary files
(either created by queries or temp tables), PostgreSQL does not have such
information, and can't safely prevent a careless user of creating a
non-temporary relation on this tablespace.
Right.
So, what you guys think about letting PG know somehow that a tablespace is
temporary?
PG would need to enforce that it's only used for temporary objects as
well, of course.. Or at least, that was my thinking on this.
I have took some small time to make a PoC just to see if that is doable.
And so I did a new syntax like:CREATE TABLESPACE spcname [TEMP | TEMPORARY] LOCATION ...
So, if TEMP or TEMPORARY is present, I mark a new column at pg_tablespace
as true. On every table creation or moving to a new tablespace, I just
check this, and fails if the tablespace is "temporary" but the
"relpersistence" says the table is not.
Not sure about that specific syntax (don't we have SET options now?) but
I do like the general idea.
The other part is, every time some query or relation is asked to be stored
on this tablespace, I create (on-demand) the PG_TEMP_FILES_DIR inside of it
(also if it is temporary).The attached patch (and also on my Github, [1]), shows the PoC. For now,
I'm not worried about the code quality, there are yet a lot of work to be
done there (like ALTER TABLESPACE, better testing, use relcache, etc...),
and it is my first hacking on PG (so I'm a newbie). But I'd like to hear
from you guys if such feature is desirable and if I could starting working
on that for real. Also some thoughts about better way of implementing it.
Yeah, +1 here and it should go into an appropriate commitfest to get a
proper review. This would be *really* nice to have.
Thanks,
Stephen
On Wed, Jun 18, 2014 at 9:00 AM, Stephen Frost <sfrost@snowman.net> wrote:
I have took some small time to make a PoC just to see if that is doable.
And so I did a new syntax like:CREATE TABLESPACE spcname [TEMP | TEMPORARY] LOCATION ...
So, if TEMP or TEMPORARY is present, I mark a new column at
pg_tablespace
as true. On every table creation or moving to a new tablespace, I just
check this, and fails if the tablespace is "temporary" but the
"relpersistence" says the table is not.Not sure about that specific syntax (don't we have SET options now?) but
I do like the general idea.
Maybe something like that:
CREATE TABLESPACE spcname LOCATION '/foo/bar' WITH (only_temp_relations =
true);
Have in mind you must take care if you use ALTER TABLESPACE spcname SET
(...) to guarantee that exists only temp objects stored in the target
tablespace, and if exists a regular object you must throw an exception.
Makes sense?
Regards,
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Show quoted text
Timbira: http://www.timbira.com.br
Blog sobre TI: http://fabriziomello.blogspot.com
Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
* Fabrízio de Royes Mello (fabriziomello@gmail.com) wrote:
On Wed, Jun 18, 2014 at 9:00 AM, Stephen Frost <sfrost@snowman.net> wrote:
Not sure about that specific syntax (don't we have SET options now?) but
I do like the general idea.Maybe something like that:
CREATE TABLESPACE spcname LOCATION '/foo/bar' WITH (only_temp_relations =
true);
Yeah, that's more along the lines of what I was thinking.
Have in mind you must take care if you use ALTER TABLESPACE spcname SET
(...) to guarantee that exists only temp objects stored in the target
tablespace, and if exists a regular object you must throw an exception.Makes sense?
Yes. I'd definitely like to see an ALTER TABLESPACE option, with an
ERROR that lists out all of the non-temporary objects which are found
(and lists any other databases which have objects in those
tablespaces..). That would allow administrators who have existing
notionally temporary-only tablespaces to go clean things up to make them
actually temporary-only.
Thanks,
Stephen
Stephen Frost <sfrost@snowman.net> writes:
Yes. I'd definitely like to see an ALTER TABLESPACE option, with an
ERROR that lists out all of the non-temporary objects which are found
(and lists any other databases which have objects in those
tablespaces..). That would allow administrators who have existing
notionally temporary-only tablespaces to go clean things up to make them
actually temporary-only.
That seems just about impossible from a concurrency standpoint
(ie, what if someone is creating a new table in the tablespace
concurrently with your check? Possibly in a different database?)
I would certainly suggest that the first version of the patch not
undertake to allow this property to be ALTERed; the cost-benefit
ratio isn't there IMO.
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 Wed, Jun 18, 2014 at 1:59 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Stephen Frost <sfrost@snowman.net> writes:
Yes. I'd definitely like to see an ALTER TABLESPACE option, with an
ERROR that lists out all of the non-temporary objects which are found
(and lists any other databases which have objects in those
tablespaces..). That would allow administrators who have existing
notionally temporary-only tablespaces to go clean things up to make them
actually temporary-only.That seems just about impossible from a concurrency standpoint
(ie, what if someone is creating a new table in the tablespace
concurrently with your check? Possibly in a different database?)
You are correct, I completely forgot that CREATE TABLESPACE is not
transaction safe.
I would certainly suggest that the first version of the patch not
undertake to allow this property to be ALTERed; the cost-benefit
ratio isn't there IMO.
+1
Regards,
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Show quoted text
Timbira: http://www.timbira.com.br
Blog sobre TI: http://fabriziomello.blogspot.com
Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Stephen Frost <sfrost@snowman.net> writes:
Yes. I'd definitely like to see an ALTER TABLESPACE option, with an
ERROR that lists out all of the non-temporary objects which are found
(and lists any other databases which have objects in those
tablespaces..). That would allow administrators who have existing
notionally temporary-only tablespaces to go clean things up to make them
actually temporary-only.That seems just about impossible from a concurrency standpoint
(ie, what if someone is creating a new table in the tablespace
concurrently with your check? Possibly in a different database?)
Yeah, that's definitely an annoying complexity.
I would certainly suggest that the first version of the patch not
undertake to allow this property to be ALTERed; the cost-benefit
ratio isn't there IMO.
I suppose scheduling downtime to do the check manually across all
databases, then drop and recreate the tablespace, would work. As
someone who's working with a couple of these cases, it'd be awful nice
if there was a way PG would handle it for me.
Thanks,
Stephen
Stephen Frost wrote:
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Stephen Frost <sfrost@snowman.net> writes:
Yes. I'd definitely like to see an ALTER TABLESPACE option, with an
ERROR that lists out all of the non-temporary objects which are found
(and lists any other databases which have objects in those
tablespaces..). That would allow administrators who have existing
notionally temporary-only tablespaces to go clean things up to make them
actually temporary-only.
I would certainly suggest that the first version of the patch not
undertake to allow this property to be ALTERed; the cost-benefit
ratio isn't there IMO.I suppose scheduling downtime to do the check manually across all
databases, then drop and recreate the tablespace, would work. As
someone who's working with a couple of these cases, it'd be awful nice
if there was a way PG would handle it for me.
I wonder if some form of NOT VALID marking could be useful here. Of
course, this is not a constraint. But a mechanism of a similar spirit
seems apropos. It seems reasonable to leave such a thing for future
improvement.
--
�lvaro Herrera 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
On Wed, Jun 18, 2014 at 4:53 PM, Alvaro Herrera <alvherre@2ndquadrant.com>
wrote:
Stephen Frost wrote:
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Stephen Frost <sfrost@snowman.net> writes:
Yes. I'd definitely like to see an ALTER TABLESPACE option, with an
ERROR that lists out all of the non-temporary objects which are
found
(and lists any other databases which have objects in those
tablespaces..). That would allow administrators who have existing
notionally temporary-only tablespaces to go clean things up to make
them
actually temporary-only.
I would certainly suggest that the first version of the patch not
undertake to allow this property to be ALTERed; the cost-benefit
ratio isn't there IMO.I suppose scheduling downtime to do the check manually across all
databases, then drop and recreate the tablespace, would work. As
someone who's working with a couple of these cases, it'd be awful nice
if there was a way PG would handle it for me.I wonder if some form of NOT VALID marking could be useful here. Of
course, this is not a constraint. But a mechanism of a similar spirit
seems apropos. It seems reasonable to leave such a thing for future
improvement.
+1
Then, to summarize Matheus must do:
* use an option instead of change the syntax and catalog to indicate that a
tablespace is used to store temp objects
* throw an exception if we try ALTER the option "only_temp_relations"
* add regression tests
* add documentation
And, of course, register to the next open commitfest [1]https://commitfest.postgresql.org/ to get detailed
feedback and review.
Regards,
[1]: https://commitfest.postgresql.org/
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Show quoted text
Timbira: http://www.timbira.com.br
Blog sobre TI: http://fabriziomello.blogspot.com
Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
I was going to answer each message, but Fabrízio summarized everything so
far really nicely. Thanks Fabrízio.
On Wed, Jun 18, 2014 at 5:25 PM, Fabrízio de Royes Mello <
fabriziomello@gmail.com> wrote:
Then, to summarize Matheus must do:
* use an option instead of change the syntax and catalog to indicate that
a tablespace is used to store temp objects
Yes. I myself wasn't sure TEMPORARY syntax would be good, but I would just
like to hear more about. Storing as an options seems nice to me.
* throw an exception if we try ALTER the option "only_temp_relations"
I think we should postpone the ALTER option, perhaps as another patch.
Wasn't that what was decided?
I'll investigate the options better before going this route. Let me work on
CREATE first.
* add regression tests
* add documentationAnd, of course, register to the next open commitfest [1] to get detailed
feedback and review.
Noted. It will be on the next commitfest. Just knowing some people do want
this make me more comfortable on working better.
Best regards,
--
Matheus de Oliveira
Analista de Banco de Dados
Dextra Sistemas - MPS.Br nível F!
www.dextra.com.br/postgres
On Wed, Jun 18, 2014 at 10:39 PM, Matheus de Oliveira <
matioli.matheus@gmail.com> wrote:
I was going to answer each message, but Fabrízio summarized everything so
far really nicely. Thanks Fabrízio.
You're welcome. :-)
On Wed, Jun 18, 2014 at 5:25 PM, Fabrízio de Royes Mello <
fabriziomello@gmail.com> wrote:
Then, to summarize Matheus must do:
* use an option instead of change the syntax and catalog to indicate
that a tablespace is used to store temp objects
Yes. I myself wasn't sure TEMPORARY syntax would be good, but I would
just like to hear more about. Storing as an options seems nice to me.
Are there somebody that reject this idea? Thoughts?
* throw an exception if we try ALTER the option "only_temp_relations"
I think we should postpone the ALTER option, perhaps as another patch.
Wasn't that what was decided?
Don't worry about that, we can help you. ;-)
I'll investigate the options better before going this route. Let me work
on CREATE first.
See this files:
- src/include/commands/tablespace.h (TableSpaceOpts struct at line 35)
- src/backend/access/common/reloptions.c (tablespace_reloptions at line
1333)
* add regression tests
* add documentationAnd, of course, register to the next open commitfest [1] to get detailed
feedback and review.
Noted. It will be on the next commitfest. Just knowing some people do
want this make me more comfortable on working better.
Nice... I would like to be a reviewer.
Att,
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Show quoted text
Timbira: http://www.timbira.com.br
Blog sobre TI: http://fabriziomello.blogspot.com
Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 06/18/2014 08:00 PM, Stephen Frost wrote:
PG would need to enforce that it's only used for temporary objects
as well, of course.. Or at least, that was my thinking on this.
A way to put UNLOGGED objects in such a space and have them recovered
if they vanish would also be valuable, IMO.
Not necessarily in the same patch, I'd just rather keep it in mind so
any chosen design doesn't preclude adding that later.
- --
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQEcBAEBAgAGBQJTpmsOAAoJELBXNkqjr+S2SNUH/A1zor2WUbQg3PTyD+6wyxlM
v6iMpH9zaeMWYyavIHM9N8a+h7hl9ajKDPMuK0vYLMs83W88LXI4T/t1iJlgZ3Po
WhyjjdNonChn5v1NEJncb4KgqmRQxyWY43rzgEgofSJQgjFbo8UAwoD8Xa32ghDu
4dx2ijYqYsx/hC1Gb1u/HbQ9rX+OkTA0DwljdUgPqPorW/mn5RVscz9Qo5p/W+XH
tdvpu9CKRi4AFPkshZOij+Mu3My863K1+k7sHUpRcsRGJT/AIO1V0wUQE2PuWTAV
5InU5vw/1C6CsYpH4v++XvpeTOnQ4QrMnllh99UTxV91UXN+jx/ENv1SYhBfYGE=
=JGYF
-----END PGP SIGNATURE-----
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 06/22/2014 01:35 PM, Craig Ringer wrote:
A way to put UNLOGGED objects in such a space and have them
recovered if they vanish would also be valuable, IMO.
By which I mean "re-created empty, as if after a crash".
- --
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQEcBAEBAgAGBQJTpmuwAAoJELBXNkqjr+S284oIALPSZVCkqv/A6Oriu48n9LOm
KI0cnyiv+kJ8Kor8kDIbweM6DdaUaP0msQOj3vRh8t1GGldF1hmEo83IH/t+sx4t
3B2SUPmHQX5QHa27zmzCk8qAhYKYyUy/HOI0u2DxLm5fGtxpzCjyojo4k8+YkSgM
b17eDGrAzlxXJF9BiRj4SBITOxekdgml0yF6J0UPWk98N4cTaXXsMv5r/QBlyXOm
/3bdZ0rx0extLcuH+x9iUo7aXC4qCgFJ1ZEzp3xSVGVxU7VA3vQl1O1OR7GHRKJb
E3SqS6RiHpWqT2JxD1su/a5XblpF/d5/EfXNxSY7etdNBBarHF5Q6xCNDNard8Y=
=30iz
-----END PGP SIGNATURE-----
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
* Craig Ringer (craig@2ndquadrant.com) wrote:
On 06/18/2014 08:00 PM, Stephen Frost wrote:
PG would need to enforce that it's only used for temporary objects
as well, of course.. Or at least, that was my thinking on this.A way to put UNLOGGED objects in such a space and have them recovered
if they vanish would also be valuable, IMO.
Interesting idea.
Not necessarily in the same patch, I'd just rather keep it in mind so
any chosen design doesn't preclude adding that later.
Perhaps we need a more complex definition than just "temporary
tablespace", as in, we should have a way for users to say "this
tablespace is allowed to have objects of type X, Y, Z"? I can see a
couple of ways to do that and I don't think it'd require much in the way
of changes to the current patch...
Thanks,
Stephen
On Wed, Jun 18, 2014 at 9:39 PM, Matheus de Oliveira
<matioli.matheus@gmail.com> wrote:
Then, to summarize Matheus must do:
* use an option instead of change the syntax and catalog to indicate that
a tablespace is used to store temp objectsYes. I myself wasn't sure TEMPORARY syntax would be good, but I would just
like to hear more about. Storing as an options seems nice to me.
I kind of like the TEMPORARY syntax, by analogy with what we do for
tables. On the other hand, this situation isn't quite analogous: the
tablespace itself is permanent; it's only the objects inside the
tablespace that are temporary. Hmm.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Jun 23, 2014 at 1:17 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Jun 18, 2014 at 9:39 PM, Matheus de Oliveira
<matioli.matheus@gmail.com> wrote:Then, to summarize Matheus must do:
* use an option instead of change the syntax and catalog to indicate
that
a tablespace is used to store temp objects
Yes. I myself wasn't sure TEMPORARY syntax would be good, but I would
just
like to hear more about. Storing as an options seems nice to me.
I kind of like the TEMPORARY syntax, by analogy with what we do for
tables. On the other hand, this situation isn't quite analogous: the
tablespace itself is permanent; it's only the objects inside the
tablespace that are temporary. Hmm.
The suggested syntax is about "TEMP LOCATION" and not "TEMP TABLESPACE"...
Regards,
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Show quoted text
Timbira: http://www.timbira.com.br
Blog sobre TI: http://fabriziomello.blogspot.com
Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
On Sun, Jun 22, 2014 at 2:35 AM, Craig Ringer <craig@2ndquadrant.com> wrote:
A way to put UNLOGGED objects in such a space and have them recovered
if they vanish would also be valuable, IMO.Not necessarily in the same patch, I'd just rather keep it in mind so
any chosen design doesn't preclude adding that later.
The idea is nice, but I think you should think more about it. Were would we
put the "init" files in this case? It surely can't be in the tablespace.
Best regards,
--
Matheus de Oliveira
Analista de Banco de Dados
Dextra Sistemas - MPS.Br nível F!
www.dextra.com.br/postgres
Hi Hackers,
I have worked on that patch a little more. So now I have functional patch
(although still WIP) attached. The feature works as following:
- Added a boolean parameter "only_temp_files" to pg_tablespace.spcoptions;
- This parameter can be set to true only during CREATE TABLESPACE, not on
ALTER TABLESPACE (I have thought of ways of implementing the latter, and
I'd like to discuss it more latter);
- On the creation of relations, it is checked if it is a
temporary-tablespace, and an error occurs when it is and the relation is
not temporary (temp table or index on a temp table);
- When a temporary file (either relation file or sort/agg file) is created
inside a temporary-tablespace, the entire directories structure is created
on-demand (e.g. if pg_tblspc/<oid>/<TABLESPACE_VERSION_DIRECTORY> is
missing, it is created on demand) it is done on
OpenTemporaryFileInTablespace, at fd.c (I wonder if shouldn't we do that
for any tablespace) and on TablespaceCreateDbspace, at tablespace.c.
I still haven't change documentation, as I think I need some insights about
the changes. I have some more thoughts about the syntax and I still think
that "TEMP LOCATION" syntax is better suited for this patch. First because
of the nature of the changes I made, it seems more suitable to a column on
pg_tablespace rather than an option. Second because no ALTER is available
(so far) and I think it is odd to have an option that can't be changed.
Third, I think "TEMP" keyword is more clear and users can be more used to
it.
Thoughts?
I'm going to add the CF app entry next. Could I get some review now or
after discussion about how things are going (remember I'm a newbie on this,
so I'm a little lost)?
Regards,
--
Matheus de Oliveira
Analista de Banco de Dados
Dextra Sistemas - MPS.Br nível F!
www.dextra.com.br/postgres
Attachments:
temptablespace.patchtext/x-patch; charset=US-ASCII; name=temptablespace.patchDownload
*** a/src/backend/access/common/reloptions.c
--- b/src/backend/access/common/reloptions.c
***************
*** 71,76 **** static relopt_bool boolRelOpts[] =
--- 71,84 ----
},
{
{
+ "only_temp_files",
+ "Allow only temporary files to be created on this tablespace",
+ RELOPT_KIND_TABLESPACE
+ },
+ false
+ },
+ {
+ {
"fastupdate",
"Enables \"fast update\" feature for this GIN index",
RELOPT_KIND_GIN
***************
*** 1337,1343 **** tablespace_reloptions(Datum reloptions, bool validate)
int numoptions;
static const relopt_parse_elt tab[] = {
{"random_page_cost", RELOPT_TYPE_REAL, offsetof(TableSpaceOpts, random_page_cost)},
! {"seq_page_cost", RELOPT_TYPE_REAL, offsetof(TableSpaceOpts, seq_page_cost)}
};
options = parseRelOptions(reloptions, validate, RELOPT_KIND_TABLESPACE,
--- 1345,1352 ----
int numoptions;
static const relopt_parse_elt tab[] = {
{"random_page_cost", RELOPT_TYPE_REAL, offsetof(TableSpaceOpts, random_page_cost)},
! {"seq_page_cost", RELOPT_TYPE_REAL, offsetof(TableSpaceOpts, seq_page_cost)},
! {"only_temp_files", RELOPT_TYPE_BOOL, offsetof(TableSpaceOpts, only_temp_files)}
};
options = parseRelOptions(reloptions, validate, RELOPT_KIND_TABLESPACE,
*** a/src/backend/commands/dbcommands.c
--- b/src/backend/commands/dbcommands.c
***************
*** 394,399 **** createdb(const CreatedbStmt *stmt)
--- 394,405 ----
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("pg_global cannot be used as default tablespace")));
+ /* can't create a database on temporary tablespace */
+ if (is_tablespace_temp_only(dst_deftablespace))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("this tablespace only allows temporary files")));
+
/*
* If we are trying to change the default tablespace of the template,
* we require that the template not have any files in the new default
***************
*** 1083,1088 **** movedb(const char *dbname, const char *tblspcname)
--- 1089,1100 ----
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("pg_global cannot be used as default tablespace")));
+ /* can't create a database on temporary tablespace */
+ if (is_tablespace_temp_only(dst_tblspcoid))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("this tablespace only allows temporary files")));
+
/*
* No-op if same tablespace
*/
*** a/src/backend/commands/indexcmds.c
--- b/src/backend/commands/indexcmds.c
***************
*** 432,437 **** DefineIndex(Oid relationId,
--- 432,446 ----
get_tablespace_name(tablespaceId));
}
+ /* Can't save relations on temporary tablespace */
+ if (rel->rd_rel->relpersistence != RELPERSISTENCE_TEMP &&
+ is_tablespace_temp_only(OidIsValid(tablespaceId) ? tablespaceId : MyDatabaseTableSpace))
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("this tablespace only allows temporary files")));
+ }
+
/*
* Force shared indexes into the pg_global tablespace. This is a bit of a
* hack but seems simpler than marking them in the BKI commands. On the
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
***************
*** 523,528 **** DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId)
--- 523,537 ----
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("only shared relations can be placed in pg_global tablespace")));
+ /* Can't save relations on temporary tablespace */
+ if (stmt->relation->relpersistence != RELPERSISTENCE_TEMP &&
+ is_tablespace_temp_only(OidIsValid(tablespaceId) ? tablespaceId : MyDatabaseTableSpace))
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("this tablespace only allows temporary files")));
+ }
+
/* Identify user ID that will own the table */
if (!OidIsValid(ownerId))
ownerId = GetUserId();
***************
*** 8824,8829 **** ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel, char *tablespacename, L
--- 8833,8847 ----
aclcheck_error(aclresult, ACL_KIND_TABLESPACE, tablespacename);
}
+ /* Can't save relations on temporary tablespace */
+ if (rel->rd_rel->relpersistence != RELPERSISTENCE_TEMP &&
+ is_tablespace_temp_only(OidIsValid(tablespaceId) ? tablespaceId : MyDatabaseTableSpace))
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("this tablespace only allows temporary files")));
+ }
+
/* Save info for Phase 3 to do the real work */
if (OidIsValid(tab->newTableSpace))
ereport(ERROR,
*** a/src/backend/commands/tablespace.c
--- b/src/backend/commands/tablespace.c
***************
*** 81,86 ****
--- 81,87 ----
#include "utils/memutils.h"
#include "utils/rel.h"
#include "utils/tqual.h"
+ #include "utils/spccache.h"
/* GUC variables */
***************
*** 154,170 **** TablespaceCreateDbspace(Oid spcNode, Oid dbNode, bool isRedo)
{
char *parentdir;
! /* Failure other than not exists or not in WAL replay? */
! if (errno != ENOENT || !isRedo)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not create directory \"%s\": %m",
dir)));
/*
! * Parent directories are missing during WAL replay, so
! * continue by creating simple parent directories rather
! * than a symlink.
*/
/* create two parents up if not exist */
--- 155,172 ----
{
char *parentdir;
! /* Failure other than not exists or not in WAL replay with a non-temp tablespace? */
! if (errno != ENOENT || !( isRedo || is_tablespace_temp_only(spcNode) ) )
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not create directory \"%s\": %m",
dir)));
/*
! * Parent directories are missing during WAL replay, and
! * they can be missing for temp tablespaces, so continue
! * by creating simple parent directories rather than a
! * symlink.
*/
/* create two parents up if not exist */
***************
*** 929,934 **** AlterTableSpaceOptions(AlterTableSpaceOptionsStmt *stmt)
--- 931,938 ----
Oid tablespaceoid;
Datum datum;
Datum newOptions;
+ TableSpaceOpts *tsopts;
+ TableSpaceOpts *tsoptsOld;
Datum repl_val[Natts_pg_tablespace];
bool isnull;
bool repl_null[Natts_pg_tablespace];
***************
*** 960,969 **** AlterTableSpaceOptions(AlterTableSpaceOptionsStmt *stmt)
/* Generate new proposed spcoptions (text array) */
datum = heap_getattr(tup, Anum_pg_tablespace_spcoptions,
RelationGetDescr(rel), &isnull);
newOptions = transformRelOptions(isnull ? (Datum) 0 : datum,
stmt->options, NULL, NULL, false,
stmt->isReset);
! (void) tablespace_reloptions(newOptions, true);
/* Build new tuple. */
memset(repl_null, false, sizeof(repl_null));
--- 964,983 ----
/* Generate new proposed spcoptions (text array) */
datum = heap_getattr(tup, Anum_pg_tablespace_spcoptions,
RelationGetDescr(rel), &isnull);
+ tsoptsOld = (TableSpaceOpts *) tablespace_reloptions(datum, false);
newOptions = transformRelOptions(isnull ? (Datum) 0 : datum,
stmt->options, NULL, NULL, false,
stmt->isReset);
! tsopts = (TableSpaceOpts *) tablespace_reloptions(newOptions, true);
!
! /* Can't save relations on temporary tablespace */
! if (tsopts->only_temp_files)
! {
! if (!tsoptsOld->only_temp_files)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("cannot alter a tablespace to become temporary")));
! }
/* Build new tuple. */
memset(repl_null, false, sizeof(repl_null));
*** a/src/backend/storage/file/fd.c
--- b/src/backend/storage/file/fd.c
***************
*** 76,81 ****
--- 76,82 ----
#include "storage/ipc.h"
#include "utils/guc.h"
#include "utils/resowner_private.h"
+ #include "utils/spccache.h"
/*
***************
*** 1132,1137 **** OpenTemporaryFileInTablespace(Oid tblspcOid, bool rejectError)
--- 1133,1156 ----
file = PathNameOpenFile(tempfilepath,
O_RDWR | O_CREAT | O_TRUNC | PG_BINARY,
0600);
+ /* On a temporary tablespace, we need to recreate its structure */
+ if (file <= 0 && is_tablespace_temp_only(tblspcOid))
+ {
+ /*
+ * XXX: Should we only do that for temp tablespace? Or blindly do for
+ * any tablespace?
+ */
+ char *parentdir;
+ parentdir = pstrdup(tempdirpath);
+ get_parent_directory(parentdir);
+ /* As above, don't check error for mkdir */
+ mkdir(parentdir, S_IRWXU);
+ pfree(parentdir);
+
+ file = PathNameOpenFile(tempfilepath,
+ O_RDWR | O_CREAT | O_TRUNC | PG_BINARY,
+ 0600);
+ }
if (file <= 0 && rejectError)
elog(ERROR, "could not create temporary file \"%s\": %m",
tempfilepath);
*** a/src/backend/utils/cache/spccache.c
--- b/src/backend/utils/cache/spccache.c
***************
*** 199,201 **** get_tablespace_page_costs(Oid spcid,
--- 199,230 ----
*spc_seq_page_cost = spc->opts->seq_page_cost;
}
}
+
+ /*
+ * is_tablespace_temp_only
+ * Return true if the tablespace only allows temporary files
+ */
+ bool
+ is_tablespace_temp_only(Oid spcid)
+ {
+ TableSpaceCacheEntry *spc;
+
+ /*
+ * pg_global and pg_default are never temporary, so no need to
+ * check the cache
+ */
+ if (spcid == GLOBALTABLESPACE_OID || spcid == DEFAULTTABLESPACE_OID)
+ return false;
+
+ spc = get_tablespace(spcid);
+
+ Assert(spc != NULL);
+
+ if (spc->opts == NULL)
+ {
+ /* no options, so this tablespace can't be considered temporary */
+ return false;
+ }
+
+ return spc->opts->only_temp_files;
+ }
*** a/src/include/commands/tablespace.h
--- b/src/include/commands/tablespace.h
***************
*** 37,42 **** typedef struct TableSpaceOpts
--- 37,43 ----
int32 vl_len_; /* varlena header (do not touch directly!) */
float8 random_page_cost;
float8 seq_page_cost;
+ bool only_temp_files;
} TableSpaceOpts;
extern Oid CreateTableSpace(CreateTableSpaceStmt *stmt);
*** a/src/include/utils/spccache.h
--- b/src/include/utils/spccache.h
***************
*** 15,19 ****
--- 15,20 ----
void get_tablespace_page_costs(Oid spcid, float8 *spc_random_page_cost,
float8 *spc_seq_page_cost);
+ bool is_tablespace_temp_only(Oid spcid);
#endif /* SPCCACHE_H */
*** a/src/test/regress/input/tablespace.source
--- b/src/test/regress/input/tablespace.source
***************
*** 81,89 **** ALTER TABLESPACE testspace_renamed MOVE ALL TO pg_default;
--- 81,133 ----
-- Should show notice that nothing was done
ALTER TABLESPACE testspace_renamed MOVE ALL TO pg_default;
+ -- Try changing only_temp_files
+ ALTER TABLESPACE testspace_renamed SET (only_temp_files = true); --fail
+ ALTER TABLESPACE testspace_renamed SET (only_temp_files = on); --fail
+ SELECT spcoptions FROM pg_tablespace WHERE spcname = 'testspace_renamed';
+ ALTER TABLESPACE testspace_renamed SET (only_temp_files = false); --ok, already non-temporary, just explicit set
+ SELECT spcoptions FROM pg_tablespace WHERE spcname = 'testspace_renamed';
+ ALTER TABLESPACE testspace_renamed SET (only_temp_files = off); --ok, already non-temporary, just explicit set
+ SELECT spcoptions FROM pg_tablespace WHERE spcname = 'testspace_renamed';
+ ALTER TABLESPACE testspace_renamed RESET (only_temp_files); --ok
+
-- Should succeed
DROP TABLESPACE testspace_renamed;
+ -- Now, let's create temporary tablespace
+ CREATE TABLESPACE testspace LOCATION '@testtablespace@' WITH (only_temp_files = true); -- ok
+ CREATE TABLE testschema.nontemp(a int) TABLESPACE testspace; -- fail
+ CREATE TABLE testschema.nontemp(a int); -- ok
+ ALTER TABLE testschema.nontemp SET TABLESPACE testspace; -- fail
+ CREATE INDEX nontemp_idx ON testschema.nontemp(a) TABLESPACE testspace; -- fail
+ CREATE INDEX nontemp_idx ON testschema.nontemp(a); -- ok
+ ALTER INDEX testschema.nontemp_idx SET TABLESPACE testspace; -- fail
+
+ -- Explicit created (on CREATE and ALTER)
+ CREATE TEMP TABLE temptbl(a int) TABLESPACE testspace;
+ CREATE INDEX temptbl_idx ON temptbl(a);
+ CREATE TEMP TABLE temptbl2(a int);
+ CREATE INDEX temptbl2_idx ON temptbl2(a);
+ ALTER TABLE temptbl2 SET TABLESPACE testspace;
+ ALTER INDEX temptbl2_idx SET TABLESPACE testspace;
+ CREATE INDEX temptbl_idx ON temptbl (a);
+ SELECT relname, spcname FROM pg_catalog.pg_tablespace t, pg_catalog.pg_class c
+ where c.reltablespace = t.oid AND c.relnamespace = pg_my_temp_schema() AND c.relname IN ('temptbl', 'temptbl_idx', 'temptbl2', 'temptbl2_idx');
+ DROP TABLE temptbl;
+ DROP TABLE temptbl2;
+
+ -- Use temp_tablespaces
+ SET temp_tablespaces TO 'testspace';
+ CREATE TEMP TABLE temptbl(a int);
+ CREATE INDEX temptbl_idx ON temptbl (a);
+ SELECT relname, spcname FROM pg_catalog.pg_tablespace t, pg_catalog.pg_class c
+ where c.reltablespace = t.oid AND c.relnamespace = pg_my_temp_schema() AND c.relname IN ('temptbl', 'temptbl_idx');
+
+ -- Use this tablespace in a sort operation, and check if any files on pgsql_tmp has been used
+ SET work_mem TO '1MB';
+ WITH o AS (SELECT i, md5(random()::text) FROM generate_series(1, 100000) i ORDER BY md5) SELECT count(*) > 0 FROM (SELECT pg_ls_dir('pg_tblspc/' || oid || '/' || pg_ls_dir('pg_tblspc/' || oid || '/') || '/pgsql_tmp/') FROM pg_tablespace, (SELECT count(*) FROM o) t1 WHERE spcname = 'testspace') t2;
+
+ -- Should succeed
DROP SCHEMA testschema CASCADE;
DROP ROLE tablespace_testuser1;
*** a/src/test/regress/output/tablespace.source
--- b/src/test/regress/output/tablespace.source
***************
*** 97,109 **** ALTER TABLESPACE testspace_renamed MOVE ALL TO pg_default;
-- Should show notice that nothing was done
ALTER TABLESPACE testspace_renamed MOVE ALL TO pg_default;
NOTICE: no matching relations in tablespace "testspace_renamed" found
-- Should succeed
DROP TABLESPACE testspace_renamed;
DROP SCHEMA testschema CASCADE;
! NOTICE: drop cascades to 4 other objects
DETAIL: drop cascades to table testschema.foo
drop cascades to table testschema.asselect
drop cascades to table testschema.asexecute
drop cascades to table testschema.atable
DROP ROLE tablespace_testuser1;
DROP ROLE tablespace_testuser2;
--- 97,189 ----
-- Should show notice that nothing was done
ALTER TABLESPACE testspace_renamed MOVE ALL TO pg_default;
NOTICE: no matching relations in tablespace "testspace_renamed" found
+ -- Try changing only_temp_files
+ ALTER TABLESPACE testspace_renamed SET (only_temp_files = true); --fail
+ ERROR: cannot alter a tablespace to become temporary
+ ALTER TABLESPACE testspace_renamed SET (only_temp_files = on); --fail
+ ERROR: cannot alter a tablespace to become temporary
+ SELECT spcoptions FROM pg_tablespace WHERE spcname = 'testspace_renamed';
+ spcoptions
+ ------------
+
+ (1 row)
+
+ ALTER TABLESPACE testspace_renamed SET (only_temp_files = false); --ok, already non-temporary, just explicit set
+ SELECT spcoptions FROM pg_tablespace WHERE spcname = 'testspace_renamed';
+ spcoptions
+ -------------------------
+ {only_temp_files=false}
+ (1 row)
+
+ ALTER TABLESPACE testspace_renamed SET (only_temp_files = off); --ok, already non-temporary, just explicit set
+ SELECT spcoptions FROM pg_tablespace WHERE spcname = 'testspace_renamed';
+ spcoptions
+ -----------------------
+ {only_temp_files=off}
+ (1 row)
+
+ ALTER TABLESPACE testspace_renamed RESET (only_temp_files); --ok
-- Should succeed
DROP TABLESPACE testspace_renamed;
+ -- Now, let's create temporary tablespace
+ CREATE TABLESPACE testspace LOCATION '@testtablespace@' WITH (only_temp_files = true); -- ok
+ CREATE TABLE testschema.nontemp(a int) TABLESPACE testspace; -- fail
+ ERROR: this tablespace only allows temporary files
+ CREATE TABLE testschema.nontemp(a int); -- ok
+ ALTER TABLE testschema.nontemp SET TABLESPACE testspace; -- fail
+ ERROR: this tablespace only allows temporary files
+ CREATE INDEX nontemp_idx ON testschema.nontemp(a) TABLESPACE testspace; -- fail
+ ERROR: this tablespace only allows temporary files
+ CREATE INDEX nontemp_idx ON testschema.nontemp(a); -- ok
+ ALTER INDEX testschema.nontemp_idx SET TABLESPACE testspace; -- fail
+ ERROR: this tablespace only allows temporary files
+ -- Explicit created (on CREATE and ALTER)
+ CREATE TEMP TABLE temptbl(a int) TABLESPACE testspace;
+ CREATE INDEX temptbl_idx ON temptbl(a);
+ CREATE TEMP TABLE temptbl2(a int);
+ CREATE INDEX temptbl2_idx ON temptbl2(a);
+ ALTER TABLE temptbl2 SET TABLESPACE testspace;
+ ALTER INDEX temptbl2_idx SET TABLESPACE testspace;
+ CREATE INDEX temptbl_idx ON temptbl (a);
+ ERROR: relation "temptbl_idx" already exists
+ SELECT relname, spcname FROM pg_catalog.pg_tablespace t, pg_catalog.pg_class c
+ where c.reltablespace = t.oid AND c.relnamespace = pg_my_temp_schema() AND c.relname IN ('temptbl', 'temptbl_idx', 'temptbl2', 'temptbl2_idx');
+ relname | spcname
+ --------------+-----------
+ temptbl | testspace
+ temptbl2_idx | testspace
+ temptbl2 | testspace
+ (3 rows)
+
+ DROP TABLE temptbl;
+ DROP TABLE temptbl2;
+ -- Use temp_tablespaces
+ SET temp_tablespaces TO 'testspace';
+ CREATE TEMP TABLE temptbl(a int);
+ CREATE INDEX temptbl_idx ON temptbl (a);
+ SELECT relname, spcname FROM pg_catalog.pg_tablespace t, pg_catalog.pg_class c
+ where c.reltablespace = t.oid AND c.relnamespace = pg_my_temp_schema() AND c.relname IN ('temptbl', 'temptbl_idx');
+ relname | spcname
+ -------------+-----------
+ temptbl | testspace
+ temptbl_idx | testspace
+ (2 rows)
+
+ -- Use this tablespace in a sort operation, and check if any files on pgsql_tmp has been used
+ SET work_mem TO '1MB';
+ WITH o AS (SELECT i, md5(random()::text) FROM generate_series(1, 100000) i ORDER BY md5) SELECT count(*) > 0 FROM (SELECT pg_ls_dir('pg_tblspc/' || oid || '/' || pg_ls_dir('pg_tblspc/' || oid || '/') || '/pgsql_tmp/') FROM pg_tablespace, (SELECT count(*) FROM o) t1 WHERE spcname = 'testspace') t2;
+ ?column?
+ ----------
+ t
+ (1 row)
+
+ -- Should succeed
DROP SCHEMA testschema CASCADE;
! NOTICE: drop cascades to 5 other objects
DETAIL: drop cascades to table testschema.foo
drop cascades to table testschema.asselect
drop cascades to table testschema.asexecute
drop cascades to table testschema.atable
+ drop cascades to table testschema.nontemp
DROP ROLE tablespace_testuser1;
DROP ROLE tablespace_testuser2;
On 06/29/2014 02:19 AM, Matheus de Oliveira wrote:
Hi Hackers,
I have worked on that patch a little more. So now I have functional
patch (although still WIP) attached. The feature works as following:- Added a boolean parameter "only_temp_files" to pg_tablespace.spcoptions;
- This parameter can be set to true only during CREATE TABLESPACE, not
on ALTER TABLESPACE (I have thought of ways of implementing the latter,
and I'd like to discuss it more latter);
- On the creation of relations, it is checked if it is a
temporary-tablespace, and an error occurs when it is and the relation is
not temporary (temp table or index on a temp table);
- When a temporary file (either relation file or sort/agg file) is
created inside a temporary-tablespace, the entire directories structure
is created on-demand (e.g. if
pg_tblspc/<oid>/<TABLESPACE_VERSION_DIRECTORY> is missing, it is created
on demand) it is done on OpenTemporaryFileInTablespace, at fd.c (I
wonder if shouldn't we do that for any tablespace) and on
TablespaceCreateDbspace, at tablespace.c.
Right now PostgreSQL appears to rely on the absence of the tablespace
directory as a flag to tell it "don't start up, something's badly wrong
here".
If the user creates the tablespace directory, it figures they at least
know what they're doing and carries on starting up with the empty
tablespace. It's treated as an admin statement - "I know it's gone, it's
not coming back, just carry on as best you can".
If Pg were to auto-create the directory, then if (say) a mount of a
tablespace dir failed at boot, Pg would still happily start up and might
create files in the tablespace, despite there being inaccessible
tables/indexes/etc on the not-mounted volume that's *supposed* to be the
tablespace storage. That'd create plenty of mess.
So no, I don't think Pg should auto-create the tablespace directory if
it's missing for non-temporary tablespaces. Not unless the current (less
than friendly) behaviour around lost tablespaces is replaced with
something like an `ignore_missing_tablespaces` or
`drop_missing_tablespaces` GUC - akin to our existing
`zero_missing_pages` recovery GUC.
--
Craig Ringer 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
On Mon, Jun 30, 2014 at 5:14 AM, Craig Ringer <craig@2ndquadrant.com> wrote:
Right now PostgreSQL appears to rely on the absence of the tablespace
directory as a flag to tell it "don't start up, something's badly wrong
here".
Well, in fact the behavior is just to give a LOG message:
LOG: could not open tablespace directory
"pg_tblspc/100607/PG_9.3_201306121": No such file or directory
But it starts fine. No?
I ask because I'm relying on that. My patch does not on startup, so in the
absence of the tablespace directory, the above LOG is shown even for
"temporary-tablespace", and it tries to create the directory when any
process tries to use it. As we don't have catalog access on startup, it
will require me to use some kind of _init fork for tablespace if we want it
to be re-created during startup. Should we bother about this?
If the user creates the tablespace directory, it figures they at least
know what they're doing and carries on starting up with the empty
tablespace. It's treated as an admin statement - "I know it's gone, it's
not coming back, just carry on as best you can".
Well, I think for a tablespace like that it is okay to create it on-demand
(note that I have done this only for "temp", not ordinary ones).
If Pg were to auto-create the directory, then if (say) a mount of a
tablespace dir failed at boot, Pg would still happily start up and might
create files in the tablespace, despite there being inaccessible
tables/indexes/etc on the not-mounted volume that's *supposed* to be the
tablespace storage. That'd create plenty of mess.So no, I don't think Pg should auto-create the tablespace directory if
it's missing for non-temporary tablespaces.
Ok. I agree. Let's create only for temporary ones (as done by the patch
now).
Not unless the current (less
than friendly) behaviour around lost tablespaces is replaced with
something like an `ignore_missing_tablespaces` or
`drop_missing_tablespaces` GUC - akin to our existing
`zero_missing_pages` recovery GUC.
You meant `zero_damaged_pages`, I think. I don't think that is really
necessary in this case.
Anyway, I was thinking about some way to this tablespace to also allow
creation of non-temporary indexes, and after a crash we could mark such
indexes as `NOT VALID` (perhaps `zero_damaged_pages` could be of help
here?!). That should be part of another patch thought, and would require
more thoughts. But I think that would be a great improvement for some
environments, like those on AWS with their ephemeral SSDs.
Regards,
--
Matheus de Oliveira
Analista de Banco de Dados
Dextra Sistemas - MPS.Br nível F!
www.dextra.com.br/postgres
On Sun, Jun 29, 2014 at 3:19 AM, Matheus de Oliveira
<matioli.matheus@gmail.com> wrote:
Hi Hackers,
I have worked on that patch a little more. So now I have functional patch
(although still WIP) attached. The feature works as following:- Added a boolean parameter "only_temp_files" to pg_tablespace.spcoptions;
- This parameter can be set to true only during CREATE TABLESPACE, not on
ALTER TABLESPACE (I have thought of ways of implementing the latter, and I'd
like to discuss it more latter);
- On the creation of relations, it is checked if it is a
temporary-tablespace, and an error occurs when it is and the relation is not
temporary (temp table or index on a temp table);
- When a temporary file (either relation file or sort/agg file) is created
inside a temporary-tablespace, the entire directories structure is created
on-demand (e.g. if pg_tblspc/<oid>/<TABLESPACE_VERSION_DIRECTORY> is
missing, it is created on demand) it is done on
OpenTemporaryFileInTablespace, at fd.c (I wonder if shouldn't we do that for
any tablespace) and on TablespaceCreateDbspace, at tablespace.c.I still haven't change documentation, as I think I need some insights about
the changes. I have some more thoughts about the syntax and I still think
that "TEMP LOCATION" syntax is better suited for this patch. First because
of the nature of the changes I made, it seems more suitable to a column on
pg_tablespace rather than an option. Second because no ALTER is available
(so far) and I think it is odd to have an option that can't be changed.
Third, I think "TEMP" keyword is more clear and users can be more used to
it.Thoughts?
I'm going to add the CF app entry next. Could I get some review now or after
discussion about how things are going (remember I'm a newbie on this, so I'm
a little lost)?
I got the following warning messages at the compilation.
dbcommands.c:420: warning: implicit declaration of function
'is_tablespace_temp_only'
indexcmds.c:437: warning: implicit declaration of function
'is_tablespace_temp_only'
tablecmds.c:527: warning: implicit declaration of function
'is_tablespace_temp_only'
When I created temporary tablespace and executed pg_dumpall,
it generated the SQL "ALTER TABLESPACE hoge SET (only_temp_files=on);".
This is clearly a bug because that SQL is not allowed to be executed.
ERROR: this tablespace only allows temporary files
I think that this ERROR message can be improved, for example, for the
database creation case, what about something like the following?
ERROR: database cannot be created on temporary tablespace
HINT: temporary tablespace is allowed to store only temporary files.
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi, I also tried this. This looks nice but seems a bit difficult
to find a rasonable behavior.
I have worked on that patch a little more. So now I have functional patch
(although still WIP) attached. The feature works as following:- Added a boolean parameter "only_temp_files" to pg_tablespace.spcoptions;
- This parameter can be set to true only during CREATE TABLESPACE, not on
ALTER TABLESPACE (I have thought of ways of implementing the latter, and
I'd like to discuss it more latter);
- On the creation of relations, it is checked if it is a
temporary-tablespace, and an error occurs when it is and the relation is
not temporary (temp table or index on a temp table);
- When a temporary file (either relation file or sort/agg file) is created
inside a temporary-tablespace, the entire directories structure is created
on-demand (e.g. if pg_tblspc/<oid>/<TABLESPACE_VERSION_DIRECTORY> is
missing, it is created on demand) it is done on
OpenTemporaryFileInTablespace, at fd.c (I wonder if shouldn't we do that
for any tablespace) and on TablespaceCreateDbspace, at tablespace.c.I still haven't change documentation, as I think I need some insights about
the changes. I have some more thoughts about the syntax and I still think
that "TEMP LOCATION" syntax is better suited for this patch. First because
of the nature of the changes I made, it seems more suitable to a column on
pg_tablespace rather than an option. Second because no ALTER is available
(so far) and I think it is odd to have an option that can't be changed.
Third, I think "TEMP" keyword is more clear and users can be more used to
it.Thoughts?
I'm going to add the CF app entry next. Could I get some review now or
after discussion about how things are going (remember I'm a newbie on this,
so I'm a little lost)?
Here is some random comments.
1. I think some users may want to store the temp tablespace in
specially created subdirectory, like this.
| =# CREATE TABLESPACE hoge LOCATION '/mount_point_of_nonpersist_device/temptblspc1'
| WITH (only_temp_files = true);
I saw the following message for create table after restarting
after "rm -r /mount...ice/*".
| =# create temp table thoge (a int) tablespace hoge;
| ERROR: could not create directory "pg_tblspc/16435/PG_9.5_201408162": No such file or directory
Multiple-depth mkdir would be needed.
2. Creating a temporary table in a tablespace with
(only_temp_files = false) after erasing the directory then
restarting the server failed showing me the following message
only for the first time,
| =# create temp table thoge (a int) tablespace hoge;
| ERROR: could not create directory "pg_tblspc/16435/PG_9.5_201408162/13004": Success
Unpatched head seems always showing 'No such file or directory'
from the first time for the case.
3. I saw the following error message during startup after
deleting the tablespace directory for the only-temp tablespace.
| $ postgres
| LOG: database system was shut down at 2014-09-02 16:54:39 JST
*| LOG: could not open tablespace directory "pg_tblspc/16435/PG_9.5_201408162": No such file or directory
| LOG: autovacuum launcher started
| LOG: database system is ready to accept connections
I think the server should refrain from showing this message for
laking of the directories for only-temp teblespaces.
4. You inhibited the option only_temp_files from ALTER'ing from
false to true but pg_tablesspace.spcoptions unfortunately can be
changed directly. Other reloptions for all objects seems not so
harmful.
| =# update pg_tablespace set spcoptions = '{only_temp_files=true}' where spcname = 'hoge';
Are we allowed to store such a kind of option as reoptions? Or a
result from such a bogus operation should be ignored? Or do we
ought to protect spcoptions from direct modification? Or ...
Any Thoughts?
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers