In __reconcile() WT tracks, among other things, the time spent building the page image. If this time exceeds the maximum seen, WT updates the statistic that tracks "maximum milliseconds spent in building disk image in a reconciliation." The code for this appears inconsistent/incorrect. This ticket should evaluate whether the code is correct and either fix it or refactor/comment it so it is more clear what it doing and why it is correct.

Here are the code snippets from that function where we do this work:

    /* Only update if we are in the first entry into eviction. */
    if (!session->evict_timeline.reentry_hs_eviction)
        session->reconcile_timeline.image_build_start = __wt_clock(session);

    ...

    if (!session->evict_timeline.reentry_hs_eviction)
        session->reconcile_timeline.image_build_finish = __wt_clock(session);

    ...

    rec_img_build = WT_CLOCKDIFF_MS(session->reconcile_timeline.image_build_finish,
      session->reconcile_timeline.image_build_start);

    ...

    if (rec_img_build > conn->rec_maximum_image_build_milliseconds)
        conn->rec_maximum_image_build_milliseconds = rec_img_build;

In brief, we conditionally get timestamps before and after building the page image. Then we unconditionally compute the difference of these timestamps and update the global statistic.

This feels incorrect, although I haven't chased through all the relevant code. My concerns are:

• Why does the fact that we are reentering eviction for the history store, indicated by session->evict_timeline.reentry_hs_eviction, mean we don't want to time this reconciliation?
• If we don't want to track the time of this reconciliation it is surprising that we try to update the global statistic.
• The reentry_hs_eviction flag seems to only get set during eviction. So it's value will be undefined on checkpoint evictions.