On Wed, Jan 24, 2024 at 11:43 PM Tomas Vondra <tomas.von...@enterprisedb.com> wrote:
> On 1/22/24 07:35, Konstantin Knizhnik wrote: > > > > On 22/01/2024 1:47 am, Tomas Vondra wrote: > >> h, right. Well, you're right in this case we perhaps could set just one > >> of those flags, but the "purpose" of the two places is quite different. > >> > >> The "prefetch" flag is fully controlled by the prefetcher, and it's up > >> to it to change it (e.g. I can easily imagine some new logic touching > >> setting it to "false" for some reason). > >> > >> The "data" flag is fully controlled by the custom callbacks, so whatever > >> the callback stores, will be there. > >> > >> I don't think it's worth simplifying this. In particular, I don't think > >> the callback can assume it can rely on the "prefetch" flag. > >> > > Why not to add "all_visible" flag to IndexPrefetchEntry ? If will not > > cause any extra space overhead (because of alignment), but allows to > > avoid dynamic memory allocation (not sure if it is critical, but nice to > > avoid if possible). > > > While reading through the first patch I got some questions, I haven't read it complete yet but this is what I got so far. 1. +static bool +IndexPrefetchBlockIsSequential(IndexPrefetch *prefetch, BlockNumber block) +{ + int idx; ... + if (prefetch->blockItems[idx] != (block - i)) + return false; + + /* Don't prefetch if the block happens to be the same. */ + if (prefetch->blockItems[idx] == block) + return false; + } + + /* not sequential, not recently prefetched */ + return true; +} The above function name is BlockIsSequential but at the end, it returns true if it is not sequential, seem like a problem? Also other 2 checks right above the end of the function are returning false if the block is the same or the pattern is sequential I think those are wrong too. 2. I have noticed that the prefetch history is maintained at the backend level, but what if multiple backends are trying to fetch the same heap blocks maybe scanning the same index, so should that be in some shared structure? I haven't thought much deeper about this from the implementation POV, but should we think about it, or it doesn't matter? -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com