Moving tablespaces
Do we have any documentation about how to move a tablespace to a new
directory? If not, I think we should write some.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
On Sun, Dec 4, 2011 at 00:43, Bruce Momjian <bruce@momjian.us> wrote:
Do we have any documentation about how to move a tablespace to a new
directory? If not, I think we should write some.
Do we have any support for doing it? (Yes, it works, but anything that
requires manual hacking of system catalogs really can't be considered
supported, can it?)
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
Magnus Hagander wrote:
On Sun, Dec 4, 2011 at 00:43, Bruce Momjian <bruce@momjian.us> wrote:
Do we have any documentation about how to move a tablespace to a new
directory? ?If not, I think we should write some.Do we have any support for doing it? (Yes, it works, but anything that
requires manual hacking of system catalogs really can't be considered
supported, can it?)
True. It is something we just don't support? They have to dump, edit
the dump, and reload, to change a tablespace directory? Yikes. Is that
the state we are in? Has no one complained about this? They just use
symlinks?
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
On Sun, Dec 4, 2011 at 17:12, Bruce Momjian <bruce@momjian.us> wrote:
Magnus Hagander wrote:
On Sun, Dec 4, 2011 at 00:43, Bruce Momjian <bruce@momjian.us> wrote:
Do we have any documentation about how to move a tablespace to a new
directory? ?If not, I think we should write some.Do we have any support for doing it? (Yes, it works, but anything that
requires manual hacking of system catalogs really can't be considered
supported, can it?)True. It is something we just don't support? They have to dump, edit
the dump, and reload, to change a tablespace directory? Yikes. Is that
the state we are in? Has no one complained about this? They just use
symlinks?
AFAIK, we don't. What you can do is take the db offline, change the
symlink, start it up agin and manually change
pg_tablespace.spclocation. That seems quite ugly though. And if you
forget one step, everything seems to work, but you have two
inconsistent definitions of the tablespace.
And IIRC, we don't actually *use* spclocation anywhere. How about we
just get rid of them as independents? We could either:
1) Remove the column. Rely on the symlink. Create a
pg_get_tablespace_location(oid) function, that could be used by
pg_dumpall and friends, that just reads the symlink.
2) Forcibly update the spclocation column when we start the server to
be whatever the symlink points to. That will at least automatically
restore the system to being consistent.
Option 1 would also make it a lot easier to in a supported way allow
tablespaces to have different locations on replica masters and slaves.
A tool like pg_basebackup could easily provide for something like
--relocate-tablespace mytblspc=/new/path and just rewrite the symlink
on the fly. But we cannot modify the pg_tablespace system catalog to
be different on the slave and the master...
It does seem rather obvious to me that this would be a win, so I'm
most likely missing something here. So please shoot a hole in the
theory for me ;)
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
Magnus Hagander <magnus@hagander.net> writes:
And IIRC, we don't actually *use* spclocation anywhere.
Just for pg_dump, I think.
How about we
just get rid of them as independents? We could either:
1) Remove the column. Rely on the symlink. Create a
pg_get_tablespace_location(oid) function, that could be used by
pg_dumpall and friends, that just reads the symlink.
Hm, how portable is symlink-reading? If we can actually do that
without big headaches, then +1.
2) Forcibly update the spclocation column when we start the server to
be whatever the symlink points to. That will at least automatically
restore the system to being consistent.
-1, running a transaction at that level will be a giant pita.
And how would you do it at all on a hot standby slave?
regards, tom lane
On Sun, Dec 4, 2011 at 17:41, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Magnus Hagander <magnus@hagander.net> writes:
And IIRC, we don't actually *use* spclocation anywhere.
Just for pg_dump, I think.
pg_dumpall :-)
It's also used in pg_upgrade and pg_basebackup, but those are easily
dealt with if we define a function for it.
How about we
just get rid of them as independents? We could either:1) Remove the column. Rely on the symlink. Create a
pg_get_tablespace_location(oid) function, that could be used by
pg_dumpall and friends, that just reads the symlink.Hm, how portable is symlink-reading? If we can actually do that
without big headaches, then +1.
We already use readlink in a couple of places - including getting
called from find_my_exec. It's also used in pg_basebackup. The use in
find_my_exec is conditional on HAVE_READLINK, but not the one in
backend/replication/basebackup.c. An oversight from my side when
putting that in, but it shows that every single bf member we have now
has readlink - else the whole compilation would fail, AFAICT.
It's not directly portable to win32, but we have a wrapper function
for it in port/ already.
So I think we're fairly committed to having readlink already.
2) Forcibly update the spclocation column when we start the server to
be whatever the symlink points to. That will at least automatically
restore the system to being consistent.-1, running a transaction at that level will be a giant pita.
And how would you do it at all on a hot standby slave?
yeah, it would break that usage scenario, so I'm -1 for that one as well.
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
On 12/04/2011 11:41 AM, Tom Lane wrote:
1) Remove the column. Rely on the symlink. Create a
pg_get_tablespace_location(oid) function, that could be used by
pg_dumpall and friends, that just reads the symlink.Hm, how portable is symlink-reading? If we can actually do that
without big headaches, then +1.
I wondered that, specifically about Windows junction points, but we seem
to have support for it already in dirmod.c::pgreadlink(). Surely there's
no other currently supported platform where it would even be a question?
cheers
andrew
Andrew Dunstan <andrew@dunslane.net> writes:
On 12/04/2011 11:41 AM, Tom Lane wrote:
Hm, how portable is symlink-reading? If we can actually do that
without big headaches, then +1.
I wondered that, specifically about Windows junction points, but we seem
to have support for it already in dirmod.c::pgreadlink(). Surely there's
no other currently supported platform where it would even be a question?
readlink is required by Single Unix Spec v2 (1997), which is what we've
been treating as our baseline expectation for Unix-oid platforms for
awhile now. Given that we dealt with the Windows side already, I don't
see a problem with making this assumption. At worst we'd end up needing
a couple more emulations in src/port, since surely there's *some* way to
do it on any platform with symlinks.
regards, tom lane
On Sun, Dec 4, 2011 at 18:07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Andrew Dunstan <andrew@dunslane.net> writes:
On 12/04/2011 11:41 AM, Tom Lane wrote:
Hm, how portable is symlink-reading? If we can actually do that
without big headaches, then +1.I wondered that, specifically about Windows junction points, but we seem
to have support for it already in dirmod.c::pgreadlink(). Surely there's
no other currently supported platform where it would even be a question?readlink is required by Single Unix Spec v2 (1997), which is what we've
been treating as our baseline expectation for Unix-oid platforms for
awhile now. Given that we dealt with the Windows side already, I don't
see a problem with making this assumption. At worst we'd end up needing
a couple more emulations in src/port, since surely there's *some* way to
do it on any platform with symlinks.
AFAICT, it should be as simple as the attached.
Doesn't include the required fixes for pg_upgrade, I'll get on those next.
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
Attachments:
spclocation.difftext/x-patch; charset=US-ASCII; name=spclocation.diffDownload
*** a/doc/src/sgml/catalogs.sgml
--- b/doc/src/sgml/catalogs.sgml
***************
*** 5392,5404 ****
</row>
<row>
- <entry><structfield>spclocation</structfield></entry>
- <entry><type>text</type></entry>
- <entry></entry>
- <entry>Location (directory path) of the tablespace</entry>
- </row>
-
- <row>
<entry><structfield>spcacl</structfield></entry>
<entry><type>aclitem[]</type></entry>
<entry></entry>
--- 5392,5397 ----
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***************
*** 13612,13617 **** SELECT pg_type_is_visible('myschema.widget'::regtype);
--- 13612,13621 ----
</indexterm>
<indexterm>
+ <primary>pg_tablespace_location</primary>
+ </indexterm>
+
+ <indexterm>
<primary>pg_typeof</primary>
</indexterm>
***************
*** 13759,13764 **** SELECT pg_type_is_visible('myschema.widget'::regtype);
--- 13763,13773 ----
<entry>get the set of database OIDs that have objects in the tablespace</entry>
</row>
<row>
+ <entry><literal><function>pg_tablespace_location(<parameter>tablespace_oid</parameter>)</function></literal></entry>
+ <entry><type>text</type></entry>
+ <entry>get the path in the filesystem that this tablespace is located in</entry>
+ </row>
+ <row>
<entry><literal><function>pg_typeof(<parameter>any</parameter>)</function></literal></entry>
<entry><type>regtype</type></entry>
<entry>get the data type of any value</entry>
*** a/doc/src/sgml/xaggr.sgml
--- b/doc/src/sgml/xaggr.sgml
***************
*** 154,160 **** SELECT attrelid::regclass, array_accum(attname)
attrelid | array_accum
---------------+---------------------------------------
! pg_tablespace | {spcname,spcowner,spclocation,spcacl}
(1 row)
SELECT attrelid::regclass, array_accum(atttypid::regtype)
--- 154,160 ----
attrelid | array_accum
---------------+---------------------------------------
! pg_tablespace | {spcname,spcowner,spcacl,spcoptions}
(1 row)
SELECT attrelid::regclass, array_accum(atttypid::regtype)
***************
*** 164,170 **** SELECT attrelid::regclass, array_accum(atttypid::regtype)
attrelid | array_accum
---------------+---------------------------
! pg_tablespace | {name,oid,text,aclitem[]}
(1 row)
</programlisting>
</para>
--- 164,170 ----
attrelid | array_accum
---------------+---------------------------
! pg_tablespace | {name,oid,aclitem[],text[]}
(1 row)
</programlisting>
</para>
*** a/src/backend/commands/tablespace.c
--- b/src/backend/commands/tablespace.c
***************
*** 314,321 **** CreateTableSpace(CreateTableSpaceStmt *stmt)
DirectFunctionCall1(namein, CStringGetDatum(stmt->tablespacename));
values[Anum_pg_tablespace_spcowner - 1] =
ObjectIdGetDatum(ownerId);
- values[Anum_pg_tablespace_spclocation - 1] =
- CStringGetTextDatum(location);
nulls[Anum_pg_tablespace_spcacl - 1] = true;
nulls[Anum_pg_tablespace_spcoptions - 1] = true;
--- 314,319 ----
*** a/src/backend/utils/adt/misc.c
--- b/src/backend/utils/adt/misc.c
***************
*** 18,23 ****
--- 18,24 ----
#include <signal.h>
#include <dirent.h>
#include <math.h>
+ #include <unistd.h>
#include "catalog/catalog.h"
#include "catalog/pg_tablespace.h"
***************
*** 262,267 **** pg_tablespace_databases(PG_FUNCTION_ARGS)
--- 263,298 ----
/*
+ * pg_tablespace_location - get location for a tablespace
+ */
+ Datum
+ pg_tablespace_location(PG_FUNCTION_ARGS)
+ {
+ Oid tablespaceOid = PG_GETARG_OID(0);
+ char sourcepath[MAXPGPATH];
+ char targetpath[MAXPGPATH];
+
+ /*
+ * Return empty string for our two default tablespace
+ */
+ if (tablespaceOid == DEFAULTTABLESPACE_OID ||
+ tablespaceOid == GLOBALTABLESPACE_OID)
+ PG_RETURN_TEXT_P(cstring_to_text(""));
+
+ /*
+ * Find the location of the tablespace by reading the symbolic link that is
+ * in pg_tblspc/<oid>.
+ */
+ snprintf(sourcepath, sizeof(sourcepath), "pg_tblspc/%d", tablespaceOid);
+ MemSet(targetpath, 0, sizeof(targetpath));
+ if (readlink(sourcepath, targetpath, sizeof(targetpath)) == -1)
+ ereport(ERROR,
+ (errmsg("could not read symbolic link \"%s\": %m", sourcepath)));
+
+ PG_RETURN_TEXT_P(cstring_to_text(targetpath));
+ }
+
+ /*
* pg_sleep - delay for N seconds
*/
Datum
*** a/src/bin/pg_dump/pg_dumpall.c
--- b/src/bin/pg_dump/pg_dumpall.c
***************
*** 997,1003 **** dumpTablespaces(PGconn *conn)
* Get all tablespaces except built-in ones (which we assume are named
* pg_xxx)
*/
! if (server_version >= 90000)
res = executeQuery(conn, "SELECT oid, spcname, "
"pg_catalog.pg_get_userbyid(spcowner) AS spcowner, "
"spclocation, spcacl, "
--- 997,1012 ----
* Get all tablespaces except built-in ones (which we assume are named
* pg_xxx)
*/
! if (server_version >= 90200)
! res = executeQuery(conn, "SELECT oid, spcname, "
! "pg_catalog.pg_get_userbyid(spcowner) AS spcowner, "
! "pg_catalog.pg_tablespace_location(oid), spcacl, "
! "array_to_string(spcoptions, ', '),"
! "pg_catalog.shobj_description(oid, 'pg_tablespace') "
! "FROM pg_catalog.pg_tablespace "
! "WHERE spcname !~ '^pg_' "
! "ORDER BY 1");
! else if (server_version >= 90000)
res = executeQuery(conn, "SELECT oid, spcname, "
"pg_catalog.pg_get_userbyid(spcowner) AS spcowner, "
"spclocation, spcacl, "
*** a/src/bin/psql/describe.c
--- b/src/bin/psql/describe.c
***************
*** 139,151 **** describeTablespaces(const char *pattern, bool verbose)
initPQExpBuffer(&buf);
! printfPQExpBuffer(&buf,
! "SELECT spcname AS \"%s\",\n"
! " pg_catalog.pg_get_userbyid(spcowner) AS \"%s\",\n"
! " spclocation AS \"%s\"",
! gettext_noop("Name"),
! gettext_noop("Owner"),
! gettext_noop("Location"));
if (verbose)
{
--- 139,160 ----
initPQExpBuffer(&buf);
! if (pset.sversion >= 90200)
! printfPQExpBuffer(&buf,
! "SELECT spcname AS \"%s\",\n"
! " pg_catalog.pg_get_userbyid(spcowner) AS \"%s\",\n"
! " pg_catalog.pg_tablespace_location(oid) AS \"%s\"",
! gettext_noop("Name"),
! gettext_noop("Owner"),
! gettext_noop("Location"));
! else
! printfPQExpBuffer(&buf,
! "SELECT spcname AS \"%s\",\n"
! " pg_catalog.pg_get_userbyid(spcowner) AS \"%s\",\n"
! " spclocation AS \"%s\"",
! gettext_noop("Name"),
! gettext_noop("Owner"),
! gettext_noop("Location"));
if (verbose)
{
*** a/src/include/catalog/pg_proc.h
--- b/src/include/catalog/pg_proc.h
***************
*** 2670,2675 **** DESCR("statistics: reset collected statistics for a single table or index in the
--- 2670,2678 ----
DATA(insert OID = 3777 ( pg_stat_reset_single_function_counters PGNSP PGUID 12 1 0 0 0 f f f f f v 1 0 2278 "26" _null_ _null_ _null_ _null_ pg_stat_reset_single_function_counters _null_ _null_ _null_ ));
DESCR("statistics: reset collected statistics for a single function in the current database");
+ DATA(insert OID = 3778 ( pg_tablespace_location PGNSP PGUID 12 1 0 0 0 f f f t f s 1 0 25 "26" _null_ _null_ _null_ _null_ pg_tablespace_location _null_ _null_ _null_ ));
+ DESCR("tablespace location");
+
DATA(insert OID = 1946 ( encode PGNSP PGUID 12 1 0 0 0 f f f t f i 2 0 25 "17 25" _null_ _null_ _null_ _null_ binary_encode _null_ _null_ _null_ ));
DESCR("convert bytea value into some ascii-only text string");
DATA(insert OID = 1947 ( decode PGNSP PGUID 12 1 0 0 0 f f f t f i 2 0 17 "25 25" _null_ _null_ _null_ _null_ binary_decode _null_ _null_ _null_ ));
*** a/src/include/catalog/pg_tablespace.h
--- b/src/include/catalog/pg_tablespace.h
***************
*** 32,38 **** CATALOG(pg_tablespace,1213) BKI_SHARED_RELATION
{
NameData spcname; /* tablespace name */
Oid spcowner; /* owner of tablespace */
- text spclocation; /* physical location (VAR LENGTH) */
aclitem spcacl[1]; /* access permissions (VAR LENGTH) */
text spcoptions[1]; /* per-tablespace options */
} FormData_pg_tablespace;
--- 32,37 ----
***************
*** 49,63 **** typedef FormData_pg_tablespace *Form_pg_tablespace;
* ----------------
*/
! #define Natts_pg_tablespace 5
#define Anum_pg_tablespace_spcname 1
#define Anum_pg_tablespace_spcowner 2
! #define Anum_pg_tablespace_spclocation 3
! #define Anum_pg_tablespace_spcacl 4
! #define Anum_pg_tablespace_spcoptions 5
! 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
--- 48,61 ----
* ----------------
*/
! #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
*** a/src/include/utils/builtins.h
--- b/src/include/utils/builtins.h
***************
*** 456,461 **** extern Datum pg_cancel_backend(PG_FUNCTION_ARGS);
--- 456,462 ----
extern Datum pg_terminate_backend(PG_FUNCTION_ARGS);
extern Datum pg_reload_conf(PG_FUNCTION_ARGS);
extern Datum pg_tablespace_databases(PG_FUNCTION_ARGS);
+ extern Datum pg_tablespace_location(PG_FUNCTION_ARGS);
extern Datum pg_rotate_logfile(PG_FUNCTION_ARGS);
extern Datum pg_sleep(PG_FUNCTION_ARGS);
extern Datum pg_get_keywords(PG_FUNCTION_ARGS);
Magnus Hagander <magnus@hagander.net> writes:
+ snprintf(sourcepath, sizeof(sourcepath), "pg_tblspc/%d", tablespaceOid);
%u for an OID, please. Otherwise seems reasonably sane on first glance.
regards, tom lane
Magnus Hagander <magnus@hagander.net> writes:
AFAICT, it should be as simple as the attached.
Oh, one other thought is that the function body has to be
conditionalized on HAVE_READLINK (the fact that you forgot that
somewhere else isn't an excuse for not doing it here). IIRC,
only the two built-in tablespaces can exist when not HAVE_READLINK,
so it seems sufficient to handle those cases and then PG_RETURN_NULL
(or maybe throw error) when not HAVE_READLINK.
regards, tom lane
On Tue, Dec 6, 2011 at 16:12, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Magnus Hagander <magnus@hagander.net> writes:
AFAICT, it should be as simple as the attached.
Oh, one other thought is that the function body has to be
conditionalized on HAVE_READLINK (the fact that you forgot that
somewhere else isn't an excuse for not doing it here). IIRC,
only the two built-in tablespaces can exist when not HAVE_READLINK,
so it seems sufficient to handle those cases and then PG_RETURN_NULL
(or maybe throw error) when not HAVE_READLINK.
Hmm. good point.
Throwing an error seems a lot more safe in this case than just
returning NULL. Since it's a situtation that really shouldn't happen.
Maybe an assert, but I think a regular ereport(ERROR) would be the
best.
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
Magnus Hagander <magnus@hagander.net> writes:
Throwing an error seems a lot more safe in this case than just
returning NULL. Since it's a situtation that really shouldn't happen.
Maybe an assert, but I think a regular ereport(ERROR) would be the
best.
Not an assert, since it's trivially user-triggerable.
regards, tom lane
On Tue, Dec 6, 2011 at 16:17, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Magnus Hagander <magnus@hagander.net> writes:
Throwing an error seems a lot more safe in this case than just
returning NULL. Since it's a situtation that really shouldn't happen.
Maybe an assert, but I think a regular ereport(ERROR) would be the
best.Not an assert, since it's trivially user-triggerable.
Right.
There is some nice precedent in the CREATE TABLESPACE command (though
dependent on HAVE_SYMLINK and not HAVE_READLINK), so I'm just going to
copy the error message from there.
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
Magnus Hagander <magnus@hagander.net> writes:
There is some nice precedent in the CREATE TABLESPACE command (though
dependent on HAVE_SYMLINK and not HAVE_READLINK), so I'm just going to
copy the error message from there.
Fair enough.
Looking at the existing readlink use in port/exec.c, it strikes me that
another thing you'd better do is include a check for buffer overrun,
ie the test needs to be more like
rllen = readlink(fname, link_buf, sizeof(link_buf));
if (rllen < 0 || rllen >= sizeof(link_buf))
... fail ...
Also, you're assuming that the result is already null-terminated,
which is incorrect.
regards, tom lane
On Tue, Dec 6, 2011 at 17:07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Magnus Hagander <magnus@hagander.net> writes:
There is some nice precedent in the CREATE TABLESPACE command (though
dependent on HAVE_SYMLINK and not HAVE_READLINK), so I'm just going to
copy the error message from there.Fair enough.
Looking at the existing readlink use in port/exec.c, it strikes me that
another thing you'd better do is include a check for buffer overrun,
ie the test needs to be more likerllen = readlink(fname, link_buf, sizeof(link_buf));
if (rllen < 0 || rllen >= sizeof(link_buf))
... fail ...
Seems reasonable, yeah. I'll go put a similar check in the
basebackup.c file as well when I'm done here.
Also, you're assuming that the result is already null-terminated,
which is incorrect.
No, I'm not - I'm MemSet()ing the whole buffer to 0 before I start.
But I'll change that to work the same way as the on in port/exec.c,
for consistency.
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
On Wed, Dec 7, 2011 at 10:05, Magnus Hagander <magnus@hagander.net> wrote:
On Tue, Dec 6, 2011 at 17:07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Magnus Hagander <magnus@hagander.net> writes:
There is some nice precedent in the CREATE TABLESPACE command (though
dependent on HAVE_SYMLINK and not HAVE_READLINK), so I'm just going to
copy the error message from there.Fair enough.
Looking at the existing readlink use in port/exec.c, it strikes me that
another thing you'd better do is include a check for buffer overrun,
ie the test needs to be more likerllen = readlink(fname, link_buf, sizeof(link_buf));
if (rllen < 0 || rllen >= sizeof(link_buf))
... fail ...Seems reasonable, yeah. I'll go put a similar check in the
basebackup.c file as well when I'm done here.
To close this thread (hopefully): Fixed and applied.
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/