On Fri, Oct 4, 2019 at 9:54 PM Pavel Stehule <pavel.stehule@gmail.com> wrote: > >> >> Can we add few tests for this feature. > > there are not any other test for DROP DATABASE >
I think there are no dedicated tests for it but in a few tests, we use it like in contrib\sepgsql\sql\alter.sql. I am not sure if we can write a predictable test for force option because it will never be guaranteed to drop the database in the presence of other active sessions.
done - I push tests to /tests/regress/psql.sql
Few more comments: --------------------------------- 1. + if (nprepared > 0) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_IN_USE), + errmsg("database \"%s\" is used by %d prepared transaction(s)", + get_database_name(databaseId), + nprepared))); +
You need to use errdetail_plural here to avoid translation problems, see docs[1] for a detailed explanation. You can use function errdetail_busy_db. Also, the indentation is not proper.
fixed
2. TerminateOtherDBBackends() { .. + /* We know so we have all necessary rights now */ + foreach (lc, pids) + { + int pid = lfirst_int(lc); + PGPROC *proc = BackendPidGetProc(pid); + + if (proc != NULL) + { + /* If we have setsid(), signal the backend's whole process group */ +#ifdef HAVE_SETSID + (void) kill(-pid, SIGTERM); +#else + (void) kill(pid, SIGTERM); +#endif + } + } + + /* sleep 100ms */ + pg_usleep(100 * 1000L); .. }
So here we are sending SIGTERM to all the processes and wait for 100ms to allow them to exit. Have you tested this with many processes? I think we can create 100~500 sessions or maybe more to the database being dropped and see what is behavior? One thing to notice is that in function CountOtherDBBackends() we are sending SIGTERM to 10 autovacuum processes at-a-time. That has been introduced in commit 4abd7b49f1e9, but the reason for the same is not mentioned. I am not sure if it is to avoid sending SIGTERM to many processes in quick succession.
I tested it on linux Linux nemesis
5.3.6-300.fc31.x86_64 #1 SMP Mon Oct 14 12:26:42 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Tested with 1800 connections without any problem (under low load (only pg_sleep was called).
I think there should be more comments atop TerminateOtherDBBackends to explain the working of it and some assumptions of that function.
