Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 2432b1a

Browse files
committed
Avoid spamming the client with multiple ParameterStatus messages.
Up to now, we sent a ParameterStatus message to the client immediately upon any change in the active value of any GUC_REPORT variable. This was only barely okay when the feature was designed; now that we have things like function SET clauses, there are very plausible use-cases where a GUC_REPORT variable might change many times within a query --- and even end up back at its original value, perhaps. Fortunately most of our GUC_REPORT variables are unlikely to be changed often; but there are proposals in play to enlarge that set, or even make it user-configurable. Hence, let's fix things to not generate more than one ParameterStatus message per variable per query, and to not send any message at all unless the end-of-query value is different from what we last reported. Discussion: https://postgr.es/m/5708.1601145259@sss.pgh.pa.us
1 parent f739992 commit 2432b1a

File tree

4 files changed

+80
-7
lines changed

4 files changed

+80
-7
lines changed

src/backend/tcop/postgres.c

+3
Original file line numberDiff line numberDiff line change
@@ -4229,6 +4229,9 @@ PostgresMain(int argc, char *argv[],
42294229
pgstat_report_activity(STATE_IDLE, NULL);
42304230
}
42314231

4232+
/* Report any recently-changed GUC options */
4233+
ReportChangedGUCOptions();
4234+
42324235
ReadyForQuery(whereToSendOutput);
42334236
send_ready_for_query = false;
42344237
}

src/backend/utils/misc/guc.c

+72-6
Original file line numberDiff line numberDiff line change
@@ -4822,6 +4822,8 @@ static bool guc_dirty; /* true if need to do commit/abort work */
48224822

48234823
static bool reporting_enabled; /* true to enable GUC_REPORT */
48244824

4825+
static bool report_needed; /* true if any GUC_REPORT reports are needed */
4826+
48254827
static int GUCNestLevel = 0; /* 1 when in main transaction */
48264828

48274829

@@ -5452,6 +5454,7 @@ InitializeOneGUCOption(struct config_generic *gconf)
54525454
gconf->reset_scontext = PGC_INTERNAL;
54535455
gconf->stack = NULL;
54545456
gconf->extra = NULL;
5457+
gconf->last_reported = NULL;
54555458
gconf->sourcefile = NULL;
54565459
gconf->sourceline = 0;
54575460

@@ -5828,7 +5831,10 @@ ResetAllOptions(void)
58285831
gconf->scontext = gconf->reset_scontext;
58295832

58305833
if (gconf->flags & GUC_REPORT)
5831-
ReportGUCOption(gconf);
5834+
{
5835+
gconf->status |= GUC_NEEDS_REPORT;
5836+
report_needed = true;
5837+
}
58325838
}
58335839
}
58345840

@@ -6215,7 +6221,10 @@ AtEOXact_GUC(bool isCommit, int nestLevel)
62156221

62166222
/* Report new value if we changed it */
62176223
if (changed && (gconf->flags & GUC_REPORT))
6218-
ReportGUCOption(gconf);
6224+
{
6225+
gconf->status |= GUC_NEEDS_REPORT;
6226+
report_needed = true;
6227+
}
62196228
} /* end of stack-popping loop */
62206229

62216230
if (stack != NULL)
@@ -6257,26 +6266,80 @@ BeginReportingGUCOptions(void)
62576266
if (conf->flags & GUC_REPORT)
62586267
ReportGUCOption(conf);
62596268
}
6269+
6270+
report_needed = false;
6271+
}
6272+
6273+
/*
6274+
* ReportChangedGUCOptions: report recently-changed GUC_REPORT variables
6275+
*
6276+
* This is called just before we wait for a new client query.
6277+
*
6278+
* By handling things this way, we ensure that a ParameterStatus message
6279+
* is sent at most once per variable per query, even if the variable
6280+
* changed multiple times within the query. That's quite possible when
6281+
* using features such as function SET clauses. Function SET clauses
6282+
* also tend to cause values to change intraquery but eventually revert
6283+
* to their prevailing values; ReportGUCOption is responsible for avoiding
6284+
* redundant reports in such cases.
6285+
*/
6286+
void
6287+
ReportChangedGUCOptions(void)
6288+
{
6289+
/* Quick exit if not (yet) enabled */
6290+
if (!reporting_enabled)
6291+
return;
6292+
6293+
/* Quick exit if no values have been changed */
6294+
if (!report_needed)
6295+
return;
6296+
6297+
/* Transmit new values of interesting variables */
6298+
for (int i = 0; i < num_guc_variables; i++)
6299+
{
6300+
struct config_generic *conf = guc_variables[i];
6301+
6302+
if ((conf->flags & GUC_REPORT) && (conf->status & GUC_NEEDS_REPORT))
6303+
ReportGUCOption(conf);
6304+
}
6305+
6306+
report_needed = false;
62606307
}
62616308

62626309
/*
62636310
* ReportGUCOption: if appropriate, transmit option value to frontend
6311+
*
6312+
* We need not transmit the value if it's the same as what we last
6313+
* transmitted. However, clear the NEEDS_REPORT flag in any case.
62646314
*/
62656315
static void
62666316
ReportGUCOption(struct config_generic *record)
62676317
{
6268-
if (reporting_enabled && (record->flags & GUC_REPORT))
6318+
char *val = _ShowOption(record, false);
6319+
6320+
if (record->last_reported == NULL ||
6321+
strcmp(val, record->last_reported) != 0)
62696322
{
6270-
char *val = _ShowOption(record, false);
62716323
StringInfoData msgbuf;
62726324

62736325
pq_beginmessage(&msgbuf, 'S');
62746326
pq_sendstring(&msgbuf, record->name);
62756327
pq_sendstring(&msgbuf, val);
62766328
pq_endmessage(&msgbuf);
62776329

6278-
pfree(val);
6330+
/*
6331+
* We need a long-lifespan copy. If strdup() fails due to OOM, we'll
6332+
* set last_reported to NULL and thereby possibly make a duplicate
6333+
* report later.
6334+
*/
6335+
if (record->last_reported)
6336+
free(record->last_reported);
6337+
record->last_reported = strdup(val);
62796338
}
6339+
6340+
pfree(val);
6341+
6342+
record->status &= ~GUC_NEEDS_REPORT;
62806343
}
62816344

62826345
/*
@@ -7695,7 +7758,10 @@ set_config_option(const char *name, const char *value,
76957758
}
76967759

76977760
if (changeVal && (record->flags & GUC_REPORT))
7698-
ReportGUCOption(record);
7761+
{
7762+
record->status |= GUC_NEEDS_REPORT;
7763+
report_needed = true;
7764+
}
76997765

77007766
return changeVal ? 1 : -1;
77017767
}

src/include/utils/guc.h

+1
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,7 @@ extern void AtStart_GUC(void);
363363
extern int NewGUCNestLevel(void);
364364
extern void AtEOXact_GUC(bool isCommit, int nestLevel);
365365
extern void BeginReportingGUCOptions(void);
366+
extern void ReportChangedGUCOptions(void);
366367
extern void ParseLongOption(const char *string, char **name, char **value);
367368
extern bool parse_int(const char *value, int *result, int flags,
368369
const char **hintmsg);

src/include/utils/guc_tables.h

+4-1
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,8 @@ struct config_generic
161161
GucContext reset_scontext; /* context that set the reset value */
162162
GucStack *stack; /* stacked prior values */
163163
void *extra; /* "extra" pointer for current actual value */
164+
char *last_reported; /* if variable is GUC_REPORT, value last sent
165+
* to client (NULL if not yet sent) */
164166
char *sourcefile; /* file current setting is from (NULL if not
165167
* set in config file) */
166168
int sourceline; /* line in source file */
@@ -172,7 +174,8 @@ struct config_generic
172174
* Caution: the GUC_IS_IN_FILE bit is transient state for ProcessConfigFile.
173175
* Do not assume that its value represents useful information elsewhere.
174176
*/
175-
#define GUC_PENDING_RESTART 0x0002
177+
#define GUC_PENDING_RESTART 0x0002 /* changed value cannot be applied yet */
178+
#define GUC_NEEDS_REPORT 0x0004 /* new value must be reported to client */
176179

177180

178181
/* GUC records for specific variable types */

0 commit comments

Comments
 (0)