On Sat, Feb 23, 2019 at 10:28 PM Haribabu Kommi <kommi.harib...@gmail.com> wrote: > > > On Thu, Feb 14, 2019 at 9:17 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: >> >> On Wed, Feb 13, 2019 at 9:32 PM Haribabu Kommi <kommi.harib...@gmail.com> >> wrote: >> > >> > >> > On Sat, Feb 9, 2019 at 11:47 PM Masahiko Sawada <sawada.m...@gmail.com> >> > wrote: >> >> >> >> On Tue, Feb 5, 2019 at 12:14 PM Haribabu Kommi <kommi.harib...@gmail.com> >> >> wrote: >> >> > >> >> > >> >> > On Fri, Feb 1, 2019 at 8:19 AM Masahiko Sawada <sawada.m...@gmail.com> >> >> > wrote: >> >> >> >> >> >> >> >> >> The passing stats = NULL to amvacuumcleanup and ambulkdelete means the >> >> >> first time execution. For example, btvacuumcleanup skips cleanup if >> >> >> it's not NULL.In the normal vacuum we pass NULL to ambulkdelete or >> >> >> amvacuumcleanup when the first time calling. And they store the result >> >> >> stats to the memory allocated int the local memory. Therefore in the >> >> >> parallel vacuum I think that both worker and leader need to move it to >> >> >> the shared memory and mark it as updated as different worker could >> >> >> vacuum different indexes at the next time. >> >> > >> >> > >> >> > OK, understood the point. But for btbulkdelete whenever the stats are >> >> > NULL, >> >> > it allocates the memory. So I don't see a problem with it. >> >> > >> >> > The only problem is with btvacuumcleanup, when there are no dead tuples >> >> > present in the table, the btbulkdelete is not called and directly the >> >> > btvacuumcleanup >> >> > is called at the end of vacuum, in that scenario, there is code flow >> >> > difference >> >> > based on the stats. so why can't we use the deadtuples number to >> >> > differentiate >> >> > instead of adding another flag? >> >> >> >> I don't understand your suggestion. What do we compare deadtuples >> >> number to? Could you elaborate on that please? >> > >> > >> > The scenario where the stats should pass NULL to btvacuumcleanup function >> > is >> > when there no dead tuples, I just think that we may use that deadtuples >> > structure >> > to find out whether stats should pass NULL or not while avoiding the extra >> > memcpy. >> > >> >> Thank you for your explanation. I understood. Maybe I'm worrying too >> much but I'm concernced compatibility; currently we handle indexes >> individually. So if there is an index access method whose ambulkdelete >> returns NULL at the first call but returns a palloc'd struct at the >> second time or other, that doesn't work fine. >> >> The documentation says that passed-in 'stats' is NULL at the first >> time call of ambulkdelete but doesn't say about the second time or >> more. Index access methods may expect that the passed-in 'stats' is >> the same as what they has returned last time. So I think to add an >> extra flag for keeping comptibility. > > > I checked some of the ambulkdelete functions, and they are not returning > a NULL data whenever those functions are called. But the palloc'd structure > doesn't get filled with the details. > > IMO, there is no need of any extra code that is required for parallel vacuum > compared to normal vacuum. >
Hmm, I think that this code is necessary to faithfully keep the same index vacuum behavior, especially for communication between lazy vacuum and IAMs, as it is. The IAMs in postgres don't worry about that but other third party AMs might not, and it might be developed in the future. On the other hand, I can understand your concerns; if such IAM is quite rare we might not need to make the code complicated needlessly. I'd like to hear more opinions also from other hackers. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
