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]), 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
*** 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;
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to