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 |
Message-ID: | 20221101154251.64vyq7wy6w7aykrn@awork3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
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).
Greetings,
Andres Freund
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 |