On 2020/03/19 1:13, Fujii Masao wrote:
>
>
> On 2020/03/19 0:37, Magnus Hagander wrote:
>> On Wed, Mar 11, 2020 at 5:53 AM Fujii Masao <masao.fujii@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.fujii@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.fujii@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.
>
>>>> Also, should it really be the server version that decides how this
>>>> feature behaves, and not the pg_basebackup version? Given that the
>>>> implementation is entirely in the client, it seems that's more
>>>> logical?
>>>
>>> Yeah, you're right. I changed the patch that way.
>>> Attached is the updated version of the patch.
>>
>> The other changes in it look good!
>
> Thanks for the review!
Pushed! Thanks!
Regards,
--
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters
