public inbox for [email protected]
help / color / mirror / Atom feedFrom: Lukas Fittl <[email protected]>
To: Robert Haas <[email protected]>
Cc: Jakub Wartak <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: pg_plan_advice
Date: Tue, 31 Mar 2026 23:33:50 -0700
Message-ID: <CAP53Pkyq-dVyJRxqui-fN8P0Qv5=oJXZoOyWn0pL=N5Rqi1HRw@mail.gmail.com> (raw)
In-Reply-To: <CAP53PkxvjckT=1CokN7-PMcthr0OvWNMrAXTH7dStaXrKi6q2A@mail.gmail.com>
References: <CA+TgmoZ-Jh1T6QyWoCODMVQdhTUPYkaZjWztzP1En4=ZHoKPzw@mail.gmail.com>
<CAKZiRmxtJAFG7e1+Vs9B8ngON=AOzJbuws+1ZeH4LsbJh5AzoQ@mail.gmail.com>
<CA+TgmoY9Ne_Sh10u6LSPc3wvOQPLp3kF9nBp3nqJEG2JGF2QiA@mail.gmail.com>
<CA+Tgmoa57S6mP=aTOXH2-gDAL4TMO1WbGgrHSg0s6J4zUH=04g@mail.gmail.com>
<[email protected]>
<CA+Tgmoaf__2B0BUL+vrg28P+3buX=Ti-kybqkHiLTtFrrCfzuA@mail.gmail.com>
<CA+TgmoYpcLNOuypOTdgCSLW7FuA=t6BtB3meTARHX2-Dj_81xQ@mail.gmail.com>
<[email protected]>
<CA+TgmoZjv9OyFu1Gkt78w0vWEti8S33w8trYHmErf-GMmGSi=w@mail.gmail.com>
<[email protected]>
<CA+TgmoaOSBQD9Ux4eG40w723ZN=c0J7p-+oX4+J8urUeyLMo5w@mail.gmail.com>
<CAOYmi+=g+MMoOpWkk2weXWKJcKH0eKey8gKHHdH0dF4Tiawrhw@mail.gmail.com>
<CA+TgmobwaT=PXPDDrgDup+jA8KHBbkxigtziD-zNzAKKkQYVgQ@mail.gmail.com>
<CAOYmi+mOmEW=amDRQMfw6-Fb3ZmDEQFaJzwk8Bc8W8DzaP85XQ@mail.gmail.com>
<CA+TgmoaX2AMW4cdFM3OngBJxmxpkdmzF33R7-CWhvRLfucbFMg@mail.gmail.com>
<CA+TgmoZ0x3ym_oueXRWzbM_=6ucKoPZVGj3rRMLBDC_FnetXDw@mail.gmail.com>
<CAP53Pkycc=7N2bLzVT3x+qE1JamvRZWev5tFjdLJ1+-AV3Di+Q@mail.gmail.com>
<CA+TgmoaKhuD91RnazbRyGkmP7--JdNq8oNDC3UcgTZSWbMxC7w@mail.gmail.com>
<CAP53Pkw5-wMEeDJXFmqo_RTyL_spzCXb7HHKrbSnQqokVoZcNQ@mail.gmail.com>
<CA+Tgmob-69bzbdi3U_QtebqAf6u1y8js=5=oNK639csVe1VbhA@mail.gmail.com>
<CA+TgmoaZMOikxK=LqS+Jn+835h9S139JLGk-3LyETVXw5W5j=w@mail.gmail.com>
<[email protected]>
<CAP53PkwZ1ZTMARKg6iEfAw9qzBhkjBitj-9gr_Jvy7k2AwGgWA@mail.gmail.com>
<CAMbWs4--NuEUFE_xTo991TRXaZryE29jarJPDnVxoaQOYdt7tA@mail.gmail.com>
<CA+TgmobzR+XMGbRosVPbjHbSo4+cgJn=qZK6w05aF1sbj=C+9Q@mail.gmail.com>
<CA+TgmoawzvCoZAwFS85tE5+c8vBkqgcS8ZstQ_ohjXQ9wGT9sw@mail.gmail.com>
<CA+TgmoYS4ZCVAF2jTce=bMP0Oq_db_srocR4cZyO0OBp9oUoGg@mail.gmail.com>
<CAK98qZ2RzbgCHrSg4zLkvpzyBam_X6te-KF8w1+_vON9BAVMEw@mail.gmail.com>
<CA+TgmoaCdsuvNn6T6SfQ_0YD2Hh2+hgTXh9fTGHQhPg1zvy2rQ@mail.gmail.com>
<CA+Tgmob7ozJAs5SU6bD2RfAt4w_AmsMGz-aaVA6WeLXHkBypOg@mail.gmail.com>
<CAK98qZ1J42RoAsHnYWGPPmHziZmzmqE=Lp_O6WJ-9aKK2qjikA@mail.gmail.com>
<CA+TgmoYjcBA6dw3nwiyfDzPXTCrxTZPXDMrc2TrDJcL1cPK6iA@mail.gmail.com>
<CA+TgmoYru-vxoTKfwjQby30r2OkTXfb18Km_=VLs6qk8Akr0-g@mail.gmail.com>
<CA+Tgmoau7yJtvbeH-0kPt1Q=Gt_ezRdgM35Q1=LT665U_86Etg@mail.gmail.com>
<[email protected]>
<CA+TgmobOLrMn5jEinWNPL5SrDH1DPpo3a4j+S6-4yhsZwWgzLg@mail.gmail.com>
<CA+TgmoZUN8FT1Ah=m6Uis5bHa4FUa+_hMDWtcABG17toEfpiUg@mail.gmail.com>
<CA+TgmoYh2-kM+tscOz=jVYq9Tf4SRPVqzPojs3KLZcW6E9m1BQ@mail.gmail.com>
<CA+TgmoaK=4w7-qknUo3QhUJ53pXZq=c=KgZmRyD+k7ytqfmgSg@mail.gmail.com>
<CAP53Pkz3DSFaaowYvbO5LULf3NhydD_UhHkighfWf6_pwxiqUw@mail.gmail.com>
<CA+TgmoZ45n5jaNKKgbbj4-kYV8WsPvUn=Z8HnoZ7tUb_p9WKXg@mail.gmail.com>
<CA+TgmoYuWmN-00Ec5pY7zAcpSFQUQLbgAdVWGR9kOR-HM-fHrA@mail.gmail.com>
<CAKZiRmwFKhVz-HWvQmgPY7nZES9mCqdXHD++m9b7F4digcJpRQ@mail.gmail.com>
<CAP53Pkxak127z_Xp7xW5G0+X4FELCBzNyoWCqTQ-xGfLog3KyQ@mail.gmail.com>
<CA+Tgmoandcm1p=BCwLZPGRG_cno3pKSj4Ejt8AQZ4Hu-yhP1Mw@mail.gmail.com>
<CAP53PkxJp=9PMzD8mKB3=4fK3Qp1iFZnA4pQSavP0nkpiNyoaA@mail.gmail.com>
<CA+TgmobnwuyTgYXweQrvOz_kFuBuCbKhsLz-mUPasFw_DQ8f6w@mail.gmail.com>
<CAP53PkxvjckT=1CokN7-PMcthr0OvWNMrAXTH7dStaXrKi6q2A@mail.gmail.com>
On Tue, Mar 31, 2026 at 7:25 PM Lukas Fittl <[email protected]> wrote:
>
> On Mon, Mar 30, 2026 at 7:53 AM Robert Haas <[email protected]> wrote:
> >
> >
> > The module doesn't have a built-in way to do that right now. Are you
> > thinking we would document that pg_get_dsm_registry_allocations() can
> > be used?
>
> Yeah, for example. Alternatively we could provide a function/view that
> lists all advice across all stashes, so you can more easily see the
> result size of that and estimate what the in-memory use is. But
> pointing to pg_get_dsm_registry_allocations seems easier.
Actually, that won't work in practice with the code as of v23 -
pg_get_dsm_registry_allocations() always returns the fixed 64 byte
allocation from GetNamedDSMSegment, but is oblivious to the individual
DSA allocations (even after adding hundreds of entries):
SELECT * FROM pg_get_dsm_registry_allocations();
name | type | size
-----------------+---------+------
pg_stash_advice | segment | 64
(1 row)
Is there a reason you didn't use GetNamedDSA / GetNamedDSHash for the
other allocations? (which we have as of fe07100e82b09)
With the adjustments in the attached patch, this gets reported as expected:
SELECT * FROM pg_get_dsm_registry_allocations();
name | type | size
-----------------------+---------+-----------
pg_stash_advice | segment | 24
pg_stash_advice_stash | hash | 1048576
pg_stash_advice_dsa | area | 803209216
pg_stash_advice_entry | hash | 1048576
(4 rows)
>
> > > In practice for a good amount of our user base these days the question
> > > will be "Does my cloud provider give me access to create stash
> > > entries", so its maybe worth thinking about if we could also allow
> > > pg_maintain to manage entries by default?
> >
> > Wouldn't it make more sense for the cloud provider to grant execute
> > permissions on these functions to pg_maintain if they are so inclined?
> > This is a brand-new facility, so I think we had better be conservative
> > in terms of default permissions.
>
> I guess. I'm always worried that providers get that wrong and forget
> to give end users the permissions - but I suppose end users can
> complain to their providers if that's the case.
>
> I've done another look over pg_set_stashed_advice and I think its in
> good shape. The only trailing thought I have is that we could consider
> running a fuzzer against the pg_set_advice function in particular,
> just to see if anything pops up (beyond having the ability to make a
> very large memory allocation through a large advice string, which is
> maybe a problem?).
Obviously I meant "I've done another look over pg_stash_advice and I
think its in good shape".
I've done some basic fuzzing with the pg_set_stashed_advice function,
including concurrently setting advice, and that didn't yield any
surprises.
Thanks,
Lukas
--
Lukas Fittl
Attachments:
[application/octet-stream] nocfbot-3-0001-Use-GetNamedDSA-GetNamedDSHash-for-shared.patch (5.7K, 2-nocfbot-3-0001-Use-GetNamedDSA-GetNamedDSHash-for-shared.patch)
download | inline diff:
From a9de0367fc504de403b93be179f36615a3884fc7 Mon Sep 17 00:00:00 2001
From: Lukas Fittl <[email protected]>
Date: Tue, 31 Mar 2026 23:30:11 -0700
Subject: [PATCH vnocfbot-3] Use GetNamedDSA / GetNamedDSHash for shared memory
allocations
This allows utilizing pg_get_dsm_registry_allocations to see the amount
of memory used by the internal storage.
---
contrib/pg_stash_advice/pg_stash_advice.c | 108 +++-------------------
1 file changed, 11 insertions(+), 97 deletions(-)
diff --git a/contrib/pg_stash_advice/pg_stash_advice.c b/contrib/pg_stash_advice/pg_stash_advice.c
index 22122236694..c74d0eebf45 100644
--- a/contrib/pg_stash_advice/pg_stash_advice.c
+++ b/contrib/pg_stash_advice/pg_stash_advice.c
@@ -43,13 +43,7 @@ PG_FUNCTION_INFO_V1(pg_set_stashed_advice);
typedef struct pgsa_shared_state
{
LWLock lock;
- int dsa_tranche;
- int stash_tranche;
- int entry_tranche;
uint64 next_stash_id;
- dsa_handle area;
- dshash_table_handle stash_hash;
- dshash_table_handle entry_hash;
} pgsa_shared_state;
typedef struct pgsa_stash
@@ -113,29 +107,27 @@ static dshash_table *pgsa_stash_dshash;
static dshash_table *pgsa_entry_dshash;
/* Shared memory hash table parameters */
-static dshash_parameters pgsa_stash_dshash_parameters = {
+static const dshash_parameters pgsa_stash_dshash_parameters = {
NAMEDATALEN,
sizeof(pgsa_stash),
dshash_strcmp,
dshash_strhash,
dshash_strcpy,
- LWTRANCHE_INVALID /* gets set at runtime */
+ LWTRANCHE_INVALID /* assigned by GetNamedDSHash */
};
-static dshash_parameters pgsa_entry_dshash_parameters = {
+static const dshash_parameters pgsa_entry_dshash_parameters = {
sizeof(pgsa_entry_key),
sizeof(pgsa_entry),
dshash_memcmp,
dshash_memhash,
dshash_memcpy,
- LWTRANCHE_INVALID /* gets set at runtime */
+ LWTRANCHE_INVALID /* assigned by GetNamedDSHash */
};
/* GUC variable */
static char *pg_stash_advice_stash_name = "";
-/* Other global variables */
-static MemoryContext pg_stash_advice_mcxt;
/* Function prototypes */
static char *pgsa_advisor(PlannerGlobal *glob,
@@ -519,17 +511,6 @@ static void
pgsa_attach(void)
{
bool found;
- MemoryContext oldcontext;
-
- /*
- * Create a memory context to make sure that any control structures
- * allocated in local memory are sufficiently persistent.
- */
- if (pg_stash_advice_mcxt == NULL)
- pg_stash_advice_mcxt = AllocSetContextCreate(TopMemoryContext,
- "pg_stash_advice",
- ALLOCSET_DEFAULT_SIZES);
- oldcontext = MemoryContextSwitchTo(pg_stash_advice_mcxt);
/* Attach to the fixed-size state object if not already done. */
if (pgsa_state == NULL)
@@ -540,80 +521,19 @@ pgsa_attach(void)
/* Attach to the DSA area if not already done. */
if (pgsa_dsa_area == NULL)
- {
- dsa_handle area_handle;
-
- LWLockAcquire(&pgsa_state->lock, LW_EXCLUSIVE);
- area_handle = pgsa_state->area;
- if (area_handle == DSA_HANDLE_INVALID)
- {
- pgsa_dsa_area = dsa_create(pgsa_state->dsa_tranche);
- dsa_pin(pgsa_dsa_area);
- pgsa_state->area = dsa_get_handle(pgsa_dsa_area);
- LWLockRelease(&pgsa_state->lock);
- }
- else
- {
- LWLockRelease(&pgsa_state->lock);
- pgsa_dsa_area = dsa_attach(area_handle);
- }
- dsa_pin_mapping(pgsa_dsa_area);
- }
+ pgsa_dsa_area = GetNamedDSA("pg_stash_advice_dsa", &found);
/* Attach to the stash_name->stash_id hash table if not already done. */
if (pgsa_stash_dshash == NULL)
- {
- dshash_table_handle stash_handle;
-
- LWLockAcquire(&pgsa_state->lock, LW_EXCLUSIVE);
- pgsa_stash_dshash_parameters.tranche_id = pgsa_state->stash_tranche;
- stash_handle = pgsa_state->stash_hash;
- if (stash_handle == DSHASH_HANDLE_INVALID)
- {
- pgsa_stash_dshash = dshash_create(pgsa_dsa_area,
- &pgsa_stash_dshash_parameters,
- NULL);
- pgsa_state->stash_hash =
- dshash_get_hash_table_handle(pgsa_stash_dshash);
- LWLockRelease(&pgsa_state->lock);
- }
- else
- {
- LWLockRelease(&pgsa_state->lock);
- pgsa_stash_dshash = dshash_attach(pgsa_dsa_area,
- &pgsa_stash_dshash_parameters,
- stash_handle, NULL);
- }
- }
+ pgsa_stash_dshash = GetNamedDSHash("pg_stash_advice_stash",
+ &pgsa_stash_dshash_parameters,
+ &found);
/* Attach to the entry hash table if not already done. */
if (pgsa_entry_dshash == NULL)
- {
- dshash_table_handle entry_handle;
-
- LWLockAcquire(&pgsa_state->lock, LW_EXCLUSIVE);
- pgsa_entry_dshash_parameters.tranche_id = pgsa_state->entry_tranche;
- entry_handle = pgsa_state->entry_hash;
- if (entry_handle == DSHASH_HANDLE_INVALID)
- {
- pgsa_entry_dshash = dshash_create(pgsa_dsa_area,
- &pgsa_entry_dshash_parameters,
- NULL);
- pgsa_state->entry_hash =
- dshash_get_hash_table_handle(pgsa_entry_dshash);
- LWLockRelease(&pgsa_state->lock);
- }
- else
- {
- LWLockRelease(&pgsa_state->lock);
- pgsa_entry_dshash = dshash_attach(pgsa_dsa_area,
- &pgsa_entry_dshash_parameters,
- entry_handle, NULL);
- }
- }
-
- /* Restore previous memory context. */
- MemoryContextSwitchTo(oldcontext);
+ pgsa_entry_dshash = GetNamedDSHash("pg_stash_advice_entry",
+ &pgsa_entry_dshash_parameters,
+ &found);
}
/*
@@ -789,13 +709,7 @@ pgsa_init_shared_state(void *ptr, void *arg)
LWLockInitialize(&state->lock,
LWLockNewTrancheId("pg_stash_advice_lock"));
- state->dsa_tranche = LWLockNewTrancheId("pg_stash_advice_dsa");
- state->stash_tranche = LWLockNewTrancheId("pg_stash_advice_stash");
- state->entry_tranche = LWLockNewTrancheId("pg_stash_advice_entry");
state->next_stash_id = UINT64CONST(1);
- state->area = DSA_HANDLE_INVALID;
- state->stash_hash = DSHASH_HANDLE_INVALID;
- state->entry_hash = DSHASH_HANDLE_INVALID;
}
/*
--
2.47.1
view thread (184+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: pg_plan_advice
In-Reply-To: <CAP53Pkyq-dVyJRxqui-fN8P0Qv5=oJXZoOyWn0pL=N5Rqi1HRw@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox