Andres Freund writes:
> On 2021-11-06 18:32:54 -0400, Tom Lane wrote:
>> Good point. The note at list_delete_last that it's O(1) isn't really
>> on point --- instead, the text for list_delete_first should be like
>>
>> + * Note that this takes time proportional to the length of the list,
>> + *
On 2021-11-06 18:32:54 -0400, Tom Lane wrote:
> Andres Freund writes:
> > On 2021-11-06 14:06:12 -0400, Tom Lane wrote:
> >> + * Note that this takes time proportional to the length of the list,
> >> + * since the remaining entries must be moved.
> >> */
> >> List *
> >> list_delete_first(List *li
Andres Freund writes:
> On 2021-11-06 14:06:12 -0400, Tom Lane wrote:
>> + * Note that this takes time proportional to the length of the list,
>> + * since the remaining entries must be moved.
>> */
>> List *
>> list_delete_first(List *list)
> Perhaps we could point to list_delete_last()? But it'
Hi,
On 2021-11-06 14:06:12 -0400, Tom Lane wrote:
> I wrote:
> > Hm. I think it's not the only list function with O(N) behavior;
> > in fact there used to be more such functions than there are now.
> > But I could get behind a patch that annotates all of them.
Personally I think the delete first
I wrote:
> Hm. I think it's not the only list function with O(N) behavior;
> in fact there used to be more such functions than there are now.
> But I could get behind a patch that annotates all of them.
Here's a quick hack at that. Having done it, I'm not sure if it's
really worth the trouble or
Michael Paquier writes:
> On Thu, Nov 04, 2021 at 08:21:56PM -0400, Tom Lane wrote:
>> Hm. I think it's not the only list function with O(N) behavior;
>> in fact there used to be more such functions than there are now.
>> But I could get behind a patch that annotates all of them.
> Documenting t
On Thu, Nov 04, 2021 at 08:21:56PM -0400, Tom Lane wrote:
> Andres Freund writes:
> > I wonder if it's worth adding a note to list_delete_first() mentioning its
> > O(N) behaviour. It's not immediately visible from the code, and from the
> > list
> > name one could very well be excused to not be
Andres Freund writes:
> I wonder if it's worth adding a note to list_delete_first() mentioning its
> O(N) behaviour. It's not immediately visible from the code, and from the list
> name one could very well be excused to not be worried about O(N) costs.
Hm. I think it's not the only list function
Hi,
On 2021-11-02 11:35:55 -0400, Tom Lane wrote:
> It's not clear that the remaining list_delete_first
> callers have any real problem; and changing them would be complex.
I wonder if it's worth adding a note to list_delete_first() mentioning its
O(N) behaviour. It's not immediately visible from
On 11/2/21, 8:36 AM, "Tom Lane" wrote:
> I've pushed the SyncPostCheckpoint change, and I think I'm going
> to stop here. It's not clear that the remaining list_delete_first
> callers have any real problem; and changing them would be complex.
> We can revisit the question if we find out there is
I've pushed the SyncPostCheckpoint change, and I think I'm going
to stop here. It's not clear that the remaining list_delete_first
callers have any real problem; and changing them would be complex.
We can revisit the question if we find out there is an issue.
Or, if somebody else wants to pursue t
Kyotaro Horiguchi writes:
> At Mon, 01 Nov 2021 18:01:18 -0400, Tom Lane wrote in
>> So what I did in the attached is add a "canceled" flag to
>> PendingUnlinkEntry, which lets us deal with canceled or finished
>> entries without having to delete them from the list right away.
>> Then we only ne
At Mon, 01 Nov 2021 18:01:18 -0400, Tom Lane wrote in
> "Bossart, Nathan" writes:
> > On 10/31/21, 1:55 PM, "Tom Lane" wrote:
> >> 2. I think we almost certainly have a problem in SyncPostCheckpoint.
>
> > This one doesn't look as straightforward. It looks like we might need
> > a list_delete
At Mon, 1 Nov 2021 17:02:49 +, "Bossart, Nathan"
wrote in
> On 11/1/21, 9:58 AM, "Tom Lane" wrote:
> > "Bossart, Nathan" writes:
> >> Should there be a list_free(trgmNFA->queue) at the end of
> >> transformGraph()?
> >
> > There could be, but that's visibly invoked only once per
> > create
At Mon, 01 Nov 2021 11:58:35 -0400, Tom Lane wrote in
> Kyotaro Horiguchi writes:
> > At Sun, 31 Oct 2021 16:55:01 -0400, Tom Lane wrote in
> >> I looked at the remaining list_delete_first callers.
> >>
> >> 1. Attached is a proposed patch to get rid of the calls in trgm_regexp.c.
> >> I'm no
"Bossart, Nathan" writes:
> On 10/31/21, 1:55 PM, "Tom Lane" wrote:
>> 2. I think we almost certainly have a problem in SyncPostCheckpoint.
> This one doesn't look as straightforward. It looks like we might need
> a list_delete_first_n() to delete the first N entries all at once to
> improve th
On 11/1/21, 10:34 AM, "Tom Lane" wrote:
> Thanks for looking! Here's an expanded patch that also takes care
> of the other two easy-to-fix cases, nodeAgg.c and llvmjit.c.
> AFAICS, llvm_release_context is like StandbyReleaseLockList
> in that we don't need to worry about whether the data structur
"Bossart, Nathan" writes:
> Ah, I see it now. The patch looks good to me, then.
Thanks for looking! Here's an expanded patch that also takes care
of the other two easy-to-fix cases, nodeAgg.c and llvmjit.c.
AFAICS, llvm_release_context is like StandbyReleaseLockList
in that we don't need to wor
On 11/1/21, 9:58 AM, "Tom Lane" wrote:
> "Bossart, Nathan" writes:
>> Should there be a list_free(trgmNFA->queue) at the end of
>> transformGraph()?
>
> There could be, but that's visibly invoked only once per
> createTrgmNFAInternal call, so I didn't think it was worthwhile
> to do so (unlike th
"Bossart, Nathan" writes:
> On 10/31/21, 1:55 PM, "Tom Lane" wrote:
>> 1. Attached is a proposed patch to get rid of the calls in trgm_regexp.c.
> Should there be a list_free(trgmNFA->queue) at the end of
> transformGraph()?
There could be, but that's visibly invoked only once per
createTrgmNF
On 10/31/21, 1:55 PM, "Tom Lane" wrote:
> 1. Attached is a proposed patch to get rid of the calls in trgm_regexp.c.
> I'm not certain that those lists could get long enough to be a problem,
> given the existing complexity limits in that file (MAX_EXPANDED_STATES
> etc). But I'm not certain they c
On 10/31/21, 12:39 PM, "Tom Lane" wrote:
> Yeah, there's no expectation that this data structure needs to be kept
> consistent after an error; and I'm not real sure that the existing
> code could claim to satisfy such a requirement if we did need it.
> (There's at least a short window where the ca
Kyotaro Horiguchi writes:
> At Sun, 31 Oct 2021 16:55:01 -0400, Tom Lane wrote in
>> I looked at the remaining list_delete_first callers.
>>
>> 1. Attached is a proposed patch to get rid of the calls in trgm_regexp.c.
>> I'm not certain that those lists could get long enough to be a problem,
>>
At Sun, 31 Oct 2021 16:55:01 -0400, Tom Lane wrote in
> I wrote:
> > Pushed the patch as given. I've not yet reviewed other list_delete_first
> > callers, but I'll take a look. (I seem to remember that I did survey
> > them while writing 1cff1b95a, but I evidently missed that this code
> > coul
I wrote:
> Pushed the patch as given. I've not yet reviewed other list_delete_first
> callers, but I'll take a look. (I seem to remember that I did survey
> them while writing 1cff1b95a, but I evidently missed that this code
> could be dealing with a list long enough to be problematic.)
I looked
Hi,
On 2021-10-31 15:38:15 -0400, Tom Lane wrote:
> Yeah, there's no expectation that this data structure needs to be kept
> consistent after an error; and I'm not real sure that the existing
> code could claim to satisfy such a requirement if we did need it.
To be clear, I was making that commen
"Bossart, Nathan" writes:
> On 10/28/21, 11:53 PM, "Michael Paquier" wrote:
>> Actually, as the list of recovery locks is saved in TopMemoryContext,
>> wouldn't it be better to keep a per-cell deletion of the list, which
>> would mean that we'd better do the operation in the reverse order to
>> m
On 10/28/21, 11:53 PM, "Michael Paquier" wrote:
> On Thu, Oct 28, 2021 at 04:52:48PM -0700, Andres Freund wrote:
>> I suspect the reverse lock order release could be tad faster. But I probably
>> wouldn't change it either - I was more thinking of some of the other cases
>> that deleted the first e
On Thu, Oct 28, 2021 at 04:52:48PM -0700, Andres Freund wrote:
> I suspect the reverse lock order release could be tad faster. But I probably
> wouldn't change it either - I was more thinking of some of the other cases
> that deleted the first element, here it's a bit harder to know wether there's
Hi,
On 2021-10-28 19:24:08 -0400, Tom Lane wrote:
> "Bossart, Nathan" writes:
> > On 10/28/21, 3:25 PM, "Tom Lane" wrote:
> >> Does it matter what order we're releasing the locks in?
>
> > I'm not seeing anything that indicates the ordering matters. AFAICT
> > either approach would work in thi
"Bossart, Nathan" writes:
> On 10/28/21, 3:25 PM, "Tom Lane" wrote:
>> Does it matter what order we're releasing the locks in?
> I'm not seeing anything that indicates the ordering matters. AFAICT
> either approach would work in this case. IMO changing the order is
> scarier than switching to
On 10/28/21, 3:25 PM, "Tom Lane" wrote:
> Andres Freund writes:
>> Which leads to to wonder whether the better fix would be to switch to
>> deleting
>> the last element, but still use the while (!empty) style. That should convert
>> the O(n^2) due to 1cff1b9 back to O(n). It might or might not b
On 10/28/21, 3:15 PM, "Andres Freund" wrote:
> Which leads to to wonder whether the better fix would be to switch to deleting
> the last element, but still use the while (!empty) style. That should convert
> the O(n^2) due to 1cff1b9 back to O(n). It might or might not be faster/slower
> than usin
Andres Freund writes:
> Which leads to to wonder whether the better fix would be to switch to deleting
> the last element, but still use the while (!empty) style. That should convert
> the O(n^2) due to 1cff1b9 back to O(n). It might or might not be faster/slower
> than using foreach(), but it sho
Hi,
On 2021-10-28 15:07:48 -0700, Andres Freund wrote:
> On 2021-10-28 15:57:51 +0900, Kyotaro Horiguchi wrote:
> > I found several other instances of the pattern
> > "while(list){list_delete_first(); /*no-break*/}" in
> > llvm_release_context, gistProcessEmptyingQueue, AtEOXact_Namespace and
> >
Hi,
On 2021-10-28 15:57:51 +0900, Kyotaro Horiguchi wrote:
> I found several other instances of the pattern
> "while(list){list_delete_first(); /*no-break*/}" in
> llvm_release_context, gistProcessEmptyingQueue, AtEOXact_Namespace and
> maybe transformGraph and processState in trgm_regexp.c. We m
On 10/28/21, 12:58 AM, "Michael Paquier" wrote:
> On Thu, Oct 28, 2021 at 03:57:51PM +0900, Kyotaro Horiguchi wrote:
>> However, I'm fine with fixing only StandbyRelaseLockList(), which
>> actually suffers from list_delete_first().
>
> I can also see a large gap between one technique and the other
On Thu, Oct 28, 2021 at 03:57:51PM +0900, Kyotaro Horiguchi wrote:
> I found several other instances of the pattern
> "while(list){list_delete_first(); /*no-break*/}" in
> llvm_release_context, gistProcessEmptyingQueue, AtEOXact_Namespace and
> maybe transformGraph and processState in trgm_regexp.c
At Thu, 28 Oct 2021 09:54:42 +0530, Amul Sul wrote in
> On Thu, Oct 28, 2021 at 9:07 AM Bossart, Nathan wrote:
> >
> > Hi hackers,
> >
> > I've seen a few cases now for v13 where the startup process on a
> > standby appears to be stuck on StandbyReleaseLockList(). It looks
> > like most of the
On Thu, Oct 28, 2021 at 9:07 AM Bossart, Nathan wrote:
>
> Hi hackers,
>
> I've seen a few cases now for v13 where the startup process on a
> standby appears to be stuck on StandbyReleaseLockList(). It looks
> like most of the time is spent on list_delete_first(). I believe this
> is related to
40 matches
Mail list logo
