Skip to content

Commit 097f32a

Browse files
author
Rucha Kulkarni
committed
Adding bbf version of pgstat_init_function_usage
Signed-off-by: Rucha Kulkarni <ruchask@amazon.com>
1 parent cd314d6 commit 097f32a

3 files changed

Lines changed: 77 additions & 6 deletions

File tree

src/backend/utils/activity/pgstat_function.c

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,76 @@ pgstat_init_function_usage(FunctionCallInfo fcinfo,
138138
INSTR_TIME_SET_CURRENT(fcu->start);
139139
}
140140

141+
void
142+
pgstat_init_function_usage_no_drop(FunctionCallInfo fcinfo,
143+
PgStat_FunctionCallUsage *fcu)
144+
{
145+
PgStat_EntryRef *entry_ref;
146+
PgStat_FunctionCounts *pending;
147+
bool created_entry;
148+
149+
if (pgstat_track_functions <= fcinfo->flinfo->fn_stats)
150+
{
151+
/* stats not wanted */
152+
fcu->fs = NULL;
153+
return;
154+
}
155+
156+
entry_ref = pgstat_prep_pending_entry(PGSTAT_KIND_FUNCTION,
157+
MyDatabaseId,
158+
fcinfo->flinfo->fn_oid,
159+
&created_entry);
160+
161+
/*
162+
* If no shared entry already exists, check if the function has been
163+
* deleted concurrently. This can go unnoticed until here because
164+
* executing a statement that just calls a function, does not trigger
165+
* cache invalidation processing. The reason we care about this case is
166+
* that otherwise we could create a new stats entry for an already dropped
167+
* function (for relations etc this is not possible because emitting stats
168+
* requires a lock for the relation to already have been acquired).
169+
*
170+
* It's somewhat ugly to have a behavioral difference based on
171+
* track_functions being enabled/disabled. But it seems acceptable, given
172+
* that there's already behavioral differences depending on whether the
173+
* function is the caches etc.
174+
*
175+
* For correctness it'd be sufficient to set ->dropped to true. However,
176+
* the accepted invalidation will commonly cause "low level" failures in
177+
* PL code, with an OID in the error message. Making this harder to
178+
* test...
179+
*
180+
* Unlike pgstat_init_function_usage, we do NOT call
181+
* pgstat_drop_entry here. In Babelfish's concurrent scenarios,
182+
* calling pgstat_drop_entry conflicts with the transactional
183+
* drop path (AtEOXact_PgStat_DroppedStats), causing a server crash.
184+
* The entry will be cleaned up by the transactional path at commit time.
185+
*/
186+
if (created_entry)
187+
{
188+
AcceptInvalidationMessages();
189+
if (!SearchSysCacheExists1(PROCOID, ObjectIdGetDatum(fcinfo->flinfo->fn_oid)))
190+
{
191+
fcu->fs = NULL;
192+
ereport(ERROR, errcode(ERRCODE_UNDEFINED_FUNCTION),
193+
errmsg("function call to dropped function"));
194+
}
195+
}
196+
197+
pending = entry_ref->pending;
198+
199+
fcu->fs = pending;
200+
201+
/* save stats for this function, later used to compensate for recursion */
202+
fcu->save_f_total_time = pending->total_time;
203+
204+
/* save current backend-wide total time */
205+
fcu->save_total = total_func_time;
206+
207+
/* get clock time as of function start */
208+
INSTR_TIME_SET_CURRENT(fcu->start);
209+
}
210+
141211
/*
142212
* Calculate function call usage and update stat counters.
143213
* Called by the executor after invoking a function.

src/backend/utils/fmgr/fmgr.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -857,13 +857,11 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
857857
* and the dialect is TSQL then we call this func using hook
858858
* otherwise we will fall back to pgstat_init_function_usage
859859
*/
860-
861-
if(pgstat_function_wrapper_hook && sql_dialect == SQL_DIALECT_TSQL)
860+
if (!pgstat_function_wrapper_hook || sql_dialect != SQL_DIALECT_TSQL ||
861+
!(*pgstat_function_wrapper_hook)(fcinfo, &fcusage, cacheTupleProcname))
862862
{
863-
(*pgstat_function_wrapper_hook)(fcinfo, &fcusage, cacheTupleProcname);
863+
pgstat_init_function_usage(fcinfo, &fcusage);
864864
}
865-
866-
pgstat_init_function_usage(fcinfo, &fcusage);
867865

868866
if(cacheTupleProcname)
869867
{

src/include/pgstat.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -865,7 +865,7 @@ extern PGDLLEXPORT invalidate_stat_table_hook_type invalidate_stat_table_hook;
865865
typedef bool (*tsql_has_pgstat_permissions_hook_type) (Oid role);
866866
extern PGDLLEXPORT tsql_has_pgstat_permissions_hook_type tsql_has_pgstat_permissions_hook;
867867

868-
typedef void (*pgstat_function_wrapper_hook_type)(FunctionCallInfo, PgStat_FunctionCallUsage *, char *);
868+
typedef bool (*pgstat_function_wrapper_hook_type)(FunctionCallInfo, PgStat_FunctionCallUsage *, char *);
869869
extern PGDLLEXPORT pgstat_function_wrapper_hook_type pgstat_function_wrapper_hook;
870870

871871
typedef void (*pltsql_pgstat_end_function_usage_hook_type) (FunctionCallInfo fcinfo,
@@ -875,4 +875,7 @@ extern PGDLLEXPORT pltsql_pgstat_end_function_usage_hook_type pltsql_pgstat_end_
875875

876876
extern bool lookup_pgstat_entry_in_cache(PgStat_Kind kind, Oid dboid, Oid objoid);
877877

878+
extern void pgstat_init_function_usage_no_drop(FunctionCallInfo fcinfo,
879+
PgStat_FunctionCallUsage *fcu);
880+
878881
#endif /* PGSTAT_H */

0 commit comments

Comments
 (0)