On Fri, Nov 17, 2023 at 12:13 PM Andres Freund <and...@anarazel.de> wrote: > > On 2023-11-17 10:42:54 -0800, Jeff Davis wrote: > > Right now, if allocation fails while growing a hashtable, it's left in > > an inconsistent state and can't be used again.
+1 to the patch. > I'm not against allowing this - but I am curious, in which use cases is this > useful? I don't have an answer to that, but a guess would be when the server is dealing with memory pressure. In my view the patch has merit purely on the grounds of increasing robustness. > > @@ -446,10 +459,11 @@ SH_CREATE(MemoryContext ctx, uint32 nelements, void > > *private_data) > > /* increase nelements by fillfactor, want to store nelements elements > > */ > > size = Min((double) SH_MAX_SIZE, ((double) nelements) / > > SH_FILLFACTOR); > > > > - SH_COMPUTE_PARAMETERS(tb, size); > > + size = SH_COMPUTE_SIZE(size); > > > > - tb->data = (SH_ELEMENT_TYPE *) SH_ALLOCATE(tb, > > sizeof(SH_ELEMENT_TYPE) * tb->size); > > + tb->data = (SH_ELEMENT_TYPE *) SH_ALLOCATE(tb, > > sizeof(SH_ELEMENT_TYPE) * size); > > > > + SH_UPDATE_PARAMETERS(tb, size); > > return tb; > > } > > Maybe add a comment explaining why it's important to update parameters after > allocating? Perhaps something like this: + /* + * Update parameters _after_ allocation succeeds; prevent + * bogus/corrupted state. + */ + SH_UPDATE_PARAMETERS(tb, size); Best regards, Gurjeet http://Gurje.et