Re: ResourceOwner refactoring

From: Andres Freund <andres(at)anarazel(dot)de>
To: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Cc: Julien Rouhaud <rjuju123(at)gmail(dot)com>, Aleksander Alekseev <aleksander(at)timescale(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: ResourceOwner refactoring
Date: 2022-11-01 15:42:51
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


On 2022-11-01 12:39:39 +0200, Heikki Linnakangas wrote:
> > 1) As far as I can tell, the way ResourceOwnerReleaseAll() now works seems to
> > assume that within a phase the reset order does not matter. I don't think
> > that's a good assumption. I e.g. have a patch to replace InProgressBuf with
> > resowner handling, and in-progress IO errors should be processed before
> > before pins are released.
> Hmm. Currently, you're not supposed to hold any resources at commit. You get
> warnings about resource leaks if a resource owner is not empty on
> ResourceOwnerReleaseAll(). On abort, does the order matter? I'm not familiar
> with your InProgressBuf patch, but I guess you could handle the in-progress
> IO errors in ReleaseBuffer().

I was thinking about doing that as well, but it's not really trivial to know
about the in-progress IO at that time, without additional tracking (which
isn't free).

> If we do need to worry about release order, perhaps add a "priority" or
> "phase" to each resource kind, and release them in priority order. We
> already have before- and after-locks as phases, but we could generalize
> that.
> However, I feel that trying to enforce a particular order moves the
> goalposts. If we need that, let's add it as a separate patch later.

Like Robert, I think that the patch is moving the goalpost...

> > 2) There's quite a few resource types where we actually don't need an entry in
> > an array, because we can instead add a dlist_node to the resource -
> > avoiding memory overhead and making removal cheap. I have a few pending
> > patches that use that approach, and this doesn't really provide a path for
> > that anymore.
> Is that materially better than using the array?

It's safe in critical sections. I have a, not really baked but promising,
patch to make WAL writes use AIO. There's no way to know the number of
"asynchronous IOs" needed before entering the critical section.

> The fast path with an array is very fast. If it is better, perhaps we should
> bite the bullet and require a dlist node and use that mechanism for all
> resource types?

I don't think it's suitable for all - you need an exclusively owned region of
memory to embed a list in. That works nicely for some things, but not others
(e.g. buffer pins).


Andres Freund

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Mingli Zhang 2022-11-01 15:45:45 Re: [Refactor]Avoid to handle FORCE_NOT_NULL/FORCE_NULL options when COPY TO
Previous Message Jonathan S. Katz 2022-11-01 15:19:02 Re: heavily contended lwlocks with long wait queues scale badly