Re: pgsql: dshash: Add sequential scan support. - Mailing list pgsql-hackers

From Andres Freund
Subject Re: pgsql: dshash: Add sequential scan support.
Date
Msg-id 20220704232538.x75aaawzjczhpvc6@awork3.anarazel.de
Whole thread Raw
In response to Re: pgsql: dshash: Add sequential scan support.  (Thomas Munro <thomas.munro@gmail.com>)
Responses Re: pgsql: dshash: Add sequential scan support.
List pgsql-hackers
Hi,

On 2022-07-05 11:20:54 +1200, Thomas Munro wrote:
> On Tue, Jul 5, 2022 at 8:54 AM Andres Freund <andres@anarazel.de> wrote:
> > > Yeah, it's all for assertions... let's just remove it.  Those
> > > assertions were useful to me at some stage in development but won't
> > > hold as well as I thought, at least without widespread PG_FINALLY(),
> > > which wouldn't be nice.
> >
> > Hm. I'd be inclined to at least add a few more
> > Assert(!LWLockHeldByMe[InMode]()) style assertions. E.g. to
> > dshash_find_or_insert().
> 
> Yeah, I was wondering about that, but it needs to check the whole 128
> element lock array.

I think it'd be ok to just check the current partition - yes, it'd not catch
cases where we're still holding a lock on another partition, but that's imo
not too bad?


> Hmm, yeah that seems OK for assertion builds.
> Since there were 6 places with I-hold-no-lock assertions, I shoved the
> loop into a function so I could do:
> 
> -               Assert(!status->hash_table->find_locked);
> +               assert_no_lock_held_by_me(hash_table);

I am a *bit* wary about the costs of that, even in assert builds - each of the
partition checks in the loop will in turn need to iterate through
held_lwlocks. But I guess we can also just later weaken them if it turns out
to be a problem.

Greetings,

Andres Freund



pgsql-hackers by date:

Previous
From: Thomas Munro
Date:
Subject: Re: pgsql: dshash: Add sequential scan support.
Next
From: Peter Smith
Date:
Subject: Re: Handle infinite recursion in logical replication setup