Horton

Re: index prefetching

Dilip Kumar <dilipbalaut@gmail.com>

View thread
From: Dilip Kumar <dilipbalaut@gmail.com>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: Konstantin Knizhnik <knizhnik@garret.ru>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2024-01-25T10:45:31Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. aio: io_uring: Trigger async processing for large IOs

  2. read stream: Split decision about look ahead for AIO and combining

  3. read_stream: Only increase read-ahead distance when waiting for IO

  4. read_stream: Prevent distance from decaying too quickly

  5. Reduce ExecSeqScan* code size using pg_assume()

  6. Fix rare bug in read_stream.c's split IO handling.

  7. Fix multiranges to behave more like dependent types.

  8. Add EXPLAIN (MEMORY) to report planner memory consumption

  9. Optimize nbtree backward scan boundary cases.

  10. Increment xactCompletionCount during subtransaction abort.

  11. Add nbtree Valgrind buffer lock checks.

  12. Add nbtree high key "continuescan" optimization.

  13. Reduce pinning and buffer content locking for btree scans.

  14. Teach btree to handle ScalarArrayOpExpr quals natively.

On Wed, Jan 24, 2024 at 11:43 PM Tomas Vondra
<tomas.vondra@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