Skip to content

Commit

Permalink
Get pg_database tuple from disk in vac_update_datfrozenxid.
Browse files Browse the repository at this point in the history
In vac_update_datfrozenxid.(), the pg_database tuple for current
database should be fetched from disk heap table instead of system cache.
Since the cache already flatten toast tuple, so if the tuple in
pg_database contains toast attribute, heap_inplace_update() will fail
with "wrong tuple length".

Postgres upstream also has this issue. But since the issue is hot,
let's fix in GPDB first.

(cherry picked from commit 373e676)
  • Loading branch information
JunfengYang committed Nov 17, 2020
1 parent 48b1327 commit cf619ba
Show file tree
Hide file tree
Showing 3 changed files with 155 additions and 11 deletions.
63 changes: 52 additions & 11 deletions src/backend/commands/vacuum.c
Expand Up @@ -1916,6 +1916,36 @@ vac_update_relstats(Oid relid, BlockNumber num_pages, double num_tuples,
}


/*
* fetch_database_tuple - Fetch a copy of database tuple from pg_database.
*
* This using disk heap table instead of system cache.
* relation: opened pg_database relation in vac_update_datfrozenxid().
*/
static HeapTuple
fetch_database_tuple(Relation relation, Oid dbOid)
{
ScanKeyData skey[1];
SysScanDesc sscan;
HeapTuple tuple = NULL;

ScanKeyInit(&skey[0],
ObjectIdAttributeNumber,
BTEqualStrategyNumber, F_OIDEQ,
ObjectIdGetDatum(dbOid));

sscan = systable_beginscan(relation, DatabaseOidIndexId, true,
SnapshotNow, 1, skey);

tuple = systable_getnext(sscan);
if (HeapTupleIsValid(tuple))
tuple = heap_copytuple(tuple);

systable_endscan(sscan);

return tuple;
}

/*
* vac_update_datfrozenxid() -- update pg_database.datfrozenxid for our DB
*
Expand All @@ -1934,8 +1964,8 @@ vac_update_relstats(Oid relid, BlockNumber num_pages, double num_tuples,
void
vac_update_datfrozenxid(void)
{
HeapTuple tuple;
Form_pg_database dbform;
HeapTuple cached_tuple;
Form_pg_database cached_dbform;
Relation relation;
SysScanDesc scan;
HeapTuple classTup;
Expand Down Expand Up @@ -1999,30 +2029,41 @@ vac_update_datfrozenxid(void)
relation = heap_open(DatabaseRelationId, RowExclusiveLock);

/* Fetch a copy of the tuple to scribble on */
tuple = SearchSysCacheCopy(DATABASEOID,
ObjectIdGetDatum(MyDatabaseId),
0, 0, 0);
if (!HeapTupleIsValid(tuple))
elog(ERROR, "could not find tuple for database %u", MyDatabaseId);
dbform = (Form_pg_database) GETSTRUCT(tuple);
cached_tuple = SearchSysCache1(DATABASEOID, ObjectIdGetDatum(MyDatabaseId));
cached_dbform = (Form_pg_database) GETSTRUCT(cached_tuple);

/*
* Don't allow datfrozenxid to go backward (probably can't happen anyway);
* and detect the common case where it doesn't go forward either.
*/
if (TransactionIdPrecedes(dbform->datfrozenxid, newFrozenXid))
if (TransactionIdPrecedes(cached_dbform->datfrozenxid, newFrozenXid))
{
dbform->datfrozenxid = newFrozenXid;
dirty = true;
}

if (dirty)
{
HeapTuple tuple;
Form_pg_database tmp_dbform;
/*
* Fetch a copy of the tuple to scribble on from pg_database disk
* heap table instead of system cache
* "SearchSysCacheCopy1(DATABASEOID, ObjectIdGetDatum(MyDatabaseId))".
* Since the cache already flatten toast tuple, so the
* heap_inplace_update will fail with "wrong tuple length".
*/
tuple = fetch_database_tuple(relation, MyDatabaseId);
if (!HeapTupleIsValid(tuple))
elog(ERROR, "could not find tuple for database %u", MyDatabaseId);
tmp_dbform = (Form_pg_database) GETSTRUCT(tuple);
tmp_dbform->datfrozenxid = newFrozenXid;

heap_inplace_update(relation, tuple);
heap_freetuple(tuple);
SIMPLE_FAULT_INJECTOR(VacuumUpdateDatFrozenXid);
}

heap_freetuple(tuple);
ReleaseSysCache(cached_tuple);
heap_close(relation, RowExclusiveLock);

/*
Expand Down
56 changes: 56 additions & 0 deletions src/test/regress/expected/vacuum_gp.out
Expand Up @@ -320,3 +320,59 @@ SELECT reltuples FROM pg_catalog.pg_class WHERE relname = 'pt';
12
(1 row)

-- Vacuum freeze for database with toast attribute in pg_database tuple cause
-- heap_inplace_update raise error "wrong tuple length". This is because system
-- cache flatten toast tuple.
DROP DATABASE IF EXISTS vacuum_freeze_test;
CREATE DATABASE vacuum_freeze_test;
-- start_ignore
create or replace function toast_pg_database_datacl() returns text as $body$
declare
mycounter int;
begin
for mycounter in select i from generate_series(1, 2800) i loop
execute 'create role aaaabbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb' || mycounter;
execute 'grant ALL on database vacuum_freeze_test to aaaabbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb' || mycounter;
end loop;
return 'ok';
end;
$body$ language plpgsql volatile strict;

create or replace function clean_roles() returns text as $body$
declare
mycounter int;
begin
for mycounter in select i from generate_series(1, 2800) i loop
execute 'drop role aaaabbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb' || mycounter;
end loop;
return 'ok';
end;
$body$ language plpgsql volatile strict;

select toast_pg_database_datacl();
-- end_ignore
\c vacuum_freeze_test
create temp table before_vacuum as select datname, pg_column_size(datacl) > 8192 as datacl_size, age(datfrozenxid) from pg_database where datname='vacuum_freeze_test';
select datname, datacl_size from before_vacuum;
datname | datacl_size
--------------------+-------------
vacuum_freeze_test | t
(1 row)

vacuum freeze;
select datname, pg_column_size(datacl) > 8192 as datacl_size, age(datfrozenxid) != (select age from before_vacuum) as age_changed from pg_database where datname='vacuum_freeze_test';
datname | datacl_size | age_changed
--------------------+-------------+-------------
vacuum_freeze_test | t | t
(1 row)

\c regression
DROP DATABASE vacuum_freeze_test;
-- start_ignore
select clean_roles();
drop function toast_pg_database_datacl();
drop function clean_roles();
-- end_ignore
-- free pg_global space, otherwise it fails db_size_functions
VACUUM FULL pg_authid;
VACUUM FULL pg_database;
47 changes: 47 additions & 0 deletions src/test/regress/sql/vacuum_gp.sql
Expand Up @@ -207,3 +207,50 @@ VACUUM pt;
SELECT reltuples FROM pg_catalog.pg_class WHERE relname = 'pt';
VACUUM ANALYZE pt;
SELECT reltuples FROM pg_catalog.pg_class WHERE relname = 'pt';

-- Vacuum freeze for database with toast attribute in pg_database tuple cause
-- heap_inplace_update raise error "wrong tuple length". This is because system
-- cache flatten toast tuple.
DROP DATABASE IF EXISTS vacuum_freeze_test;
CREATE DATABASE vacuum_freeze_test;
-- start_ignore
create or replace function toast_pg_database_datacl() returns text as $body$
declare
mycounter int;
begin
for mycounter in select i from generate_series(1, 2800) i loop
execute 'create role aaaabbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb' || mycounter;
execute 'grant ALL on database vacuum_freeze_test to aaaabbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb' || mycounter;
end loop;
return 'ok';
end;
$body$ language plpgsql volatile strict;

create or replace function clean_roles() returns text as $body$
declare
mycounter int;
begin
for mycounter in select i from generate_series(1, 2800) i loop
execute 'drop role aaaabbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb' || mycounter;
end loop;
return 'ok';
end;
$body$ language plpgsql volatile strict;

select toast_pg_database_datacl();
-- end_ignore
\c vacuum_freeze_test
create temp table before_vacuum as select datname, pg_column_size(datacl) > 8192 as datacl_size, age(datfrozenxid) from pg_database where datname='vacuum_freeze_test';
select datname, datacl_size from before_vacuum;
vacuum freeze;
select datname, pg_column_size(datacl) > 8192 as datacl_size, age(datfrozenxid) != (select age from before_vacuum) as age_changed from pg_database where datname='vacuum_freeze_test';
\c regression
DROP DATABASE vacuum_freeze_test;
-- start_ignore
select clean_roles();
drop function toast_pg_database_datacl();
drop function clean_roles();
-- end_ignore
-- free pg_global space, otherwise it fails db_size_functions
VACUUM FULL pg_authid;
VACUUM FULL pg_database;

0 comments on commit cf619ba

Please sign in to comment.