Skip to content

Commit

Permalink
gpcrondump should not dump pg_temp schemas
Browse files Browse the repository at this point in the history
* We noticed that when gpcrondump is given with a -S option (exclude schema)
  gpcrondump will also try to dump pg_temp schemas.
  This can be detected by the following issue:
  user creates a new schema
  user creates a temp table
  -- Keep the session alive

  On another session:
  Run gpcrondump -S <exclude schema> -x <dbname>

* We've filtered out pg_temp in our queries to prevent this issue.

Dumping pg_temp schemas doesn't make sense as gpdbrestore can't restore
those schemas and tables anyways, since pg_ and gp_ schemas are
restricted for the system.

Make sure to ignore pg_temp from backup.

* We want to make sure that we ignore pg_temp and not error out if we
  can't find the pg_temp tables during a backup

Authors: Marbin Tan, Karen Huddleston, & Chumki Roy
  • Loading branch information
Chibin committed Jul 28, 2016
1 parent 86e49e5 commit 8d26668
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 14 deletions.
19 changes: 14 additions & 5 deletions gpMgmt/bin/gpcrondump
Original file line number Diff line number Diff line change
Expand Up @@ -599,18 +599,27 @@ class GpCronDump(Operation):
if self.context.incremental:
return (dirty_file, exclude_file)

# Make sure that this happens right before the include_dump_tables
# check. We are relying on the filtering that happens on the
# include_dump_tables check
if self.context.include_dump_tables_file:
self.context.include_dump_tables = get_lines_from_file(self.context.include_dump_tables_file)

if self.context.include_dump_tables:
include_file = expand_partitions_and_populate_filter_file(dbname, self.context.include_dump_tables, 'include_dump_tables_file')
dump_tables_without_pg_temp = []
for table in self.context.include_dump_tables:
if table.startswith('pg_temp_'):
continue
dump_tables_without_pg_temp.append(table)
include_file = expand_partitions_and_populate_filter_file(dbname,
dump_tables_without_pg_temp,
'include_dump_tables_file')
self.context.include_dump_tables = []

if self.context.exclude_dump_tables:
exclude_file = expand_partitions_and_populate_filter_file(dbname, self.context.exclude_dump_tables, 'exclude_dump_tables_file')
self.context.exclude_dump_tables = []

if self.context.include_dump_tables_file:
include_tables = get_lines_from_file(self.context.include_dump_tables_file)
include_file = expand_partitions_and_populate_filter_file(dbname, include_tables, 'include_dump_tables_file')

if self.context.exclude_dump_tables_file:
exclude_tables = get_lines_from_file(self.context.exclude_dump_tables_file)
exclude_file = expand_partitions_and_populate_filter_file(dbname,
Expand Down
15 changes: 12 additions & 3 deletions gpMgmt/bin/gppylib/operations/dump.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@
'pg_toast'
]

IGNORE_SCHEMA = [
'pg_temp'
]

GET_ALL_DATATABLES_SQL = """
SELECT ALLTABLES.oid, ALLTABLES.schemaname, ALLTABLES.tablename FROM
Expand Down Expand Up @@ -64,11 +68,12 @@
WHERE c.relkind = 'r'::"char" AND c.oid > 16384 AND n.nspname = '%s'
"""

# Generally used to validate that the table(s) we want to backup
GET_ALL_USER_TABLES_SQL = """
SELECT n.nspname AS schemaname, c.relname AS tablename
FROM pg_class c LEFT JOIN pg_namespace n ON n.oid = c.relnamespace
LEFT JOIN pg_tablespace t ON t.oid = c.reltablespace
WHERE c.relkind = 'r'::"char" AND c.oid > 16384 AND (c.relnamespace > 16384 or n.nspname = 'public')
WHERE c.relkind = 'r'::"char" AND c.oid > 16384 AND (c.relnamespace > 16384 or n.nspname = 'public') AND nspname NOT LIKE 'pg_temp_%'
"""

GET_APPENDONLY_DATA_TABLE_INFO_SQL = """
Expand Down Expand Up @@ -103,7 +108,7 @@
ORDER BY objid, staactionname
""" % GET_ALL_AO_CO_DATATABLES_SQL

GET_ALL_SCHEMAS_SQL = "SELECT nspname from pg_namespace;"
GET_ALL_SCHEMAS_SQL = "SELECT nspname from pg_namespace WHERE nspname NOT LIKE 'pg_temp_%';"

def get_include_schema_list_from_exclude_schema(context, exclude_schema_list):
"""
Expand All @@ -114,7 +119,9 @@ def get_include_schema_list_from_exclude_schema(context, exclude_schema_list):
include_schema_list = []
schema_list = execute_sql(GET_ALL_SCHEMAS_SQL, context.master_port, context.dump_database)
for schema in schema_list:
if schema[0] not in exclude_schema_list and schema[0] not in CATALOG_SCHEMA:
if schema[0] not in exclude_schema_list \
and schema[0] not in CATALOG_SCHEMA \
and schema[0] not in IGNORE_SCHEMA:
include_schema_list.append(schema[0])

return include_schema_list
Expand Down Expand Up @@ -1065,6 +1072,8 @@ def execute(self):
for dump_table in dump_tables:
if '.' not in dump_table:
raise ExceptionNoStackTraceNeeded("No schema name supplied for table %s" % dump_table)
if dump_table.startswith('pg_temp_'):
continue
schema, table = split_fqn(dump_table)
exists = CheckTableExists(self.context, schema, table).run()
if not exists:
Expand Down
14 changes: 14 additions & 0 deletions gpMgmt/bin/gppylib/test/behave/mgmt_utils/backup.feature
Original file line number Diff line number Diff line change
Expand Up @@ -3501,6 +3501,20 @@ Feature: Validate command line arguments
When the user runs "psql -c 'DROP ROLE "Foo%user"' -d bkdb"
Then psql should return a return code of 0
@exclude_schema
Scenario: Exclude schema (-S) should not dump pg_temp schemas
Given the test is initialized
And the user runs the command "psql bkdb -f 'gppylib/test/behave/mgmt_utils/steps/data/gpcrondump/create_temp_schema_in_transaction.sql'" in the background without sleep
When the user runs "gpcrondump -a -S good_schema -x bkdb"
Then gpcrondump should return a return code of 0
And the timestamp from gpcrondump is stored
Then read pid from file "gppylib/test/behave/mgmt_utils/steps/data/gpcrondump/pid_leak" and kill the process
And the temporary file "gppylib/test/behave/mgmt_utils/steps/data/gpcrondump/pid_leak" is removed
And waiting "2" seconds
And verify that the "dump" file in " " dir does not contain "pg_temp"
And the user runs command "dropdb bkdb"
# THIS SHOULD BE THE LAST TEST
@backupfire
Scenario: cleanup for backup feature
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
\t
create temp table temp_table(i int);
create schema good_schema;
create table good_schema.good_table(i int);
\o gppylib/test/behave/mgmt_utils/steps/data/gpcrondump/pid_leak
select pg_backend_pid();
begin;
select * from temp_table;
-- we are just picking an arbitrary number that's long enough for gpcrondump to finish
-- we will kill this process once the dump is done.
select pg_sleep(500)
20 changes: 14 additions & 6 deletions gpMgmt/bin/gppylib/test/behave/mgmt_utils/steps/mgmt_utils.py
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
import commands
import fnmatch
import getpass
import glob
import gzip
import os
import platform
import shutil
import socket
import tarfile
import thread
import fnmatch
import os
import yaml

from collections import defaultdict
from datetime import datetime

import yaml

from gppylib.commands.gp import SegmentStart, GpStandbyStart
from gppylib.commands.unix import findCmdInPath
from gppylib.operations.backup_utils import Context
Expand Down Expand Up @@ -1070,6 +1071,8 @@ def verify_file_contents(context, file_type, file_dir, text_find, should_contain
fn = '%sgp_dump_%s_schema' % (context.dump_prefix, context.backup_timestamp)
elif file_type == 'cdatabase':
fn = '%sgp_cdatabase_1_1_%s' % (context.dump_prefix, context.backup_timestamp)
elif file_type == 'dump':
fn = '%sgp_dump_1_1_%s.gz' % (context.dump_prefix, context.backup_timestamp)

subdirectory = context.backup_timestamp[0:8]

Expand All @@ -1082,8 +1085,13 @@ def verify_file_contents(context, file_type, file_dir, text_find, should_contain
raise Exception ("Can not find %s file: %s" % (file_type, full_path))

contents = ""
with open(full_path) as fd:
contents = fd.read()

if file_type == 'dump':
fd = gzip.open(full_path)
else:
fd = open(full_path)
contents = fd.read()
fd.close()

if should_contain and not text_find in contents:
raise Exception("Did not find '%s' in file %s" % (text_find, full_path))
Expand Down

0 comments on commit 8d26668

Please sign in to comment.