Re: [PATCH] improve the pg_upgrade error message
> On 1 Dec 2021, at 11:15, Daniel Gustafsson wrote: > >> On 1 Dec 2021, at 10:59, Jeevan Ladhe wrote: > >> Was wondering if we had any barriers to getting this committed. > > No barrier other than available time to, I will try to get to it shortly. The "shortly" aspect wasn't really fulfilled, but I got around to taking another look at this today and pushed it now with a few small changes to reflect how pg_upgrade has changed. -- Daniel Gustafsson https://vmware.com/
Re: [PATCH] improve the pg_upgrade error message
On Wed, Dec 1, 2021 at 3:45 PM Daniel Gustafsson wrote: > > On 1 Dec 2021, at 10:59, Jeevan Ladhe > wrote: > > > Was wondering if we had any barriers to getting this committed. > > No barrier other than available time to, I will try to get to it shortly. > Great! Thank you. > -- > Daniel Gustafsson https://vmware.com/ > >
Re: [PATCH] improve the pg_upgrade error message
> On 1 Dec 2021, at 10:59, Jeevan Ladhe wrote: > Was wondering if we had any barriers to getting this committed. No barrier other than available time to, I will try to get to it shortly. -- Daniel Gustafsson https://vmware.com/
Re: [PATCH] improve the pg_upgrade error message
Hi Daniel, Was wondering if we had any barriers to getting this committed. I believe it will be good to have this change and also it will be more in line with other check functions also. Regards, Jeevan On Thu, Oct 21, 2021 at 3:51 PM Daniel Gustafsson wrote: > > On 14 Jul 2021, at 07:27, Suraj Kharage > wrote: > > > Overall patch looks good to me. > > Agreed, I think this is a good change and in line with how the check > functions > work in general. > > > Instead of giving suggestion about updating the pg_database catalog, can > we give "ALTER DATABASE ALLOW_CONNECTIONS true;" command? > > I would actually prefer to not give any suggestions at all, we typically > don't > in these error messages. Since there are many ways to do it (dropping the > database being one) I think leaving that to the user is per application > style. > > > Also, it would be good if we give 2 spaces after full stop in an error > message. > > Correct, fixed in the attached which also tweaks the language slightly to > match > other errors. > > I propose to commit the attached, which also adds a function comment while > there, unless there are objections. > > -- > Daniel Gustafsson https://vmware.com/ > >
Re: [PATCH] improve the pg_upgrade error message
> On 14 Jul 2021, at 07:27, Suraj Kharage > wrote: > Overall patch looks good to me. Agreed, I think this is a good change and in line with how the check functions work in general. > Instead of giving suggestion about updating the pg_database catalog, can we > give "ALTER DATABASE ALLOW_CONNECTIONS true;" command? I would actually prefer to not give any suggestions at all, we typically don't in these error messages. Since there are many ways to do it (dropping the database being one) I think leaving that to the user is per application style. > Also, it would be good if we give 2 spaces after full stop in an error > message. Correct, fixed in the attached which also tweaks the language slightly to match other errors. I propose to commit the attached, which also adds a function comment while there, unless there are objections. -- Daniel Gustafsson https://vmware.com/ 0001-List-offending-databases-in-pg_upgrade-datallowconn-.patch Description: Binary data
Re: [PATCH] improve the pg_upgrade error message
Thanks Jeevan for working on this.
Overall patch looks good to me.
+ pg_fatal("All non-template0 databases must allow connections, i.e.
their\n"
+ "pg_database.datallowconn must be true. Your installation contains\n"
+ "non-template0 databases with their pg_database.datallowconn set to\n"
+ "false. Consider allowing connection for all non-template0 databases\n"
+ "using:\n"
+ "UPDATE pg_catalog.pg_database SET datallowconn='true' WHERE datname
NOT LIKE 'template0';\n"
+ "A list of databases with the problem is given in the file:\n"
+ "%s\n\n", output_path);
Instead of giving suggestion about updating the pg_database catalog, can we
give "ALTER DATABASE ALLOW_CONNECTIONS true;" command?
Also, it would be good if we give 2 spaces after full stop in an error
message.
On Tue, Jul 13, 2021 at 6:57 PM Jeevan Ladhe
wrote:
>
> The admin might fix DB123, restart their upgrade procedure, spend 5 or 15
>> minutes with that, only to have it then fail on DB1234.
>>
>
> Agree with this observation.
>
> Here is a patch that writes the list of all the databases other than
> template0
> that are having their pg_database.datallowconn to false in a file. Similar
> approach is seen in other functions like check_for_data_types_usage(),
> check_for_data_types_usage() etc. Thanks Suraj Kharage for the offline
> suggestion.
>
> PFA patch.
>
> For experiment, here is how it turns out after the fix.
>
> postgres=# update pg_database set datallowconn='false' where datname in
> ('mydb', 'mydb1', 'mydb2');
> UPDATE 3
>
> $ pg_upgrade -d /tmp/v96/data -D /tmp/v13/data -b $HOME/v96/install/bin -B
> $HOME/v13/install/bin
> Performing Consistency Checks
> -
> Checking cluster versions ok
> Checking database user is the install user ok
> Checking database connection settings fatal
>
> All non-template0 databases must allow connections, i.e. their
> pg_database.datallowconn must be true. Your installation contains
> non-template0 databases with their pg_database.datallowconn set to
> false. Consider allowing connection for all non-template0 databases
> using:
> UPDATE pg_catalog.pg_database SET datallowconn='true' WHERE datname
> NOT LIKE 'template0';
> A list of databases with the problem is given in the file:
> databases_with_datallowconn_false.txt
>
> Failure, exiting
>
> $ cat databases_with_datallowconn_false.txt
> mydb
> mydb1
> mydb2
>
>
> Regards,
> Jeevan Ladhe
>
--
--
Thanks & Regards,
Suraj kharage,
edbpostgres.com
Re: [PATCH] improve the pg_upgrade error message
> The admin might fix DB123, restart their upgrade procedure, spend 5 or 15
> minutes with that, only to have it then fail on DB1234.
>
Agree with this observation.
Here is a patch that writes the list of all the databases other than
template0
that are having their pg_database.datallowconn to false in a file. Similar
approach is seen in other functions like check_for_data_types_usage(),
check_for_data_types_usage() etc. Thanks Suraj Kharage for the offline
suggestion.
PFA patch.
For experiment, here is how it turns out after the fix.
postgres=# update pg_database set datallowconn='false' where datname in
('mydb', 'mydb1', 'mydb2');
UPDATE 3
$ pg_upgrade -d /tmp/v96/data -D /tmp/v13/data -b $HOME/v96/install/bin -B
$HOME/v13/install/bin
Performing Consistency Checks
-
Checking cluster versions ok
Checking database user is the install user ok
Checking database connection settings fatal
All non-template0 databases must allow connections, i.e. their
pg_database.datallowconn must be true. Your installation contains
non-template0 databases with their pg_database.datallowconn set to
false. Consider allowing connection for all non-template0 databases
using:
UPDATE pg_catalog.pg_database SET datallowconn='true' WHERE datname NOT
LIKE 'template0';
A list of databases with the problem is given in the file:
databases_with_datallowconn_false.txt
Failure, exiting
$ cat databases_with_datallowconn_false.txt
mydb
mydb1
mydb2
Regards,
Jeevan Ladhe
v2-0001-Improve-the-pg_upgrade-error-message.patch
Description: Binary data
Re: [PATCH] improve the pg_upgrade error message
On Mon, Jul 12, 2021 at 02:06:31PM +0200, Laurenz Albe wrote: > On Mon, 2021-07-12 at 16:58 +0530, Jeevan Ladhe wrote: > > While looking into one of the pg_upgrade issue, I found it > > challenging to find out the database that has the datallowconn set to > > 'false' that was throwing following error: > > > > "All non-template0 databases must allow connections, i.e. their > > pg_database.datallowconn must be true" > > > > It can be argued that we can query the pg_database > > catalog and find that out easily, but at times it is challenging to get > > that from the customer environment. > > > > With attached patch, now I get following error: > > "All non-template0 databases must allow connections, i.e. their > > pg_database.datallowconn must be true; database "mydb" has datallowconn set > > to false." > > I am in favor of that in principle, but I think that additional information > should be separate line. I think the style for this kind of error is established by commit 1634d3615. This shows "In database: ..." More importantly, if you're going to show the name of the problematic DB, you should show the name of *all* the problem DBs. Otherwise it gives the impression the upgrade will progress if you fix just that one. The admin might fix DB123, restart their upgrade procedure, spend 5 or 15 minutes with that, only to have it then fail on DB1234. -- Justin
Re: [PATCH] improve the pg_upgrade error message
+1 for the change. Patch looks good to me. On Mon, Jul 12, 2021 at 4:59 PM Jeevan Ladhe wrote: > While looking into one of the pg_upgrade issue, I found it > > challenging to find out the database that has the datallowconn set to > > 'false' that was throwing following error: > > > *"All non-template0 databases must allow connections, i.e. their > pg_database.datallowconn must be true"* > > > edb=# create database mydb; > > CREATE DATABASE > > edb=# update pg_database set datallowconn='false' where datname like > 'mydb'; > > UPDATE 1 > > > Now, when I try to upgrade the server, without the patch we get above > > error, which leaves no clue behind about which database has datallowconn > > set to 'false'. It can be argued that we can query the pg_database > > catalog and find that out easily, but at times it is challenging to get > > that from the customer environment. But, anyways I feel we have scope to > > improve the error message here per the attached patch. > > > With attached patch, now I get following error: > > *"All non-template0 databases must allow connections, i.e. their > pg_database.datallowconn must be true; database "mydb" has datallowconn set > to false."* > > > > Regards, > > Jeevan Ladhe > -- -- Thanks & Regards, Suraj kharage, edbpostgres.com
Re: [PATCH] improve the pg_upgrade error message
On Mon, 2021-07-12 at 16:58 +0530, Jeevan Ladhe wrote: > While looking into one of the pg_upgrade issue, I found it > challenging to find out the database that has the datallowconn set to > 'false' that was throwing following error: > > "All non-template0 databases must allow connections, i.e. their > pg_database.datallowconn must be true" > > It can be argued that we can query the pg_database > catalog and find that out easily, but at times it is challenging to get > that from the customer environment. > > With attached patch, now I get following error: > "All non-template0 databases must allow connections, i.e. their > pg_database.datallowconn must be true; database "mydb" has datallowconn set > to false." I am in favor of that in principle, but I think that additional information should be separate line. Yours, Laurenz Albe
[PATCH] improve the pg_upgrade error message
While looking into one of the pg_upgrade issue, I found it challenging to find out the database that has the datallowconn set to 'false' that was throwing following error: *"All non-template0 databases must allow connections, i.e. their pg_database.datallowconn must be true"* edb=# create database mydb; CREATE DATABASE edb=# update pg_database set datallowconn='false' where datname like 'mydb'; UPDATE 1 Now, when I try to upgrade the server, without the patch we get above error, which leaves no clue behind about which database has datallowconn set to 'false'. It can be argued that we can query the pg_database catalog and find that out easily, but at times it is challenging to get that from the customer environment. But, anyways I feel we have scope to improve the error message here per the attached patch. With attached patch, now I get following error: *"All non-template0 databases must allow connections, i.e. their pg_database.datallowconn must be true; database "mydb" has datallowconn set to false."* Regards, Jeevan Ladhe 0001-Improve-the-pg_upgrade-error-message.patch Description: Binary data
