nocfbot-0001-Make-pg_stash_advice-dump-advice-to-disk-wh.patch
application/octet-stream
Filename: nocfbot-0001-Make-pg_stash_advice-dump-advice-to-disk-wh.patch
Type: application/octet-stream
Part: 0
Message:
Re: pg_plan_advice
Patch
Same data as JSON:
GET /api/v1/attachments/:id/patch
the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes.
API reference →
Format: format-patch
Series: patch 0001
Subject: Make pg_stash_advice dump advice to disk when changed, load after restart
| File | + | − |
|---|---|---|
| contrib/pg_stash_advice/pg_stash_advice.c | 356 | 0 |
| doc/src/sgml/pgstashadvice.sgml | 6 | 4 |
From 6c02fc9318ef19c15b00ad5376b7717bfa92725f Mon Sep 17 00:00:00 2001
From: Lukas Fittl <lukas@fittl.com>
Date: Sun, 29 Mar 2026 11:47:27 -0700
Subject: [PATCH vnocfbot] Make pg_stash_advice dump advice to disk when
changed, load after restart
---
contrib/pg_stash_advice/pg_stash_advice.c | 356 ++++++++++++++++++++++
doc/src/sgml/pgstashadvice.sgml | 10 +-
2 files changed, 362 insertions(+), 4 deletions(-)
diff --git a/contrib/pg_stash_advice/pg_stash_advice.c b/contrib/pg_stash_advice/pg_stash_advice.c
index 22122236694..7dc52485617 100644
--- a/contrib/pg_stash_advice/pg_stash_advice.c
+++ b/contrib/pg_stash_advice/pg_stash_advice.c
@@ -18,6 +18,8 @@
*/
#include "postgres.h"
+#include <unistd.h>
+
#include "common/hashfn.h"
#include "common/string.h"
#include "fmgr.h"
@@ -26,12 +28,15 @@
#include "nodes/queryjumble.h"
#include "pg_plan_advice.h"
#include "storage/dsm_registry.h"
+#include "storage/fd.h"
#include "storage/lwlock.h"
#include "utils/builtins.h"
#include "utils/guc.h"
#include "utils/memutils.h"
#include "utils/tuplestore.h"
+#define PGSA_DUMP_FILE "pg_stash_advice.entries"
+
PG_MODULE_MAGIC;
PG_FUNCTION_INFO_V1(pg_create_advice_stash);
@@ -50,6 +55,7 @@ typedef struct pgsa_shared_state
dsa_handle area;
dshash_table_handle stash_hash;
dshash_table_handle entry_hash;
+ bool file_loaded;
} pgsa_shared_state;
typedef struct pgsa_stash
@@ -154,6 +160,10 @@ static void pgsa_init_shared_state(void *ptr, void *arg);
static uint64 pgsa_lookup_stash_id(char *stash_name);
static void pgsa_set_advice_string(char *stash_name, int64 queryId,
char *advice_string);
+static void pgsa_dump_to_file(void);
+static void pgsa_load_from_file(void);
+static char *pgsa_escape_string(char *str);
+static char *pgsa_unescape_string(char *str);
/*
* Initialize this module.
@@ -201,6 +211,7 @@ pg_create_advice_stash(PG_FUNCTION_ARGS)
LWLockAcquire(&pgsa_state->lock, LW_EXCLUSIVE);
pgsa_create_stash(stash_name);
LWLockRelease(&pgsa_state->lock);
+ pgsa_dump_to_file();
PG_RETURN_VOID();
}
@@ -218,6 +229,7 @@ pg_drop_advice_stash(PG_FUNCTION_ARGS)
LWLockAcquire(&pgsa_state->lock, LW_EXCLUSIVE);
pgsa_drop_stash(stash_name);
LWLockRelease(&pgsa_state->lock);
+ pgsa_dump_to_file();
PG_RETURN_VOID();
}
@@ -441,6 +453,7 @@ pg_set_stashed_advice(PG_FUNCTION_ARGS)
pgsa_set_advice_string(stash_name, queryId, advice_string);
}
+ pgsa_dump_to_file();
PG_RETURN_VOID();
}
@@ -614,6 +627,21 @@ pgsa_attach(void)
/* Restore previous memory context. */
MemoryContextSwitchTo(oldcontext);
+
+ /*
+ * If the shared state was just created (i.e. after a server restart or
+ * crash), try to restore stash data from the dump file. Use the
+ * file_loaded flag under the lock to ensure only one backend does this.
+ */
+ LWLockAcquire(&pgsa_state->lock, LW_EXCLUSIVE);
+ if (!pgsa_state->file_loaded)
+ {
+ pgsa_state->file_loaded = true;
+ LWLockRelease(&pgsa_state->lock);
+ pgsa_load_from_file();
+ }
+ else
+ LWLockRelease(&pgsa_state->lock);
}
/*
@@ -796,6 +824,7 @@ pgsa_init_shared_state(void *ptr, void *arg)
state->area = DSA_HANDLE_INVALID;
state->stash_hash = DSHASH_HANDLE_INVALID;
state->entry_hash = DSHASH_HANDLE_INVALID;
+ state->file_loaded = false;
}
/*
@@ -898,3 +927,330 @@ pgsa_set_advice_string(char *stash_name, int64 queryId, char *advice_string)
dsa_free(pgsa_dsa_area, old_dp);
LWLockRelease(&pgsa_state->lock);
}
+
+/*
+ * Dump all advice stash data to a file.
+ *
+ * The file format is a simple text format:
+ * Line 1: <<num_stashes>>
+ * Next num_stashes lines: one stash name per line
+ * Remaining lines: stash_name\tquery_id\tadvice_string
+ *
+ * Stash names and advice strings are backslash-escaped where needed.
+ */
+static void
+pgsa_dump_to_file(void)
+{
+ FILE *file;
+ char transient_dump_file_path[MAXPGPATH];
+ dshash_seq_status iter;
+ pgsa_stash *stash;
+ pgsa_entry *entry;
+ pgsa_stash_name_table_hash *nhash;
+ int num_stashes = 0;
+ int ret;
+
+ Assert(pgsa_entry_dshash != NULL);
+
+ /* Open a temporary file for writing. */
+ snprintf(transient_dump_file_path, MAXPGPATH, "%s.tmp", PGSA_DUMP_FILE);
+ file = AllocateFile(transient_dump_file_path, "w");
+ if (!file)
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not open file \"%s\": %m",
+ transient_dump_file_path)));
+
+ /*
+ * Build an ID->name lookup table. We also use this to count stashes (for
+ * the header) and to write stash names after the header.
+ */
+ nhash = pgsa_stash_name_table_create(CurrentMemoryContext, 64, NULL);
+ dshash_seq_init(&iter, pgsa_stash_dshash, true);
+ while ((stash = dshash_seq_next(&iter)) != NULL)
+ {
+ pgsa_stash_name *n;
+ bool found;
+
+ n = pgsa_stash_name_table_insert(nhash, stash->pgsa_stash_id, &found);
+ Assert(!found);
+ n->name = pstrdup(stash->name);
+ num_stashes++;
+ }
+ dshash_seq_term(&iter);
+
+ /* Write the header and stash names. */
+ ret = fprintf(file, "<<%d>>\n", num_stashes);
+ if (ret >= 0)
+ {
+ pgsa_stash_name_table_iterator i;
+
+ pgsa_stash_name_table_start_iterate(nhash, &i);
+ while (ret >= 0)
+ {
+ pgsa_stash_name *n = pgsa_stash_name_table_iterate(nhash, &i);
+
+ if (n == NULL)
+ break;
+ ret = fprintf(file, "%s\n", pgsa_escape_string(n->name));
+ }
+ }
+
+ /* Write entries: escaped_stash_name\tquery_id\tescaped_advice. */
+ if (ret >= 0)
+ {
+ dshash_seq_init(&iter, pgsa_entry_dshash, true);
+ while ((entry = dshash_seq_next(&iter)) != NULL)
+ {
+ pgsa_stash_name *n;
+ char *advice_string;
+
+ if (entry->advice_string == InvalidDsaPointer)
+ continue;
+
+ n = pgsa_stash_name_table_lookup(nhash, entry->key.pgsa_stash_id);
+ if (n == NULL)
+ continue; /* orphan entry, skip */
+
+ advice_string = dsa_get_address(pgsa_dsa_area,
+ entry->advice_string);
+ ret = fprintf(file, "%s\t%" PRId64 "\t%s\n",
+ pgsa_escape_string(n->name),
+ entry->key.queryId,
+ pgsa_escape_string(advice_string));
+ if (ret < 0)
+ break;
+ }
+ dshash_seq_term(&iter);
+ }
+
+ /* Handle any write error. */
+ if (ret < 0)
+ {
+ int save_errno = errno;
+
+ FreeFile(file);
+ unlink(transient_dump_file_path);
+ errno = save_errno;
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not write to file \"%s\": %m",
+ transient_dump_file_path)));
+ }
+
+ /* Close the file and rename it into place atomically. */
+ ret = FreeFile(file);
+ if (ret != 0)
+ {
+ int save_errno = errno;
+
+ unlink(transient_dump_file_path);
+ errno = save_errno;
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not close file \"%s\": %m",
+ transient_dump_file_path)));
+ }
+
+ (void) durable_rename(transient_dump_file_path, PGSA_DUMP_FILE, ERROR);
+}
+
+/*
+ * Load advice stash data from the dump file.
+ *
+ * This is called once when the shared memory state is first initialized
+ * (i.e. after a server restart or crash recovery), to restore the previously
+ * saved stash contents.
+ *
+ * Errors during loading are reported as warnings so that a corrupt dump file
+ * does not prevent the server from starting.
+ */
+static void
+pgsa_load_from_file(void)
+{
+ FILE *file;
+ int num_stashes;
+ int num_entries = 0;
+ int i;
+ StringInfoData buf;
+
+ file = AllocateFile(PGSA_DUMP_FILE, "r");
+ if (!file)
+ {
+ if (errno == ENOENT)
+ return; /* no dump file, nothing to load */
+ ereport(WARNING,
+ (errcode_for_file_access(),
+ errmsg("could not open file \"%s\": %m", PGSA_DUMP_FILE)));
+ return;
+ }
+
+ /* Read the header. */
+ if (fscanf(file, "<<%d>>\n", &num_stashes) != 1)
+ {
+ FreeFile(file);
+ ereport(WARNING,
+ errmsg("pg_stash_advice dump file has corrupted header"));
+ return;
+ }
+
+ initStringInfo(&buf);
+
+ /* Read and create stash names. */
+ for (i = 0; i < num_stashes; i++)
+ {
+ if (!pg_get_line_buf(file, &buf))
+ {
+ FreeFile(file);
+ pfree(buf.data);
+ ereport(WARNING,
+ errmsg("pg_stash_advice dump file is truncated at stash %d",
+ i + 1));
+ return;
+ }
+
+ /* Strip the trailing newline. */
+ if (buf.len > 0 && buf.data[buf.len - 1] == '\n')
+ buf.data[--buf.len] = '\0';
+ {
+ char *name = pgsa_unescape_string(buf.data);
+
+ /* Skip duplicates rather than ERRORing like pgsa_create_stash. */
+ if (pgsa_lookup_stash_id(name) == 0)
+ {
+ LWLockAcquire(&pgsa_state->lock, LW_EXCLUSIVE);
+ pgsa_create_stash(name);
+ LWLockRelease(&pgsa_state->lock);
+ }
+ }
+ }
+
+ /* Read and restore entries until EOF. */
+ while (pg_get_line_buf(file, &buf))
+ {
+ char *stash_name;
+ char *queryid_str;
+ char *advice_string;
+ int64 queryId;
+ ErrorSaveContext escontext = {T_ErrorSaveContext};
+
+ /* Strip the trailing newline. */
+ if (buf.len > 0 && buf.data[buf.len - 1] == '\n')
+ buf.data[--buf.len] = '\0';
+
+ /* Parse: stash_name\tquery_id\tadvice_string */
+ stash_name = buf.data;
+ queryid_str = strchr(stash_name, '\t');
+ if (queryid_str == NULL)
+ goto malformed;
+ *queryid_str++ = '\0';
+
+ advice_string = strchr(queryid_str, '\t');
+ if (advice_string == NULL)
+ goto malformed;
+ *advice_string++ = '\0';
+
+ queryId = pg_strtoint64_safe(queryid_str, (Node *) &escontext);
+ if (SOFT_ERROR_OCCURRED(&escontext))
+ goto malformed;
+
+ pgsa_set_advice_string(pgsa_unescape_string(stash_name),
+ queryId,
+ pgsa_unescape_string(advice_string));
+ num_entries++;
+ continue;
+
+malformed:
+ ereport(WARNING,
+ errmsg("pg_stash_advice dump file has malformed entry, skipping"));
+ }
+
+ FreeFile(file);
+ pfree(buf.data);
+
+ ereport(LOG,
+ errmsg("pg_stash_advice: loaded %d stashes with %d entries from \"%s\"",
+ num_stashes, num_entries, PGSA_DUMP_FILE));
+}
+
+/*
+ * Backslash-escape the string so it can be written to a tab-separated file.
+ *
+ * The escaped characters are backslash, tab, and newline.
+ */
+static char *
+pgsa_escape_string(char *str)
+{
+ StringInfoData buf;
+
+ if (!strpbrk(str, "\\\t\n"))
+ return str;
+
+ initStringInfo(&buf);
+ for (const char *p = str; *p; p++)
+ {
+ switch (*p)
+ {
+ case '\\':
+ appendStringInfoString(&buf, "\\\\");
+ break;
+ case '\t':
+ appendStringInfoString(&buf, "\\t");
+ break;
+ case '\n':
+ appendStringInfoString(&buf, "\\n");
+ break;
+ case '\r':
+ appendStringInfoString(&buf, "\\r");
+ break;
+ default:
+ appendStringInfoChar(&buf, *p);
+ break;
+ }
+ }
+
+ return buf.data;
+}
+
+/*
+ * Unescape a string that was escaped for serializing to the on-disk file.
+ */
+static char *
+pgsa_unescape_string(char *str)
+{
+ StringInfoData buf;
+
+ if (!strchr(str, '\\'))
+ return pstrdup(str);
+
+ initStringInfo(&buf);
+ for (const char *p = str; *p; p++)
+ {
+ if (*p == '\\' && p[1] != '\0')
+ {
+ p++;
+ switch (*p)
+ {
+ case '\\':
+ appendStringInfoChar(&buf, '\\');
+ break;
+ case 't':
+ appendStringInfoChar(&buf, '\t');
+ break;
+ case 'n':
+ appendStringInfoChar(&buf, '\n');
+ break;
+ case 'r':
+ appendStringInfoChar(&buf, '\r');
+ break;
+ default:
+ /* Unrecognized escape; keep as-is. */
+ appendStringInfoChar(&buf, *p);
+ break;
+ }
+ }
+ else
+ appendStringInfoChar(&buf, *p);
+ }
+ return buf.data;
+}
diff --git a/doc/src/sgml/pgstashadvice.sgml b/doc/src/sgml/pgstashadvice.sgml
index 089fc66446f..ce2c8ec3ab9 100644
--- a/doc/src/sgml/pgstashadvice.sgml
+++ b/doc/src/sgml/pgstashadvice.sgml
@@ -15,10 +15,12 @@
<link linkend="guc-compute-query-id">query identifiers</link> to plan advice
strings. Whenever a session is asked to plan a query whose query ID appears
in the relevant advice stash, the plan advice string is automatically applied
- to guide planning. Note that advice stashes exist purely in memory. This
- means both that it is important to be mindful of memory consumption when
- deciding how much plan advice to stash, and also that advice stashes must
- be recreated and repopulated whenever the server is restarted.
+ to guide planning. Advice stashes are held in memory, so it is important
+ to be mindful of memory consumption when deciding how much plan advice to
+ stash. The contents are automatically saved to a file called
+ <filename>pg_stash_advice.entries</filename> whenever they are modified,
+ and restored when the first session attaches after a server restart
+ (including after a crash).
</para>
<para>
--
2.47.1