Re: dropdb --force

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: vignesh C <vignesh21(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Ryan Lambert <ryan(at)rustprooflabs(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Anthony Nowocien <anowocien(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Filip Rembiałkowski <filip(dot)rembialkowski(at)gmail(dot)com>
Subject: Re: dropdb --force
Date: 2019-10-21 07:22:21
Message-ID: CAFj8pRDp+1OiqWtRV2wGcA0tMc46T5pU6E+SwKsxdXSpjUok7w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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
>

Attachment Content-Type Size
drop-database-force-20191021.patch text/x-patch 15.3 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Thunder 2019-10-21 07:40:24 [BUG] standby node can not provide service even it replays all log files
Previous Message Amit Langote 2019-10-21 07:08:53 Re: adding partitioned tables to publications