Thread: Initdb failure
Hi, Initdb fails when following path is provided as input: datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/ Also the cleanup also tends to fail in the cleanup path. Could be something to do with path handling. I'm not sure if this is already known. Regards, Vignesh EnterpriseDB: http://www.enterprisedb.com
On Thu, 25 Jul 2019 at 07:39, vignesh C <vignesh21@gmail.com> wrote: > > Hi, > > Initdb fails when following path is provided as input: > datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/ > > Also the cleanup also tends to fail in the cleanup path. > > Could be something to do with path handling. This is because the value of MAXPGPATH is 1024 and the path you are providing is more than that. Hence, when it is trying to read PG_VERSION in ValidatePgVersion it is going to a wrong path with just 1024 characters. > I'm not sure if this is already known. I am also not sure if it is known or intentional. On the other hand I also don't know if it is practical to give such long names for database directory anyway. -- Regards, Rafia Sabih
On Thu, Jul 25, 2019 at 4:52 PM Rafia Sabih <rafia.pghackers@gmail.com> wrote: > > On Thu, 25 Jul 2019 at 07:39, vignesh C <vignesh21@gmail.com> wrote: > > > > Hi, > > > > Initdb fails when following path is provided as input: > > datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/ > > > > Also the cleanup also tends to fail in the cleanup path. > > > > Could be something to do with path handling. > This is because the value of MAXPGPATH is 1024 and the path you are > providing is more than that. Hence, when it is trying to read > PG_VERSION in ValidatePgVersion it is going to a wrong path with just > 1024 characters. > The error occurs at a very later point after performing the initial work like creating directory. I'm thinking we should check this in the beginning and throw the error message at the beginning and exit cleanly. > > > I'm not sure if this is already known. > I am also not sure if it is known or intentional. On the other hand I > also don't know if it is practical to give such long names for > database directory anyway. > Usually they will not be using such long path, but this is one of the odd scenarios. Regards, Vignesh EnterpriseDB: http://www.enterprisedb.com
On Thu, 25 Jul 2019 at 13:50, vignesh C <vignesh21@gmail.com> wrote: > > On Thu, Jul 25, 2019 at 4:52 PM Rafia Sabih <rafia.pghackers@gmail.com> wrote: > > > > On Thu, 25 Jul 2019 at 07:39, vignesh C <vignesh21@gmail.com> wrote: > > > > > > Hi, > > > > > > Initdb fails when following path is provided as input: > > > datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/ > > > > > > Also the cleanup also tends to fail in the cleanup path. > > > > > > Could be something to do with path handling. > > This is because the value of MAXPGPATH is 1024 and the path you are > > providing is more than that. Hence, when it is trying to read > > PG_VERSION in ValidatePgVersion it is going to a wrong path with just > > 1024 characters. > > > > The error occurs at a very later point after performing the initial > work like creating directory. I'm thinking we should check this in > the beginning and throw the error message at the beginning and exit > cleanly. > Now that you say this, it does make sense to atleast inform about the correct error and that too earlier. Something like the attached patch would make sense. -- Regards, Rafia Sabih
Attachment
Rafia Sabih <rafia.pghackers@gmail.com> writes:
> On Thu, 25 Jul 2019 at 13:50, vignesh C <vignesh21@gmail.com> wrote:
>>> Initdb fails when following path is provided as input:
>>>
datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/
> Now that you say this, it does make sense to atleast inform about the
> correct error and that too earlier. Something like the attached patch
> would make sense.
I am not terribly excited about putting effort into this at all, because
I don't think that any actual user anywhere will ever get any benefit.
The proposed test case is just silly.
However, if we are going to put effort into it, it needs to be more than
this. First off, what is the actual failure point? (It's surely less
than MAXPGPATH, because we tend to append subdirectory/file names onto
whatever is given.) Second, what of absolute versus relative paths?
If converting the given path to absolute makes it exceed MAXPGPATH,
is that a problem?
regards, tom lane
On Thu, 25 Jul 2019 at 16:44, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Rafia Sabih <rafia.pghackers@gmail.com> writes: > > On Thu, 25 Jul 2019 at 13:50, vignesh C <vignesh21@gmail.com> wrote: > >>> Initdb fails when following path is provided as input: > >>> datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/ > > > Now that you say this, it does make sense to atleast inform about the > > correct error and that too earlier. Something like the attached patch > > would make sense. > > I am not terribly excited about putting effort into this at all, because > I don't think that any actual user anywhere will ever get any benefit. > The proposed test case is just silly. That I totally agree upon! But on the other hand emitting the right error message atleast would be good for the sake of correctness if nothing else. But yes that definitely should be weighed against what is the effort required for this. -- Regards, Rafia Sabih
On Thu, Jul 25, 2019 at 8:39 PM Rafia Sabih <rafia.pghackers@gmail.com> wrote: > > On Thu, 25 Jul 2019 at 16:44, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > > > Rafia Sabih <rafia.pghackers@gmail.com> writes: > > > On Thu, 25 Jul 2019 at 13:50, vignesh C <vignesh21@gmail.com> wrote: > > >>> Initdb fails when following path is provided as input: > > >>> datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/datasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafddsdatasadfasfdsafdds/ > > > > > Now that you say this, it does make sense to atleast inform about the > > > correct error and that too earlier. Something like the attached patch > > > would make sense. > > > > I am not terribly excited about putting effort into this at all, because > > I don't think that any actual user anywhere will ever get any benefit. > > The proposed test case is just silly. > > That I totally agree upon! > But on the other hand emitting the right error message atleast would > be good for the sake of correctness if nothing else. But yes that > definitely should be weighed against what is the effort required for > this. > Thanks Tom for your opinion. Thanks Rafia for your thoughts and effort in making the patch. I'm not sure if we are planning to fix this. If we are planning to fix, one suggestion from my side we can choose a safe length which would include the subdirectories and file paths. I think one of these will be the longest: base/database_oid/tables pg_wal/archive_status/ pg_wal/archive_file Fix can be something like: MAXPGPATH - (LONGEST_PATH_FROM_ABOVE) Regards, Vignesh EnterpriseDB: http://www.enterprisedb.com
On 2019-07-25 17:09, Rafia Sabih wrote: > But on the other hand emitting the right error message atleast would > be good for the sake of correctness if nothing else. But yes that > definitely should be weighed against what is the effort required for > this. I think if you want to make this more robust, get rid of the fixed-size array, use dynamic allocation with PQExpBuffer, and let the operating system complain if it doesn't like the directory name length. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Sat, Jul 27, 2019 at 2:22 AM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
> I think if you want to make this more robust, get rid of the fixed-size
> array, use dynamic allocation with PQExpBuffer, and let the operating
> system complain if it doesn't like the directory name length.
Agreed, but I think we should just do nothing. To actually fix this
in general, we'd have to get rid of every instance of MAXPGPATH in the
source tree:
[rhaas pgsql]$ git grep MAXPGPATH | wc -l
611
If somebody feels motivated to spend that amount of effort improving
this, I will stand back and cheer from the sidelines, but that's gonna
be a LOT of work for a problem that, as Tom says, is probably not
really affecting very many people.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> Agreed, but I think we should just do nothing. To actually fix this
> in general, we'd have to get rid of every instance of MAXPGPATH in the
> source tree:
> [rhaas pgsql]$ git grep MAXPGPATH | wc -l
> 611
I don't think it'd really be necessary to go that far. One of the
reasons we chdir to the data directory at postmaster start is so that
(pretty nearly) all filenames that backends deal with are relative
pathnames of very predictable, short lengths. So a lot of those
MAXPGPATH uses are probably perfectly safe, indeed likely overkill.
However, identifying which ones are not safe would still take looking
at every use case, so I agree there'd be a lot of work here.
Would there be any value in teaching initdb to do something similar,
ie chdir to the parent of the target datadir location? Then the set
of places in initdb that have to deal with long pathnames would be
pretty well constrained.
On the whole though, I don't have a problem with the "do nothing"
answer. There's no security risk here, and no issue that seems
likely to arise in actual use cases rather than try-to-break-it
test scripts.
regards, tom lane
On Tue, Jul 30, 2019 at 03:30:12PM -0400, Tom Lane wrote: > On the whole though, I don't have a problem with the "do nothing" > answer. There's no security risk here, and no issue that seems > likely to arise in actual use cases rather than try-to-break-it > test scripts. +1. -- Michael