When you say 'without any problem', do you mean to say that the drop database is successful? In your tests were those sessions idle? If so, I think we should try with busy sessions. Also, if you write any script to do these tests, kindly share the same so that others can also repeat those tests.
I run pgbench on database with -i -S 100 and -c 1000. It is maximum for my notebook. There was high load - about 50, and still DROP DATABASE FORCE is working without any problems
>(under low load (only pg_sleep was called). >
I guess this is also possible because immediately after TerminateOtherDBBackends, we call CountOtherDBBackends which again give 5s to allow active connections to die. I am wondering why not we call CountOtherDBBackends from TerminateOtherDBBackends after sending the SIGTERM to all the sessions that are connected to the database being dropped? Currently, it looks odd that first, we wait for 100ms after sending the signal and then separately wait for 5s in another function.
I checked code, and would not to change calls. Now I think the code is well readable and has logical sense. But we can decrease sleep in TerminateOtherDBBackends from 100 ms to 5 ms (just to increase chance to be all killed processes done, and then waiting in CountOtherDBBackends 100ms will not be hit.
Other comments: 1. diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql index 26a0bcf718..c8f545be18 100644 --- a/src/test/regress/sql/psql.sql +++ b/src/test/regress/sql/psql.sql @@ -1182,3 +1182,11 @@ drop role regress_partitioning_role;
-- \d on toast table (use pg_statistic's toast table, which has a known name) \d pg_toast.pg_toast_2619 + +-- +-- DROP DATABASE FORCE test of syntax (should not to raise syntax error) +-- +drop database not_exists (force); +drop database not_exists with (force); +drop database if exists not_exists (force); +drop database if exists not_exists with (force);
I don't think psql.sql is the right place to add such tests. Actually, there doesn't appear to be any database specific test file. However, if we want to add to any existing file, then maybe drop_if_exits.sql could be a better place for these tests as compare to psql.sql.
moved
2. + if (nprepared > 0) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_IN_USE), + errmsg_plural("database \"%s\" is used by %d prepared transaction", + "database \"%s\" is used by %d prepared transactions", + nprepared, + get_database_name(databaseId), nprepared)));
I think it is better if we mimic exactly what we have in the failure of CountOtherDBBackends.