pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Nathan Bossart <nathandbossart@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Nathan Bossart <nathan@postgresql.org>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: pgsql: Teach DSM registry to ERROR if attaching to an uninitialized ent
Date: Mon, 24 Nov 2025 12:36:25 -0600
Message-ID: <aSSlqWwLVuXMmN-o@nathan> (raw)
In-Reply-To: <aSIR4Z4U887f_KcQ@nathan>
References: <E1vJHUk-006I7r-37@gemulon.postgresql.org>
	<CA+TgmoZGFzLJpYANBG9yDyoakVm-P2Acdker-Y+DHvPWfvBE5w@mail.gmail.com>
	<aR89_9oYFHNtUTFa@nathan>
	<CA+TgmoZCHER55cGmnL_svMjH=k-S0w29ih+ym_+=t032TSsOFA@mail.gmail.com>
	<aR-fqzfZYSd3TVKa@nathan>
	<CA+TgmoZsxbVhoxNGTWSd6=vHvyoqOSDNXJZabBZx+St8uPduLQ@mail.gmail.com>
	<aSIR4Z4U887f_KcQ@nathan>

On Sat, Nov 22, 2025 at 01:41:21PM -0600, Nathan Bossart wrote:
> I'll give what you suggested a try.  It seems reasonable enough.

Here is a mostly-untested first try.

-- 
nathan

Attachments: download all (2 files, .tar.gz)

v1-0001-Revert-Teach-DSM-registry-to-ERROR-if-attaching-t.patch (text/plain, 3.7K) download From e639d70a10f6288034ebe433a22b9dbc53562b01 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nathan@postgresql.org> Date: Mon, 24 Nov 2025 11:03:52 -0600 Subject: [PATCH v1 1/2] Revert "Teach DSM registry to ERROR if attaching to an uninitialized entry." This reverts commit 1165a933aab1355757a43cfd9193b6cce06f573b. --- src/backend/storage/ipc/dsm_registry.c | 34 +++----------------------- 1 file changed, 4 insertions(+), 30 deletions(-) diff --git a/src/backend/storage/ipc/dsm_registry.c b/src/backend/storage/ipc/dsm_registry.c index 7eba8a8cffb..a926b9c3f32 100644 --- a/src/backend/storage/ipc/dsm_registry.c +++ b/src/backend/storage/ipc/dsm_registry.c @@ -93,7 +93,6 @@ typedef struct DSMRegistryEntry { char name[NAMEDATALEN]; DSMREntryType type; - bool initialized; union { NamedDSMState dsm; @@ -217,7 +216,6 @@ GetNamedDSMSegment(const char *name, size_t size, dsm_segment *seg; entry->type = DSMR_ENTRY_TYPE_DSM; - entry->initialized = false; /* Initialize the segment. */ seg = dsm_create(size, 0); @@ -230,21 +228,13 @@ GetNamedDSMSegment(const char *name, size_t size, if (init_callback) (*init_callback) (ret); - - entry->initialized = true; } else if (entry->type != DSMR_ENTRY_TYPE_DSM) ereport(ERROR, - (errmsg("requested DSM segment \"%s\" does not match type of existing entry", - name))); - else if (!entry->initialized) - ereport(ERROR, - (errmsg("requested DSM segment \"%s\" failed initialization", - name))); + (errmsg("requested DSM segment does not match type of existing entry"))); else if (entry->dsm.size != size) ereport(ERROR, - (errmsg("requested DSM segment \"%s\" does not match size of existing entry", - name))); + (errmsg("requested DSM segment size does not match size of existing segment"))); else { NamedDSMState *state = &entry->dsm; @@ -307,7 +297,6 @@ GetNamedDSA(const char *name, bool *found) NamedDSAState *state = &entry->dsa; entry->type = DSMR_ENTRY_TYPE_DSA; - entry->initialized = false; /* Initialize the LWLock tranche for the DSA. */ state->tranche = LWLockNewTrancheId(name); @@ -319,17 +308,10 @@ GetNamedDSA(const char *name, bool *found) /* Store handle for other backends to use. */ state->handle = dsa_get_handle(ret); - - entry->initialized = true; } else if (entry->type != DSMR_ENTRY_TYPE_DSA) ereport(ERROR, - (errmsg("requested DSA \"%s\" does not match type of existing entry", - name))); - else if (!entry->initialized) - ereport(ERROR, - (errmsg("requested DSA \"%s\" failed initialization", - name))); + (errmsg("requested DSA does not match type of existing entry"))); else { NamedDSAState *state = &entry->dsa; @@ -390,7 +372,6 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found) dsa_area *dsa; entry->type = DSMR_ENTRY_TYPE_DSH; - entry->initialized = false; /* Initialize the LWLock tranche for the hash table. */ dsh_state->tranche = LWLockNewTrancheId(name); @@ -408,17 +389,10 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found) /* Store handles for other backends to use. */ dsh_state->dsa_handle = dsa_get_handle(dsa); dsh_state->dsh_handle = dshash_get_hash_table_handle(ret); - - entry->initialized = true; } else if (entry->type != DSMR_ENTRY_TYPE_DSH) ereport(ERROR, - (errmsg("requested DSHash \"%s\" does not match type of existing entry", - name))); - else if (!entry->initialized) - ereport(ERROR, - (errmsg("requested DSHash \"%s\" failed initialization", - name))); + (errmsg("requested DSHash does not match type of existing entry"))); else { NamedDSHState *dsh_state = &entry->dsh; -- 2.39.5 (Apple Git-154)
v1-0002-handle-ERRORs-in-DSM-registry-functions.patch (text/plain, 5.4K) download From 6da47303c5e5acaca4cc9808316a0457f6c6f0af Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nathan@postgresql.org> Date: Mon, 24 Nov 2025 12:28:23 -0600 Subject: [PATCH v1 2/2] handle ERRORs in DSM registry functions --- src/backend/storage/ipc/dsm_registry.c | 129 ++++++++++++++++++++++--- 1 file changed, 115 insertions(+), 14 deletions(-) diff --git a/src/backend/storage/ipc/dsm_registry.c b/src/backend/storage/ipc/dsm_registry.c index a926b9c3f32..7d1b584d166 100644 --- a/src/backend/storage/ipc/dsm_registry.c +++ b/src/backend/storage/ipc/dsm_registry.c @@ -154,10 +154,24 @@ init_dsm_registry(void) if (DSMRegistryCtx->dshh == DSHASH_HANDLE_INVALID) { /* Initialize dynamic shared hash table for registry. */ - dsm_registry_dsa = dsa_create(LWTRANCHE_DSM_REGISTRY_DSA); + PG_TRY(); + { + dsm_registry_dsa = dsa_create(LWTRANCHE_DSM_REGISTRY_DSA); + dsm_registry_table = dshash_create(dsm_registry_dsa, &dsh_params, NULL); + } + PG_CATCH(); + { + if (dsm_registry_dsa) + { + dsa_detach(dsm_registry_dsa); + dsm_registry_dsa = NULL; + } + PG_RE_THROW(); + } + PG_END_TRY(); + dsa_pin(dsm_registry_dsa); dsa_pin_mapping(dsm_registry_dsa); - dsm_registry_table = dshash_create(dsm_registry_dsa, &dsh_params, NULL); /* Store handles in shared memory for other backends to use. */ DSMRegistryCtx->dsah = dsa_get_handle(dsm_registry_dsa); @@ -216,18 +230,38 @@ GetNamedDSMSegment(const char *name, size_t size, dsm_segment *seg; entry->type = DSMR_ENTRY_TYPE_DSM; + state->handle = DSM_HANDLE_INVALID; /* Initialize the segment. */ - seg = dsm_create(size, 0); + PG_TRY(); + { + seg = dsm_create(size, 0); + } + PG_CATCH(); + { + dshash_delete_entry(dsm_registry_table, entry); + PG_RE_THROW(); + } + PG_END_TRY(); + + PG_TRY(); + { + ret = dsm_segment_address(seg); + if (init_callback) + (*init_callback) (ret); + } + PG_CATCH(); + { + dsm_detach(seg); + dshash_delete_entry(dsm_registry_table, entry); + PG_RE_THROW(); + } + PG_END_TRY(); dsm_pin_segment(seg); dsm_pin_mapping(seg); state->handle = dsm_segment_handle(seg); state->size = size; - ret = dsm_segment_address(seg); - - if (init_callback) - (*init_callback) (ret); } else if (entry->type != DSMR_ENTRY_TYPE_DSM) ereport(ERROR, @@ -297,9 +331,19 @@ GetNamedDSA(const char *name, bool *found) NamedDSAState *state = &entry->dsa; entry->type = DSMR_ENTRY_TYPE_DSA; + state->handle = DSA_HANDLE_INVALID; /* Initialize the LWLock tranche for the DSA. */ - state->tranche = LWLockNewTrancheId(name); + PG_TRY(); + { + state->tranche = LWLockNewTrancheId(name); + } + PG_CATCH(); + { + dshash_delete_entry(dsm_registry_table, entry); + PG_RE_THROW(); + } + PG_END_TRY(); /* Initialize the DSA. */ ret = dsa_create(state->tranche); @@ -312,6 +356,16 @@ GetNamedDSA(const char *name, bool *found) else if (entry->type != DSMR_ENTRY_TYPE_DSA) ereport(ERROR, (errmsg("requested DSA does not match type of existing entry"))); + else if (entry->dsa.handle == DSA_HANDLE_INVALID) + { + NamedDSAState *state = &entry->dsa; + + ret = dsa_create(state->tranche); + + dsa_pin(ret); + dsa_pin_mapping(ret); + state->handle = dsa_get_handle(ret); + } else { NamedDSAState *state = &entry->dsa; @@ -372,19 +426,40 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found) dsa_area *dsa; entry->type = DSMR_ENTRY_TYPE_DSH; + dsh_state->dsa_handle = DSA_HANDLE_INVALID; + dsh_state->dsh_handle = DSHASH_HANDLE_INVALID; /* Initialize the LWLock tranche for the hash table. */ - dsh_state->tranche = LWLockNewTrancheId(name); + PG_TRY(); + { + dsh_state->tranche = LWLockNewTrancheId(name); + } + PG_CATCH(); + { + dshash_delete_entry(dsm_registry_table, entry); + PG_RE_THROW(); + } + PG_END_TRY(); /* Initialize the DSA for the hash table. */ dsa = dsa_create(dsh_state->tranche); - dsa_pin(dsa); - dsa_pin_mapping(dsa); /* Initialize the dshash table. */ - memcpy(&params_copy, params, sizeof(dshash_parameters)); - params_copy.tranche_id = dsh_state->tranche; - ret = dshash_create(dsa, &params_copy, NULL); + PG_TRY(); + { + memcpy(&params_copy, params, sizeof(dshash_parameters)); + params_copy.tranche_id = dsh_state->tranche; + ret = dshash_create(dsa, &params_copy, NULL); + } + PG_CATCH(); + { + dsa_detach(dsa); + PG_RE_THROW(); + } + PG_END_TRY(); + + dsa_pin(dsa); + dsa_pin_mapping(dsa); /* Store handles for other backends to use. */ dsh_state->dsa_handle = dsa_get_handle(dsa); @@ -393,6 +468,32 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found) else if (entry->type != DSMR_ENTRY_TYPE_DSH) ereport(ERROR, (errmsg("requested DSHash does not match type of existing entry"))); + else if (entry->dsh.dsa_handle == DSA_HANDLE_INVALID) + { + NamedDSHState *dsh_state = &entry->dsh; + dshash_parameters params_copy; + dsa_area *dsa; + + dsa = dsa_create(dsh_state->tranche); + + PG_TRY(); + { + memcpy(&params_copy, params, sizeof(dshash_parameters)); + params_copy.tranche_id = dsh_state->tranche; + ret = dshash_create(dsa, &params_copy, NULL); + } + PG_CATCH(); + { + dsa_detach(dsa); + PG_RE_THROW(); + } + PG_END_TRY(); + + dsa_pin(dsa); + dsa_pin_mapping(dsa); + dsh_state->dsa_handle = dsa_get_handle(dsa); + dsh_state->dsh_handle = dshash_get_hash_table_handle(ret); + } else { NamedDSHState *dsh_state = &entry->dsh; -- 2.39.5 (Apple Git-154)

view thread (19+ messages)  latest in thread

Message-ID: <aSSlqWwLVuXMmN-o@nathan>
Permalink:  ../aSSlqWwLVuXMmN-o@nathan/
Also on:    postgresql.org/message-id/aSSlqWwLVuXMmN-o@nathan

 ·  · 

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: pgsql-hackers@postgresql.org
  Cc: nathandbossart@gmail.com, robertmhaas@gmail.com, nathan@postgresql.org, pgsql-hackers@lists.postgresql.org
  Subject: Re: pgsql: Teach DSM registry to ERROR if attaching to an uninitialized ent
  In-Reply-To: <aSSlqWwLVuXMmN-o@nathan>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox