From 232ecb8d04832533f135a8233995f39aeb2ae691 Mon Sep 17 00:00:00 2001 From: Andreas Karlsson Date: Tue, 6 May 2025 18:39:14 +0200 Subject: [PATCH 1/2] Make inlined hash table lookup actually make sense There was no point in having the function for attaching the the shared memory hash table be inline if the non-inlined code had to be called every time anyway. We still maybe should just stop using an explicit inline since this is unlikely to be that performace critical code but at least if we use it we should use it correctly. --- contrib/pg_tde/src/catalog/tde_principal_key.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/contrib/pg_tde/src/catalog/tde_principal_key.c b/contrib/pg_tde/src/catalog/tde_principal_key.c index da4e88cc2133f..744b4dd611e84 100644 --- a/contrib/pg_tde/src/catalog/tde_principal_key.c +++ b/contrib/pg_tde/src/catalog/tde_principal_key.c @@ -200,9 +200,6 @@ principal_key_info_attach_shmem(void) MemoryContext oldcontext; dsa_area *dsa; - if (principalKeyLocalState.sharedHash) - return; - /* * We want the dsa to remain valid throughout the lifecycle of this * process. so switch to TopMemoryContext before attaching @@ -374,7 +371,8 @@ xl_tde_perform_rotate_key(XLogPrincipalKeyRotate *xlrec) static inline dshash_table * get_principal_key_Hash(void) { - principal_key_info_attach_shmem(); + if (!principalKeyLocalState.sharedHash) + principal_key_info_attach_shmem(); return principalKeyLocalState.sharedHash; } From b30d998961ba424f0b81cb5dba33d28b1beaf36c Mon Sep 17 00:00:00 2001 From: Andreas Karlsson Date: Tue, 6 May 2025 18:39:21 +0200 Subject: [PATCH 2/2] Use consistent casing of get_principal_key_hash() There was no good reason to have an upper case H. --- contrib/pg_tde/src/catalog/tde_principal_key.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/contrib/pg_tde/src/catalog/tde_principal_key.c b/contrib/pg_tde/src/catalog/tde_principal_key.c index 744b4dd611e84..e115bfb7b4097 100644 --- a/contrib/pg_tde/src/catalog/tde_principal_key.c +++ b/contrib/pg_tde/src/catalog/tde_principal_key.c @@ -87,7 +87,7 @@ static void initialize_objects_in_dsa_area(dsa_area *dsa, void *raw_dsa_area); static Size required_shared_mem_size(void); static void shared_memory_shutdown(int code, Datum arg); static void clear_principal_key_cache(Oid databaseId); -static inline dshash_table *get_principal_key_Hash(void); +static inline dshash_table *get_principal_key_hash(void); static TDEPrincipalKey *get_principal_key_from_cache(Oid dbOid); static bool pg_tde_is_same_principal_key(TDEPrincipalKey *a, TDEPrincipalKey *b); static void pg_tde_update_global_principal_key_everywhere(TDEPrincipalKey *oldKey, TDEPrincipalKey *newKey); @@ -369,7 +369,7 @@ xl_tde_perform_rotate_key(XLogPrincipalKeyRotate *xlrec) */ static inline dshash_table * -get_principal_key_Hash(void) +get_principal_key_hash(void) { if (!principalKeyLocalState.sharedHash) principal_key_info_attach_shmem(); @@ -384,10 +384,10 @@ get_principal_key_from_cache(Oid dbOid) { TDEPrincipalKey *cacheEntry = NULL; - cacheEntry = (TDEPrincipalKey *) dshash_find(get_principal_key_Hash(), + cacheEntry = (TDEPrincipalKey *) dshash_find(get_principal_key_hash(), &dbOid, false); if (cacheEntry) - dshash_release_lock(get_principal_key_Hash(), cacheEntry); + dshash_release_lock(get_principal_key_hash(), cacheEntry); return cacheEntry; } @@ -409,12 +409,12 @@ push_principal_key_to_cache(TDEPrincipalKey *principalKey) Oid databaseId = principalKey->keyInfo.databaseId; bool found = false; - cacheEntry = dshash_find_or_insert(get_principal_key_Hash(), + cacheEntry = dshash_find_or_insert(get_principal_key_hash(), &databaseId, &found); if (!found) *cacheEntry = *principalKey; - dshash_release_lock(get_principal_key_Hash(), cacheEntry); + dshash_release_lock(get_principal_key_hash(), cacheEntry); /* we don't want principal keys to end up paged to the swap */ if (mlock(cacheEntry, sizeof(TDEPrincipalKey)) == -1) @@ -445,11 +445,11 @@ clear_principal_key_cache(Oid databaseId) TDEPrincipalKey *cache_entry; /* Start with deleting the cache entry for the database */ - cache_entry = (TDEPrincipalKey *) dshash_find(get_principal_key_Hash(), + cache_entry = (TDEPrincipalKey *) dshash_find(get_principal_key_hash(), &databaseId, true); if (cache_entry) { - dshash_delete_entry(get_principal_key_Hash(), cache_entry); + dshash_delete_entry(get_principal_key_hash(), cache_entry); } }