On 2020/03/19 1:22, Magnus Hagander wrote:
On Wed, Mar 18, 2020 at 5:14 PM Fujii Masao <masao.fu...@oss.nttdata.com> wrote:



On 2020/03/19 0:37, Magnus Hagander wrote:
On Wed, Mar 11, 2020 at 5:53 AM Fujii Masao <masao.fu...@oss.nttdata.com> wrote:



On 2020/03/11 3:39, Magnus Hagander wrote:
On Tue, Mar 10, 2020 at 6:19 PM Fujii Masao <masao.fu...@oss.nttdata.com> wrote:



On 2020/03/10 22:43, Amit Langote wrote:
On Tue, Mar 10, 2020 at 6:09 PM Fujii Masao <masao.fu...@oss.nttdata.com> wrote:
So, I will make the patch adding support for --no-estimate-size option
in pg_basebackup.

Patch attached.

Like the idea and the patch looks mostly good.

Thanks for reviewing the patch!

+      total size. If the estimation is disabled in
+      <application>pg_basebackup</application>
+      (i.e., <literal>--no-estimate-size</literal> option is specified),
+      this is always <literal>0</literal>.

"always" seems unnecessary.

Fixed.

+        This option prevents the server from estimating the total
+        amount of backup data that will be streamed. In other words,
+        <literal>backup_total</literal> column in the
+        <structname>pg_stat_progress_basebackup</structname>
+        view always indicates <literal>0</literal> if this option is enabled.

Here too.

Fixed.

Attached is the updated version of the patch.

Would it perhaps be better to return NULL instead of 0 in the
statistics view if there is no data?

Did you miss this comment, or not agree? :)

Oh, I forgot to attached the patch... Patch attached.
This patch needs to be applied after applying
add_no_estimate_size_v3.patch.

:)

Hmm. I'm slightly irked by doing the -1 -> NULL conversion in the SQL
view.  I wonder if it might be worth teaching
pg_stat_get_progress_info() about returning NULL?

That's possible by
- adding the boolean array like st_progress_null[PGSTAT_NUM_PROGRESS_PARAM]
   that indicates whether each column is NULL or not, into PgBackendStatus
- extending pgstat_progress_update_param() and 
pgstat_progress_update_multi_param()
   so that they can update the boolean array for NULL
- updating the progress reporting code so that the extended versions of
   function are used

I didn't adopt this idea because it looks a bit overkill for the purpose.
OTOH, this would be good improvement for the progress reporting
infrastructure and I'm fine to implement it.

Regards,

--
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters


Reply via email to