Re: dropdb --force

Hi


> 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.
>

done

Regards

Pavel


> --
> With Regards,
> Amit Kapila.
> EnterpriseDB: http://www.enterprisedb.com
>