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
Appearance settings

Commit 2432b1a

Browse filesBrowse files
tglsfdctglsfdc
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
Copy full SHA for 2432b1a

File tree

Expand file treeCollapse file tree

4 files changed

+80
-7
lines changed
Filter options
Expand file treeCollapse file tree

4 files changed

+80
-7
lines changed

‎src/backend/tcop/postgres.c

Copy file name to clipboardExpand all lines: src/backend/tcop/postgres.c
+3Lines changed: 3 additions & 0 deletions
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

Copy file name to clipboardExpand all lines: src/backend/utils/misc/guc.c
+72-6Lines changed: 72 additions & 6 deletions
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

Copy file name to clipboardExpand all lines: src/include/utils/guc.h
+1Lines changed: 1 addition & 0 deletions
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

Copy file name to clipboardExpand all lines: src/include/utils/guc_tables.h
+4-1Lines changed: 4 additions & 1 deletion
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.