DROP DATABASE should remove shared buffer cache entries

This used to be handled by persistent tables. When persistent tables were
removed, we forgot to add back the dropping of shared buffer cache entries as
part of DROP DATABASE operation. This commit adds it back along with a test so
we do not ever forget.

Some relevant comments from Heikki/Jesse:
The reason why this issue was not seen is because there is a RequestCheckpoint()
near the end of dropdb(). So before dropdb() actually removes the files for the
database, all dirty buffers have already been flushed to disk. The buffer
manager will not try to write out clean buffers back to disk, so they will just
age out of the buffer cache over time.

One way this issue could have shown itself if we did not catch this could be the
rare scenario that the same database OID is reused later for a different
database, where this could cause false positives in future buffer cache lookups.

src/backend/commands/dbcommands.c

@@ -970,6 +970,13 @@ dropdb(const char *dbname, bool missing_ok)
 	 */
 	dropDatabaseDependencies(db_id);
 
+	/*
+	 * Drop pages for this database that are in the shared buffer cache. This
+	 * is important to ensure that no remaining backend tries to write out a
+	 * dirty buffer to the dead database later...
+	 */
+	DropDatabaseBuffers(db_id);
+
 	/*
 	 * Tell the stats collector to forget it immediately, too.
 	 */

src/test/regress/expected/.gitignore

@@ -40,3 +40,4 @@ partition_ddl.out
 autovacuum-template0.out
 rpt_tpch.out
 session_reset.out
+dropdb_check_shared_buffer_cache.out

src/test/regress/greenplum_schedule

@@ -219,7 +219,7 @@ test: metadata_track
 test: workfile_mgr_test
 test: session_reset
 
-test: psql_gp_commands pg_resetxlog
+test: psql_gp_commands pg_resetxlog dropdb_check_shared_buffer_cache
 
 # Check for shmem leak for instrumentation slots
 test: instr_in_shmem_verify

src/test/regress/input/dropdb_check_shared_buffer_cache.source

+-- Test that dropping a database will drop pages in the shared buffer cache.
+
+CREATE OR REPLACE FUNCTION check_shared_buffer_cache_for_dboid(Oid) RETURNS BOOL
+AS '@abs_srcdir@/regress.so', 'check_shared_buffer_cache_for_dboid'
+LANGUAGE C;
+
+-- Create a new database and a table. This should create entries in the shared
+-- buffer cache with the database Oid in the entries' buffer tag.
+CREATE DATABASE dropdb_check_shared_buffer_cache;
+\c dropdb_check_shared_buffer_cache
+CREATE TABLE dropdb_check_shared_buffer_cache_table (a int);
+\c regression
+
+-- Store the new database's Oid for later use
+-- start_ignore
+SELECT oid AS dropdb_check_shared_buffer_cache_dboid
+  FROM pg_database WHERE datname = 'dropdb_check_shared_buffer_cache';
+-- end_ignore
+\gset
+
+-- We expect 'true' that the shared buffer cache contains pages related to our
+-- newly created database.
+SELECT check_shared_buffer_cache_for_dboid(:dropdb_check_shared_buffer_cache_dboid);
+
+-- Now drop our database. This should trigger dropping of pages for this
+-- database that are in the shared buffer cache.
+DROP DATABASE dropdb_check_shared_buffer_cache;
+
+-- We expect 'false' that the shared buffer cache contains pages related to our
+-- dropped database.
+SELECT check_shared_buffer_cache_for_dboid(:dropdb_check_shared_buffer_cache_dboid);

src/test/regress/output/dropdb_check_shared_buffer_cache.source

+-- Test that dropping a database will drop pages in the shared buffer cache.
+CREATE OR REPLACE FUNCTION check_shared_buffer_cache_for_dboid(Oid) RETURNS BOOL
+AS '@abs_srcdir@/regress.so', 'check_shared_buffer_cache_for_dboid'
+LANGUAGE C;
+-- Create a new database and a table. This should create entries in the shared
+-- buffer cache with the database Oid in the entries' buffer tag.
+CREATE DATABASE dropdb_check_shared_buffer_cache;
+\c dropdb_check_shared_buffer_cache
+CREATE TABLE dropdb_check_shared_buffer_cache_table (a int);
+\c regression
+-- Store the new database's Oid for later use
+-- start_ignore
+SELECT oid AS dropdb_check_shared_buffer_cache_dboid
+  FROM pg_database WHERE datname = 'dropdb_check_shared_buffer_cache';
+ dropdb_check_shared_buffer_cache_dboid 
+----------------------------------------
+                         <database oid>
+(1 row)
+
+-- end_ignore
+\gset
+-- We expect 'true' that the shared buffer cache contains pages related to our
+-- newly created database.
+SELECT check_shared_buffer_cache_for_dboid(:dropdb_check_shared_buffer_cache_dboid);
+ check_shared_buffer_cache_for_dboid 
+-------------------------------------
+ t
+(1 row)
+
+-- Now drop our database. This should trigger dropping of pages for this
+-- database that are in the shared buffer cache.
+DROP DATABASE dropdb_check_shared_buffer_cache;
+-- We expect 'false' that the shared buffer cache contains pages related to our
+-- dropped database.
+SELECT check_shared_buffer_cache_for_dboid(:dropdb_check_shared_buffer_cache_dboid);
+ check_shared_buffer_cache_for_dboid 
+-------------------------------------
+ f
+(1 row)
+

src/test/regress/regress_gp.c

@@ -40,6 +40,8 @@
 #include "executor/spi.h"
 #include "port/atomics.h"
 #include "parser/parse_expr.h"
+#include "storage/bufmgr.h"
+#include "storage/buf_internals.h"
 #include "libpq/auth.h"
 #include "libpq/hba.h"
 #include "utils/builtins.h"
@@ -93,6 +95,9 @@ extern Datum check_auth_time_constraints(PG_FUNCTION_ARGS);
 extern Datum test_consume_xids(PG_FUNCTION_ARGS);
 extern Datum gp_execute_on_server(PG_FUNCTION_ARGS);
 
+/* Check shared buffer cache for a database Oid */
+extern Datum check_shared_buffer_cache_for_dboid(PG_FUNCTION_ARGS);
+
 /* Triggers */
 
 typedef struct
@@ -2016,3 +2021,26 @@ gp_execute_on_server(PG_FUNCTION_ARGS)
 	else
 		PG_RETURN_BOOL(true);
 }
+
+/*
+ * Check if the shared buffer cache contains any pages that have the specified
+ * database OID in their buffer tag. Return true if an entry is found, else
+ * return false.
+ */
+PG_FUNCTION_INFO_V1(check_shared_buffer_cache_for_dboid);
+Datum
+check_shared_buffer_cache_for_dboid(PG_FUNCTION_ARGS)
+{
+	Oid databaseOid = PG_GETARG_OID(0);
+	int i;
+
+	for (i = 0; i < NBuffers; i++)
+	{
+		volatile BufferDesc *bufHdr = &BufferDescriptors[i];
+
+		if (bufHdr->tag.rnode.dbNode == databaseOid)
+			PG_RETURN_BOOL(true);
+	}
+
+	PG_RETURN_BOOL(false);
+}

src/test/regress/sql/.gitignore

@@ -39,3 +39,4 @@ partition_ddl.sql
 autovacuum-template0.sql
 rpt_tpch.sql
 session_reset.sql
+dropdb_check_shared_buffer_cache.sql