Обсуждение: pgsql: dshash: Add sequential scan support.

Поиск
Список
Период
Сортировка

pgsql: dshash: Add sequential scan support.

От
Andres Freund
Дата:
dshash: Add sequential scan support.

Add ability to scan all entries sequentially to dshash. The interface is
similar but a bit different both from that of dynahash and simple dshash
search functions. The most significant differences is that dshash's interfac
always needs a call to dshash_seq_term when scan ends. Another is
locking. Dshash holds partition lock when returning an entry,
dshash_seq_next() also holds lock when returning an entry but callers
shouldn't release it, since the lock is essential to continue a scan. The
seqscan interface allows entry deletion while a scan is in progress using
dshash_delete_current().

Reviewed-By: Andres Freund <andres@anarazel.de>
Author: Kyotaro Horiguchi <horikyoga.ntt@gmail.com>

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/352d297dc74feb0bf0dcb255cc0dfaaed2b96c1e

Modified Files
--------------
src/backend/lib/dshash.c         | 163 ++++++++++++++++++++++++++++++++++++++-
src/include/lib/dshash.h         |  23 ++++++
src/tools/pgindent/typedefs.list |   1 +
3 files changed, 186 insertions(+), 1 deletion(-)


Re: pgsql: dshash: Add sequential scan support.

От
Tom Lane
Дата:
Andres Freund <andres@anarazel.de> writes:
> dshash: Add sequential scan support.
> Add ability to scan all entries sequentially to dshash. The interface is
> similar but a bit different both from that of dynahash and simple dshash
> search functions. The most significant differences is that dshash's interfac
> always needs a call to dshash_seq_term when scan ends.

Umm ... what about error recovery?  Or have you just cemented the
proposition that long-lived dshashes are unsafe?

            regards, tom lane



Re: pgsql: dshash: Add sequential scan support.

От
Andres Freund
Дата:
Hi,

On 2022-03-10 20:09:56 -0500, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > dshash: Add sequential scan support.
> > Add ability to scan all entries sequentially to dshash. The interface is
> > similar but a bit different both from that of dynahash and simple dshash
> > search functions. The most significant differences is that dshash's interfac
> > always needs a call to dshash_seq_term when scan ends.
> 
> Umm ... what about error recovery?  Or have you just cemented the
> proposition that long-lived dshashes are unsafe?

I don't think this commit made it worse. dshash_seq_term() releases an lwlock
(which will be released in case of an error) and unsets
hash_table->find_[exclusively_]locked. The latter weren't introduced by this
patch, and are also set by dshash_find().

I agree that ->find_[exclusively_]locked are problematic from an error
recovery perspective.

It's per-backend state at least and just used for assertions. We could remove
it. Or stop checking it in places where it could be set wrongly: dshash_find()
and dshash_detach() couldn't check anymore, but the rest of the assertions
would still be valid afaics?

Greetings,

Andres Freund