Re: Backpatch b61d161c14 (Introduce vacuum errcontext ...)

2020-07-08 Thread Amit Kapila
On Thu, Jul 2, 2020 at 9:07 AM Amit Kapila wrote: > > On Tue, Jun 30, 2020 at 9:30 AM Amit Kapila wrote: > > > > > > > > If I am not missing anything then that change was in > > > lazy_cleanup_index and after this patch, it won't be required because > > > we are using a different variable name.

Re: Backpatch b61d161c14 (Introduce vacuum errcontext ...)

2020-07-01 Thread Amit Kapila
On Tue, Jun 30, 2020 at 9:30 AM Amit Kapila wrote: > > > > > If I am not missing anything then that change was in > > lazy_cleanup_index and after this patch, it won't be required because > > we are using a different variable name. > > > > I have combined both the patches now. > > > > I am

Re: Backpatch b61d161c14 (Introduce vacuum errcontext ...)

2020-06-29 Thread Amit Kapila
On Fri, Jun 26, 2020 at 9:19 AM Amit Kapila wrote: > > On Fri, Jun 26, 2020 at 7:25 AM Justin Pryzby wrote: > > > > > > Comments: > > > > > * The index name is saved only during this phase and restored > > > immediately > > > > => I wouldn't say "only" since it's saved during

Re: Backpatch b61d161c14 (Introduce vacuum errcontext ...)

2020-06-25 Thread Amit Kapila
On Fri, Jun 26, 2020 at 7:25 AM Justin Pryzby wrote: > > > > I have done some testing with both the patches and would like to do > > more unless there are objections with these. > > Comments: > > > * The index name is saved only during this phase and restored > > immediately > > => I

Re: Backpatch b61d161c14 (Introduce vacuum errcontext ...)

2020-06-25 Thread Justin Pryzby
On Thu, Jun 25, 2020 at 02:31:51PM +0530, Amit Kapila wrote: > I have looked at both the patches (using separate variables (by > Justin) and using a struct (by Andres)) and found the second one bit > better. Thanks for looking. > I have improved some comments in the code and for now, kept as two

Re: Backpatch b61d161c14 (Introduce vacuum errcontext ...)

2020-06-25 Thread Amit Kapila
On Tue, Jun 23, 2020 at 11:49 PM Andres Freund wrote: > > Hi, > > On 2020-06-23 00:14:40 -0400, Tom Lane wrote: > > Andres Freund writes: > > > I am only suggesting that where you save the old location, as currently > > > done with LVRelStats olderrinfo, you instead use a more specific > > >

Re: Backpatch b61d161c14 (Introduce vacuum errcontext ...)

2020-06-23 Thread Andres Freund
Hi, On 2020-06-23 00:14:40 -0400, Tom Lane wrote: > Andres Freund writes: > > I am only suggesting that where you save the old location, as currently > > done with LVRelStats olderrinfo, you instead use a more specific > > type. Not that you should pass that anywhere (except for > >

Re: Backpatch b61d161c14 (Introduce vacuum errcontext ...)

2020-06-23 Thread Justin Pryzby
On Tue, Jun 23, 2020 at 12:14:40AM -0400, Tom Lane wrote: > Andres Freund writes: > > I am only suggesting that where you save the old location, as currently > > done with LVRelStats olderrinfo, you instead use a more specific > > type. Not that you should pass that anywhere (except for > >

Re: Backpatch b61d161c14 (Introduce vacuum errcontext ...)

2020-06-22 Thread Tom Lane
Andres Freund writes: > I am only suggesting that where you save the old location, as currently > done with LVRelStats olderrinfo, you instead use a more specific > type. Not that you should pass that anywhere (except for > update_vacuum_error_info). As things currently stand, I don't think we

Re: Backpatch b61d161c14 (Introduce vacuum errcontext ...)

2020-06-22 Thread Andres Freund
Hi, On 2020-06-22 20:43:47 -0500, Justin Pryzby wrote: > On Mon, Jun 22, 2020 at 01:57:12PM -0700, Andres Freund wrote: > > On 2020-06-22 15:43:11 -0500, Justin Pryzby wrote: > > > On Mon, Jun 22, 2020 at 01:09:39PM -0700, Andres Freund wrote: > > > > I'm also uncomfortable with the approach of

Re: Backpatch b61d161c14 (Introduce vacuum errcontext ...)

2020-06-22 Thread Amit Kapila
On Tue, Jun 23, 2020 at 7:13 AM Justin Pryzby wrote: > > On Mon, Jun 22, 2020 at 01:57:12PM -0700, Andres Freund wrote: > > > > No, I don't think that's a solution. I think it's wrong to have > > something like olderrinfo in the first place. Using a struct with ~25 > > members to store the

Re: Backpatch b61d161c14 (Introduce vacuum errcontext ...)

2020-06-22 Thread Justin Pryzby
On Mon, Jun 22, 2020 at 01:57:12PM -0700, Andres Freund wrote: > On 2020-06-22 15:43:11 -0500, Justin Pryzby wrote: > > On Mon, Jun 22, 2020 at 01:09:39PM -0700, Andres Freund wrote: > > > I'm also uncomfortable with the approach of just copying all of > > > LVRelStats in several places: > > > >

Re: Backpatch b61d161c14 (Introduce vacuum errcontext ...)

2020-06-22 Thread Tom Lane
Justin Pryzby writes: > On Mon, Jun 22, 2020 at 06:15:27PM -0400, Tom Lane wrote: >> That seems like rather pointless micro-optimization really; the struct's >> not *that* large. But I have a different complaint now that I look at >> this code: is it safe at all? I see that the indname field is

Re: Backpatch b61d161c14 (Introduce vacuum errcontext ...)

2020-06-22 Thread Justin Pryzby
On Mon, Jun 22, 2020 at 06:15:27PM -0400, Tom Lane wrote: > Andres Freund writes: > > No, I don't think that's a solution. I think it's wrong to have > > something like olderrinfo in the first place. Using a struct with ~25 > > members to store the current state of three variables just doesn't

Re: Backpatch b61d161c14 (Introduce vacuum errcontext ...)

2020-06-22 Thread Tom Lane
Andres Freund writes: > No, I don't think that's a solution. I think it's wrong to have > something like olderrinfo in the first place. Using a struct with ~25 > members to store the current state of three variables just doesn't make > sense. Why isn't this just a LVSavedPosition struct or

Re: Backpatch b61d161c14 (Introduce vacuum errcontext ...)

2020-06-22 Thread Andres Freund
Hi, On 2020-06-22 15:43:11 -0500, Justin Pryzby wrote: > On Mon, Jun 22, 2020 at 01:09:39PM -0700, Andres Freund wrote: > > I'm also uncomfortable with the approach of just copying all of > > LVRelStats in several places: > > > > > /* > > > @@ -1580,9 +1648,15 @@ lazy_vacuum_page(Relation

Re: Backpatch b61d161c14 (Introduce vacuum errcontext ...)

2020-06-22 Thread Justin Pryzby
On Mon, Jun 22, 2020 at 01:09:39PM -0700, Andres Freund wrote: > On 2020-06-22 10:35:47 +0530, Amit Kapila wrote: > > I propose to backpatch b61d161c14 [1] (Introduce vacuum errcontext to > > display additional information.). ... > I think having the additional information in the back branches