Re: Back-patch is necessary? Re: Don't try fetching future segment of a TLI.
On 2020/05/08 14:23, Fujii Masao wrote: On 2020/05/07 17:57, Amit Kapila wrote: On Thu, May 7, 2020 at 12:13 PM Fujii Masao wrote: On 2020/05/02 20:40, Amit Kapila wrote: I don't see any obvious problem with the changed code but we normally don't backpatch performance improvements. I can see that the code change here appears to be straight forward so it might be fine to backpatch this. Have we seen similar reports earlier as well? AFAIK, this functionality is for a long time and if people were facing this on a regular basis then we would have seen such reports multiple times. I mean to say if the chances of this hitting are less then we can even choose not to backpatch this. I found the following two reports. ISTM there are not so many reports... https://www.postgresql.org/message-id/16159-f5a34a3a04dc6...@postgresql.org https://www.postgresql.org/message-id/dd6690b0-ec03-6b3c-6fac-c963f91f87a7%40postgrespro.ru The first seems to be the same where this bug has been fixed. It has been moved to hackers in email [1]. Yes, that's the original report that leaded to the commit. Am, I missing something? Considering it has been encountered by two different people, I think it would not be a bad idea to back-patch this. +1 So I will do the back-patch. Done. Thanks! Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
Re: Back-patch is necessary? Re: Don't try fetching future segment of a TLI.
On 2020/05/07 17:57, Amit Kapila wrote: On Thu, May 7, 2020 at 12:13 PM Fujii Masao wrote: On 2020/05/02 20:40, Amit Kapila wrote: I don't see any obvious problem with the changed code but we normally don't backpatch performance improvements. I can see that the code change here appears to be straight forward so it might be fine to backpatch this. Have we seen similar reports earlier as well? AFAIK, this functionality is for a long time and if people were facing this on a regular basis then we would have seen such reports multiple times. I mean to say if the chances of this hitting are less then we can even choose not to backpatch this. I found the following two reports. ISTM there are not so many reports... https://www.postgresql.org/message-id/16159-f5a34a3a04dc6...@postgresql.org https://www.postgresql.org/message-id/dd6690b0-ec03-6b3c-6fac-c963f91f87a7%40postgrespro.ru The first seems to be the same where this bug has been fixed. It has been moved to hackers in email [1]. Yes, that's the original report that leaded to the commit. Am, I missing something? Considering it has been encountered by two different people, I think it would not be a bad idea to back-patch this. +1 So I will do the back-patch. Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
Re: Back-patch is necessary? Re: Don't try fetching future segment of a TLI.
On Thu, May 7, 2020 at 12:13 PM Fujii Masao wrote: > > On 2020/05/02 20:40, Amit Kapila wrote: > > > > I don't see any obvious problem with the changed code but we normally > > don't backpatch performance improvements. I can see that the code > > change here appears to be straight forward so it might be fine to > > backpatch this. Have we seen similar reports earlier as well? AFAIK, > > this functionality is for a long time and if people were facing this > > on a regular basis then we would have seen such reports multiple > > times. I mean to say if the chances of this hitting are less then we > > can even choose not to backpatch this. > > I found the following two reports. ISTM there are not so many reports... > https://www.postgresql.org/message-id/16159-f5a34a3a04dc6...@postgresql.org > https://www.postgresql.org/message-id/dd6690b0-ec03-6b3c-6fac-c963f91f87a7%40postgrespro.ru > The first seems to be the same where this bug has been fixed. It has been moved to hackers in email [1]. Am, I missing something? Considering it has been encountered by two different people, I think it would not be a bad idea to back-patch this. [1] - https://www.postgresql.org/message-id/20200129.120222.1476610231001551715.horikyota.ntt%40gmail.com -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Re: Back-patch is necessary? Re: Don't try fetching future segment of a TLI.
On 2020/05/02 20:40, Amit Kapila wrote: On Thu, Apr 30, 2020 at 7:46 PM Fujii Masao wrote: On 2020/04/08 1:49, Fujii Masao wrote: On 2020/04/07 20:21, David Steele wrote: On 4/7/20 3:48 AM, Kyotaro Horiguchi wrote: At Tue, 7 Apr 2020 12:15:00 +0900, Fujii Masao wrote in This doesn't seem a bug, so I'm thinking to merge this to next *major* version release, i.e., v13. Not a bug, perhaps, but I think we do consider back-patching performance problems. The rise in S3 usage has just exposed how poorly this performed code in high-latency environments. I understood the situation and am fine to back-patch that. But I'm not sure if it's fair to do that. Maybe we need to hear more opinions about this? OTOH, feature freeze for v13 is today, so what about committing the patch in v13 at first, and then doing the back-patch after hearing opinions and receiving many +1? +1 for commit only v13 today, then back-patch if people wants and/or accepts. Please let me revisit this. Currently Grigory Smolkin, David Steele, Michael Paquier and Pavel Suderevsky agree to the back-patch and there has been no objection to that. So we should do the back-patch? Or does anyone object to that? I don't think that this is a feature bug because archive recovery works fine from a functional perspective without this commit. OTOH, I understand that, without the commit, there is complaint about that archive recovery may be slow unnecessarily when archival storage is located in remote, e.g., Amazon S3 and it takes a long time to fetch the non-existent archive WAL file. So I'm ok to the back-patch unless there is no strong objection to that. I don't see any obvious problem with the changed code but we normally don't backpatch performance improvements. I can see that the code change here appears to be straight forward so it might be fine to backpatch this. Have we seen similar reports earlier as well? AFAIK, this functionality is for a long time and if people were facing this on a regular basis then we would have seen such reports multiple times. I mean to say if the chances of this hitting are less then we can even choose not to backpatch this. I found the following two reports. ISTM there are not so many reports... https://www.postgresql.org/message-id/16159-f5a34a3a04dc6...@postgresql.org https://www.postgresql.org/message-id/dd6690b0-ec03-6b3c-6fac-c963f91f87a7%40postgrespro.ru Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
Re: Back-patch is necessary? Re: Don't try fetching future segment of a TLI.
On Thu, Apr 30, 2020 at 7:46 PM Fujii Masao wrote: > > On 2020/04/08 1:49, Fujii Masao wrote: > > > > > > On 2020/04/07 20:21, David Steele wrote: > >> > >> On 4/7/20 3:48 AM, Kyotaro Horiguchi wrote: > >>> At Tue, 7 Apr 2020 12:15:00 +0900, Fujii Masao > >>> wrote in > >> This doesn't seem a bug, so I'm thinking to merge this to next *major* > >> version release, i.e., v13. > > Not a bug, perhaps, but I think we do consider back-patching > > performance problems. The rise in S3 usage has just exposed how poorly > > this performed code in high-latency environments. > > I understood the situation and am fine to back-patch that. But I'm not > sure > if it's fair to do that. Maybe we need to hear more opinions about > this? > OTOH, feature freeze for v13 is today, so what about committing the > patch > in v13 at first, and then doing the back-patch after hearing opinions > and > receiving many +1? > >>> > >>> +1 for commit only v13 today, then back-patch if people wants and/or > >>> accepts. > > Please let me revisit this. Currently Grigory Smolkin, David Steele, > Michael Paquier and Pavel Suderevsky agree to the back-patch and > there has been no objection to that. So we should do the back-patch? > Or does anyone object to that? > > I don't think that this is a feature bug because archive recovery works > fine from a functional perspective without this commit. OTOH, > I understand that, without the commit, there is complaint about that > archive recovery may be slow unnecessarily when archival storage is > located in remote, e.g., Amazon S3 and it takes a long time to fetch > the non-existent archive WAL file. So I'm ok to the back-patch unless > there is no strong objection to that. > I don't see any obvious problem with the changed code but we normally don't backpatch performance improvements. I can see that the code change here appears to be straight forward so it might be fine to backpatch this. Have we seen similar reports earlier as well? AFAIK, this functionality is for a long time and if people were facing this on a regular basis then we would have seen such reports multiple times. I mean to say if the chances of this hitting are less then we can even choose not to backpatch this. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Back-patch is necessary? Re: Don't try fetching future segment of a TLI.
On 2020/04/08 1:49, Fujii Masao wrote: On 2020/04/07 20:21, David Steele wrote: On 4/7/20 3:48 AM, Kyotaro Horiguchi wrote: At Tue, 7 Apr 2020 12:15:00 +0900, Fujii Masao wrote in This doesn't seem a bug, so I'm thinking to merge this to next *major* version release, i.e., v13. Not a bug, perhaps, but I think we do consider back-patching performance problems. The rise in S3 usage has just exposed how poorly this performed code in high-latency environments. I understood the situation and am fine to back-patch that. But I'm not sure if it's fair to do that. Maybe we need to hear more opinions about this? OTOH, feature freeze for v13 is today, so what about committing the patch in v13 at first, and then doing the back-patch after hearing opinions and receiving many +1? +1 for commit only v13 today, then back-patch if people wants and/or accepts. Please let me revisit this. Currently Grigory Smolkin, David Steele, Michael Paquier and Pavel Suderevsky agree to the back-patch and there has been no objection to that. So we should do the back-patch? Or does anyone object to that? I don't think that this is a feature bug because archive recovery works fine from a functional perspective without this commit. OTOH, I understand that, without the commit, there is complaint about that archive recovery may be slow unnecessarily when archival storage is located in remote, e.g., Amazon S3 and it takes a long time to fetch the non-existent archive WAL file. So I'm ok to the back-patch unless there is no strong objection to that. Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
