From: | Josef Šimánek <josef(dot)simanek(at)gmail(dot)com> |
---|---|
To: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
Cc: | Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Justin Pryzby <pryzby(at)telsasoft(dot)com> |
Subject: | Re: Improvements and additions to COPY progress reporting |
Date: | 2021-02-20 13:17:30 |
Message-ID: | CAFp7QwopPY6hHOkrJAa=Av+uuoATM2VERkMe4T6H_4QEDB+mFA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
so 20. 2. 2021 v 7:09 odesílatel Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> napsal:
>
> On Fri, Feb 19, 2021 at 2:34 AM Tomas Vondra
> <tomas(dot)vondra(at)enterprisedb(dot)com> wrote:
> > > On Mon, 15 Feb 2021 at 17:07, Tomas Vondra
> > > <tomas(dot)vondra(at)enterprisedb(dot)com> wrote:
> > >>
> > >> - The blocks in copyfrom.cc/copyto.c should be reworked - I don't think
> > >> we do this in our codebase.
> > >
> > > I saw this being used in (re)index progress reporting, that's where I
> > > took inspiration from. It has been fixed in the attached version.
> > >
> >
> > Hmmm, good point. I haven't looked at the other places reporting
> > progress and I only ever saw this pattern in old code. I kinda dislike
> > these blocks, but admittedly that's rather subjective view. So if other
> > similar places do this when reporting progress, this probably should
> > too. What's your opinion on this?
>
> Actually in the code base the style of that variable declaration and
> usage of pgstat_progress_update_multi_param is a mix. For instance, in
> lazy_scan_heap, ReindexRelationConcurrently, the variables are
> declared at the start of the function. And in _bt_spools_heapscan,
> index_build, validate_index, perform_base_backup, the variables are
> declared within a separate block.
>
> IMO, we can have the arrays declared at the start of the functions
> i.e. the way it's done in v8-0001, because we can extend them for
> reporting some other parameter(maybe in future).
>
> > >> - I fir the "io_target" name misleading, because in some cases it's
> > >> actually the *source*.
> > >
> > > Yes, I was also not quite happy with this, but couldn't find a better
> > > one at the point of writing the initial patchset. Would
> > > "io_operations", "io_port", "operates_through" or "through" maybe be
> > > better?
> > >
> >
> > No idea. Let's see if someone has a better proposal ...
>
> For COPY TO the name "source_type" column and for COPY FROM the name
> "destination_type" makes sense. To have a combined column name for
> both, how about naming that column as "io_type"?
+1 on "io_type", that is my best candidate as well
> With Regards,
> Bharath Rupireddy.
> EnterpriseDB: http://www.enterprisedb.com
From | Date | Subject | |
---|---|---|---|
Next Message | Álvaro Herrera | 2021-02-20 15:25:58 | Re: some pointless HeapTupleHeaderIndicatesMovedPartitions calls |
Previous Message | Michael Paquier | 2021-02-20 12:58:04 | Re: some pointless HeapTupleHeaderIndicatesMovedPartitions calls |