Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit5d00b76

Browse files
committed
Make pgstat tabstat lookup hash table less fragile.
Code review for commit090010f.Fix cases where an elog(ERROR) partway through a function would leave thepersistent data structures in a corrupt state. pgstat_report_stat got thiswrong by invalidating PgStat_TableEntry structs before removing hashtableentries pointing to them, and get_tabstat_entry got it wrong by ignoringthe possibility of palloc failure after it had already created a hashtableentry.Also, avoid leaking a memory context per transaction, which the previouscode did through misunderstanding hash_create's API. We do not need tocreate a context to hold the hash table; hash_create will do that.(The leak wasn't that large, amounting to only a memory context headerper iteration, but it's still surprising that nobody noticed it yet.)
1 parentb91e5b4 commit5d00b76

File tree

1 file changed

+64
-60
lines changed

1 file changed

+64
-60
lines changed

‎src/backend/postmaster/pgstat.c

Lines changed: 64 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ typedef struct TabStatusArray
174174
staticTabStatusArray*pgStatTabList=NULL;
175175

176176
/*
177-
* pgStatTabHash entry
177+
* pgStatTabHash entry: map from relation OID to PgStat_TableStatus pointer
178178
*/
179179
typedefstructTabStatHashEntry
180180
{
@@ -805,6 +805,17 @@ pgstat_report_stat(bool force)
805805
return;
806806
last_report=now;
807807

808+
/*
809+
* Destroy pgStatTabHash before we start invalidating PgStat_TableEntry
810+
* entries it points to. (Should we fail partway through the loop below,
811+
* it's okay to have removed the hashtable already --- the only
812+
* consequence is we'd get multiple entries for the same table in the
813+
* pgStatTabList, and that's safe.)
814+
*/
815+
if (pgStatTabHash)
816+
hash_destroy(pgStatTabHash);
817+
pgStatTabHash=NULL;
818+
808819
/*
809820
* Scan through the TabStatusArray struct(s) to find tables that actually
810821
* have counts, and build messages to send. We have to separate shared
@@ -855,14 +866,6 @@ pgstat_report_stat(bool force)
855866
tsa->tsa_used=0;
856867
}
857868

858-
/*
859-
* pgStatTabHash is outdated on this point so we have to clean it,
860-
* hash_destroy() will remove hash memory context, allocated in
861-
* make_sure_stat_tab_initialized()
862-
*/
863-
hash_destroy(pgStatTabHash);
864-
pgStatTabHash=NULL;
865-
866869
/*
867870
* Send partial messages. Make sure that any pending xact commit/abort
868871
* gets counted, even if there are no table stats to send.
@@ -1707,41 +1710,6 @@ pgstat_initstats(Relation rel)
17071710
rel->pgstat_info=get_tabstat_entry(rel_id,rel->rd_rel->relisshared);
17081711
}
17091712

1710-
/*
1711-
* Make sure pgStatTabList and pgStatTabHash are initialized.
1712-
*/
1713-
staticvoid
1714-
make_sure_stat_tab_initialized()
1715-
{
1716-
HASHCTLctl;
1717-
MemoryContextnew_ctx;
1718-
1719-
if(!pgStatTabList)
1720-
{
1721-
/* This is first time procedure is called */
1722-
pgStatTabList= (TabStatusArray*)MemoryContextAllocZero(TopMemoryContext,
1723-
sizeof(TabStatusArray));
1724-
}
1725-
1726-
if(pgStatTabHash)
1727-
return;
1728-
1729-
/* Hash table was freed or never existed. */
1730-
1731-
new_ctx=AllocSetContextCreate(
1732-
TopMemoryContext,
1733-
"PGStatLookupHashTableContext",
1734-
ALLOCSET_DEFAULT_SIZES);
1735-
1736-
memset(&ctl,0,sizeof(ctl));
1737-
ctl.keysize=sizeof(Oid);
1738-
ctl.entrysize=sizeof(TabStatHashEntry);
1739-
ctl.hcxt=new_ctx;
1740-
1741-
pgStatTabHash=hash_create("pgstat t_id to tsa_entry lookup hash table",
1742-
TABSTAT_QUANTUM,&ctl,HASH_ELEM |HASH_BLOBS |HASH_CONTEXT);
1743-
}
1744-
17451713
/*
17461714
* get_tabstat_entry - find or create a PgStat_TableStatus entry for rel
17471715
*/
@@ -1753,65 +1721,101 @@ get_tabstat_entry(Oid rel_id, bool isshared)
17531721
TabStatusArray*tsa;
17541722
boolfound;
17551723

1756-
make_sure_stat_tab_initialized();
1724+
/*
1725+
* Create hash table if we don't have it already.
1726+
*/
1727+
if (pgStatTabHash==NULL)
1728+
{
1729+
HASHCTLctl;
1730+
1731+
memset(&ctl,0,sizeof(ctl));
1732+
ctl.keysize=sizeof(Oid);
1733+
ctl.entrysize=sizeof(TabStatHashEntry);
1734+
1735+
pgStatTabHash=hash_create("pgstat TabStatusArray lookup hash table",
1736+
TABSTAT_QUANTUM,
1737+
&ctl,
1738+
HASH_ELEM |HASH_BLOBS);
1739+
}
17571740

17581741
/*
17591742
* Find an entry or create a new one.
17601743
*/
17611744
hash_entry=hash_search(pgStatTabHash,&rel_id,HASH_ENTER,&found);
1762-
if(found)
1745+
if (!found)
1746+
{
1747+
/* initialize new entry with null pointer */
1748+
hash_entry->tsa_entry=NULL;
1749+
}
1750+
1751+
/*
1752+
* If entry is already valid, we're done.
1753+
*/
1754+
if (hash_entry->tsa_entry)
17631755
returnhash_entry->tsa_entry;
17641756

17651757
/*
1766-
* `hash_entry` was just created and now we have to fill it.
1767-
* First make sure there is a free space in a last element of pgStatTabList.
1758+
* Locate the first pgStatTabList entry with free space, making a new list
1759+
* entry if needed. Note that we could get an OOM failure here, but if so
1760+
* we have left the hashtable and the list in a consistent state.
17681761
*/
1769-
tsa=pgStatTabList;
1770-
while(tsa->tsa_used==TABSTAT_QUANTUM)
1762+
if (pgStatTabList==NULL)
17711763
{
1772-
if(tsa->tsa_next==NULL)
1773-
{
1774-
tsa->tsa_next= (TabStatusArray*)MemoryContextAllocZero(TopMemoryContext,
1775-
sizeof(TabStatusArray));
1776-
}
1764+
/* Set up first pgStatTabList entry */
1765+
pgStatTabList= (TabStatusArray*)
1766+
MemoryContextAllocZero(TopMemoryContext,
1767+
sizeof(TabStatusArray));
1768+
}
17771769

1770+
tsa=pgStatTabList;
1771+
while (tsa->tsa_used >=TABSTAT_QUANTUM)
1772+
{
1773+
if (tsa->tsa_next==NULL)
1774+
tsa->tsa_next= (TabStatusArray*)
1775+
MemoryContextAllocZero(TopMemoryContext,
1776+
sizeof(TabStatusArray));
17781777
tsa=tsa->tsa_next;
17791778
}
17801779

17811780
/*
1782-
* Add an entry.
1781+
* Allocate a PgStat_TableStatus entry within this list entry. We assume
1782+
* the entry was already zeroed, either at creation or after last use.
17831783
*/
17841784
entry=&tsa->tsa_entries[tsa->tsa_used++];
17851785
entry->t_id=rel_id;
17861786
entry->t_shared=isshared;
17871787

17881788
/*
1789-
*Add a correspondingentryto pgStatTabHash.
1789+
*Now we can fill theentryin pgStatTabHash.
17901790
*/
17911791
hash_entry->tsa_entry=entry;
1792+
17921793
returnentry;
17931794
}
17941795

17951796
/*
17961797
* find_tabstat_entry - find any existing PgStat_TableStatus entry for rel
17971798
*
17981799
* If no entry, return NULL, don't create a new one
1800+
*
1801+
* Note: if we got an error in the most recent execution of pgstat_report_stat,
1802+
* it's possible that an entry exists but there's no hashtable entry for it.
1803+
* That's okay, we'll treat this case as "doesn't exist".
17991804
*/
18001805
PgStat_TableStatus*
18011806
find_tabstat_entry(Oidrel_id)
18021807
{
18031808
TabStatHashEntry*hash_entry;
18041809

1805-
/*
1806-
* There are no entries at all.
1807-
*/
1810+
/* If hashtable doesn't exist, there are no entries at all */
18081811
if(!pgStatTabHash)
18091812
returnNULL;
18101813

18111814
hash_entry=hash_search(pgStatTabHash,&rel_id,HASH_FIND,NULL);
18121815
if(!hash_entry)
18131816
returnNULL;
18141817

1818+
/* Note that this step could also return NULL, but that's correct */
18151819
returnhash_entry->tsa_entry;
18161820
}
18171821

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp