Re: [HACKERS] Duplicate usage of tablespace location?
Hi, At Mon, 15 May 2017 14:35:20 +0900, Michael Paquier wrote in > On Thu, May 11, 2017 at 1:09 PM, Kyotaro HORIGUCHI > wrote: > > If we can accept multiple server versions share a tablespace > > directory, pg_basebackup also can allow that situation. The > > attached patch does that. Similary to the server code, it > > correctly fails if the same version subdirectory exists. > > +#define verify_dir_is_empty_or_create(dirname, created, found) \ > +verify_and_create_dir(dirname, created, found, false) > This solution looks like a quick-and-dirty fix. I tend to prefer a You're right, it just intends to reduce the amount of modification to clarify what the patch does is. It is to be replced with the bare functions. > solution close to whet Pierre is proposing on the other thread by > localizing things in ReceiveAndUnpackTarFile(). This makes the check > more localized, and there is no need to complicate low-level APIs of > pg_basebackup.c. > > By the way, it may be better to keep discussions on the first thread created: > https://www.postgresql.org/message-id/05c62730-8670-4da6-b783-52e66fb42...@pinaraf.info > A patch has been submitted to the next CF there as well. I noticed it after the mail upthread. I have sent a comment on that and moved to the thread. Thanks for noticing. 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
Re: [HACKERS] Duplicate usage of tablespace location?
On Thu, May 11, 2017 at 1:09 PM, Kyotaro HORIGUCHI wrote: > If we can accept multiple server versions share a tablespace > directory, pg_basebackup also can allow that situation. The > attached patch does that. Similary to the server code, it > correctly fails if the same version subdirectory exists. +#define verify_dir_is_empty_or_create(dirname, created, found) \ +verify_and_create_dir(dirname, created, found, false) This solution looks like a quick-and-dirty fix. I tend to prefer a solution close to whet Pierre is proposing on the other thread by localizing things in ReceiveAndUnpackTarFile(). This makes the check more localized, and there is no need to complicate low-level APIs of pg_basebackup.c. By the way, it may be better to keep discussions on the first thread created: https://www.postgresql.org/message-id/05c62730-8670-4da6-b783-52e66fb42...@pinaraf.info A patch has been submitted to the next CF there as well. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Duplicate usage of tablespace location?
Hello, At Fri, 5 May 2017 21:42:47 +1000, Neha Khatri wrote in > As Kyotaro san pointed out, the commit 22817041 started allowing creation > of multiple "tablespace version directories" in same location. However the > original purpose of that commit was to allow that just for the upgrade > purpose. So couple of points: > - The commit violated the requirement of emptiness of the tablespace > location directory. > (Though it is still prevented to create multiple tablespaces belonging > to one server, in same location.) > - The comment did not document this change in specification. > > Probably it was not anticipated at that time that a user could create the > tablespaces for different server version at the same location. > > Now that this behaviour is present in field for a while, there is > likelihood of having systems with tablespaces for two different versions, > in same location. To avoid the problem reported in [1] for such systems, > here are couple of alternative approaches: > > 1. Allow creation of multiple tablespaces in single location for different > server versions, but not the same version(exception). > a) Also allow this capability in utilities like pg_basebackup( and others > that update tablespaces) . > b) Update the documentation about this specification change. > > I don't see this breaking any backwards compatibility. Yeah, it is just clarification of the behavior in the documentation. The current behavior is somewhat inconsistent but practical. > 2. Retain the current base rule of creating Tablespaces i.e. "The location > must be an existing, empty directory". This means: > a) For the future release, have a strict directory emptiness check while > creating new tablespace. > b) Only during upgrade, allow creation of multiple tablepaces in same > location . > c) Document the fact that only during upgrade the system would create > multiple tablespaces in same location. Honestly saying, I think it adds nothing good other than seeming consistency. (Even though I sent such a patch:p) > d) Provide a flexibility to change the location of an existing tablespace, > something like: > ALTER TABLESPACE tblspcname SET LOCATION '/path/to/newlocation' > [where newlocation is an existing empty direcotry] > > With the altered location of a tablespace it should be possible to perform > the pg_basebackup successfully. If we can accept multiple server versions share a tablespace directory, pg_basebackup also can allow that situation. The attached patch does that. Similary to the server code, it correctly fails if the same version subdirectory exists. $ pg_basebackup -D $PGDATA -h /tmp -p 5432 -X stream -T /home/horiguti/data/tsp1=/home/horiguti/data/tsp2 pg_basebackup: could not create directory "/home/horiguti/data/tsp2/PG_10_201705091": File exists pg_basebackup: removing contents of data directory "/home/horiguti/data/data_work_s" > I noticed some solutions for moving PostgreSQL tablesspaces, on internet. > But some are slow, others cause incompatibility for tools like pgAdmin. I > am not able to find any discussion about moving tablespace location in > mailing lists too. So I am not sure if there is already any conclusion > about supporting or not supporting ALTER TABLESPACE LOCATION. > To me, the first approach above looks like providing more independence to > the user about choice of tablespace location. Also, it is not clear that > why the directory emptiness rule was introduced in first place. Any insight > on that will be useful. Originally (before 9.0) files in a tablespace is directly placed in the "location" and it is reasonable at that time. > Regards, > Neha > > [1]https://www.postgresql.org/message-id/2008148.rxBNyNRHPZ@peanuts2 > > Cheers, > Neha > > On Fri, Apr 7, 2017 at 11:02 AM, Kyotaro HORIGUCHI < > horiguchi.kyot...@lab.ntt.co.jp> wrote: > > > I don't mean that this is the only or best way to go. > > > > I apologize for the possible lack of explanation. > > > > At Thu, 06 Apr 2017 12:03:51 -0400, Tom Lane wrote in > > <21084.1491494...@sss.pgh.pa.us> > > > Kyotaro HORIGUCHI writes: > > > > I noticed by the following report, PostgreSQL can share the same > > > > directory as tablespaces of two servers with different > > > > pg-versions. > > > > > > > https://www.postgresql.org/message-id/2008148.rxBNyNRHPZ@peanuts2 > > > > > > > 8.4 checked that the tablespace location is empty, but from 9.0, > > > > the check is replaced with creating a PG_PGVER_CATVER > > > > subdirectory. This works for multiple servers with the same > > > > version, but don't for servers with different versions. > > > > > > Please explain why you think it doesn't work. This patch seems to > > > be reverting an intentional behavioral change, and you haven't > > > > I understand that the change is for in-place upgrade, not for > > sharing a tablespace diretory between two version of PostgreSQL > > servers. It actually rejects the second server with the same > > version to come. If this is
Re: [HACKERS] Duplicate usage of tablespace location?
As Kyotaro san pointed out, the commit 22817041 started allowing creation of multiple "tablespace version directories" in same location. However the original purpose of that commit was to allow that just for the upgrade purpose. So couple of points: - The commit violated the requirement of emptiness of the tablespace location directory. (Though it is still prevented to create multiple tablespaces belonging to one server, in same location.) - The comment did not document this change in specification. Probably it was not anticipated at that time that a user could create the tablespaces for different server version at the same location. Now that this behaviour is present in field for a while, there is likelihood of having systems with tablespaces for two different versions, in same location. To avoid the problem reported in [1] for such systems, here are couple of alternative approaches: 1. Allow creation of multiple tablespaces in single location for different server versions, but not the same version(exception). a) Also allow this capability in utilities like pg_basebackup( and others that update tablespaces) . b) Update the documentation about this specification change. I don't see this breaking any backwards compatibility. 2. Retain the current base rule of creating Tablespaces i.e. "The location must be an existing, empty directory". This means: a) For the future release, have a strict directory emptiness check while creating new tablespace. b) Only during upgrade, allow creation of multiple tablepaces in same location . c) Document the fact that only during upgrade the system would create multiple tablespaces in same location. d) Provide a flexibility to change the location of an existing tablespace, something like: ALTER TABLESPACE tblspcname SET LOCATION '/path/to/newlocation' [where newlocation is an existing empty direcotry] With the altered location of a tablespace it should be possible to perform the pg_basebackup successfully. I noticed some solutions for moving PostgreSQL tablesspaces, on internet. But some are slow, others cause incompatibility for tools like pgAdmin. I am not able to find any discussion about moving tablespace location in mailing lists too. So I am not sure if there is already any conclusion about supporting or not supporting ALTER TABLESPACE LOCATION. To me, the first approach above looks like providing more independence to the user about choice of tablespace location. Also, it is not clear that why the directory emptiness rule was introduced in first place. Any insight on that will be useful. Regards, Neha [1]https://www.postgresql.org/message-id/2008148.rxBNyNRHPZ@peanuts2 Cheers, Neha On Fri, Apr 7, 2017 at 11:02 AM, Kyotaro HORIGUCHI < horiguchi.kyot...@lab.ntt.co.jp> wrote: > I don't mean that this is the only or best way to go. > > I apologize for the possible lack of explanation. > > At Thu, 06 Apr 2017 12:03:51 -0400, Tom Lane wrote in > <21084.1491494...@sss.pgh.pa.us> > > Kyotaro HORIGUCHI writes: > > > I noticed by the following report, PostgreSQL can share the same > > > directory as tablespaces of two servers with different > > > pg-versions. > > > > > https://www.postgresql.org/message-id/2008148.rxBNyNRHPZ@peanuts2 > > > > > 8.4 checked that the tablespace location is empty, but from 9.0, > > > the check is replaced with creating a PG_PGVER_CATVER > > > subdirectory. This works for multiple servers with the same > > > version, but don't for servers with different versions. > > > > Please explain why you think it doesn't work. This patch seems to > > be reverting an intentional behavioral change, and you haven't > > I understand that the change is for in-place upgrade, not for > sharing a tablespace diretory between two version of PostgreSQL > servers. It actually rejects the second server with the same > version to come. If this is correct, it doesn't seem right to > accept the second server of the different version. > > If we allow sharing of the directory, theoretically we can allow > the same between the same version of servers by adding system > identifier in the subdirectory name. > > > > really explained why we'd want to. It certainly doesn't look like > > it addresses the referenced complaint about pg_basebackup behavior. > > My point is that "the direcotry for newly created tablespace is > really reuiqred to be literary empty or not?" > > Practically it doesn't need to be empty and succesful creation of > PG_VER_CATVER directory is enough as the current implement > does. If we take this way the documentation and pg_basebackup > should be changed and the problem will be resolved as the result. > > https://www.postgresql.org/docs/9.6/static/manage-ag-tablespaces.html > > - The location must be an existing, empty directory that is owned > - by the PostgreSQL operating system user. All objects subsequently > - created within the tablespace will be stored in files underneath > - this directory. > + CREATE TABLESPACE creates a subdirectory
Re: [HACKERS] Duplicate usage of tablespace location?
I don't mean that this is the only or best way to go. I apologize for the possible lack of explanation. At Thu, 06 Apr 2017 12:03:51 -0400, Tom Lane wrote in <21084.1491494...@sss.pgh.pa.us> > Kyotaro HORIGUCHI writes: > > I noticed by the following report, PostgreSQL can share the same > > directory as tablespaces of two servers with different > > pg-versions. > > > https://www.postgresql.org/message-id/2008148.rxBNyNRHPZ@peanuts2 > > > 8.4 checked that the tablespace location is empty, but from 9.0, > > the check is replaced with creating a PG_PGVER_CATVER > > subdirectory. This works for multiple servers with the same > > version, but don't for servers with different versions. > > Please explain why you think it doesn't work. This patch seems to > be reverting an intentional behavioral change, and you haven't I understand that the change is for in-place upgrade, not for sharing a tablespace diretory between two version of PostgreSQL servers. It actually rejects the second server with the same version to come. If this is correct, it doesn't seem right to accept the second server of the different version. If we allow sharing of the directory, theoretically we can allow the same between the same version of servers by adding system identifier in the subdirectory name. > really explained why we'd want to. It certainly doesn't look like > it addresses the referenced complaint about pg_basebackup behavior. My point is that "the direcotry for newly created tablespace is really reuiqred to be literary empty or not?" Practically it doesn't need to be empty and succesful creation of PG_VER_CATVER directory is enough as the current implement does. If we take this way the documentation and pg_basebackup should be changed and the problem will be resolved as the result. https://www.postgresql.org/docs/9.6/static/manage-ag-tablespaces.html - The location must be an existing, empty directory that is owned - by the PostgreSQL operating system user. All objects subsequently - created within the tablespace will be stored in files underneath - this directory. + CREATE TABLESPACE creates a subdirectory named after server + version in the location. The location must not contain a file + or directory of that name for the subdirectory. All objects + subsequently created within the tablespace will be stored in + files underneath the subdirectory. Then, modify pg_basebackup to follow the description above. 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
Re: [HACKERS] Duplicate usage of tablespace location?
Kyotaro HORIGUCHI writes: > I noticed by the following report, PostgreSQL can share the same > directory as tablespaces of two servers with different > pg-versions. > https://www.postgresql.org/message-id/2008148.rxBNyNRHPZ@peanuts2 > 8.4 checked that the tablespace location is empty, but from 9.0, > the check is replaced with creating a PG_PGVER_CATVER > subdirectory. This works for multiple servers with the same > version, but don't for servers with different versions. Please explain why you think it doesn't work. This patch seems to be reverting an intentional behavioral change, and you haven't really explained why we'd want to. It certainly doesn't look like it addresses the referenced complaint about pg_basebackup behavior. 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
[HACKERS] Duplicate usage of tablespace location?
Hello,
I noticed by the following report, PostgreSQL can share the same
directory as tablespaces of two servers with different
pg-versions.
https://www.postgresql.org/message-id/2008148.rxBNyNRHPZ@peanuts2
8.4 checked that the tablespace location is empty, but from 9.0,
the check is replaced with creating a PG_PGVER_CATVER
subdirectory. This works for multiple servers with the same
version, but don't for servers with different versions.
This is apparently a bug but doesn't cause any other problem
immediately. If we choose not to do this, the documentaion needs
to be edited instead.
https://www.postgresql.org/docs/9.6/static/manage-ag-tablespaces.html
| The location must be an existing, empty directory that is owned
| by the PostgreSQL operating system user.
regards,
-
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index f9c2620..6a89be1 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -606,23 +606,21 @@ create_tablespace_directories(const char *location, const Oid tablespaceoid)
}
}
+
/*
- * The creation of the version directory prevents more than one tablespace
- * in a single location.
+ * Check the target directory is empty.
*/
+ if (!directory_is_empty(location))
+ ereport(ERROR,
+(errcode(ERRCODE_OBJECT_IN_USE),
+ errmsg("directory \"%s\" is not empty",
+ location)));
+
if (mkdir(location_with_version_dir, S_IRWXU) < 0)
- {
- if (errno == EEXIST)
- ereport(ERROR,
- (errcode(ERRCODE_OBJECT_IN_USE),
- errmsg("directory \"%s\" already in use as a tablespace",
- location_with_version_dir)));
- else
- ereport(ERROR,
- (errcode_for_file_access(),
- errmsg("could not create directory \"%s\": %m",
- location_with_version_dir)));
- }
+ ereport(ERROR,
+(errcode_for_file_access(),
+ errmsg("could not create directory \"%s\": %m",
+ location_with_version_dir)));
/*
* In recovery, remove old symlink, in case it points to the wrong place.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
