Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. pg_test_timing: store timing deltas in int64

  2. pg_test_timing: fix unit in backward-clock warning

  1. pg_test_timing: fix unit typo and widen diff type

    Chao Li <li.evan.chao@gmail.com> — 2026-04-02T03:09:23Z

    Hi,
    
    This morning, as part of my usual routine, I synced the master branch and read through the recent commits. While reading 82c0cb4e672, I noticed a mistake in an error message. The relevant code is like:
    ```
    diff = INSTR_TIME_GET_NANOSEC(diff_time);
    
    fprintf(stderr, _("Time warp: %d ms\n"), diff);
    ```
    
    Here, “diff" is in nanoseconds, but the error message prints ms as the unit, which is incorrect.
    
    To fix that, I think there are two possible options:
    
    1. Use INSTR_TIME_GET_MILLISEC to get “diff"
    2. Change “ms" to “ns" in the error message
    
    After reading through the whole file, I think option 2 is the right fix. While doing that, I also noticed another issue.
    
    “diff" is currently defined as int32. Although one might think that is enough for a time delta, I believe it should be int64 for two reasons:
    
    * INSTR_TIME_GET_NANOSEC() explicitly returns int64:
    ```
    #define INSTR_TIME_GET_NANOSEC(t) \
                   ((int64) (t).ticks)
    ```
    
    * The current code has a sanity check for backward clock drift:
    ```
            /* Did time go backwards? */
            if (unlikely(diff < 0))
            {
                fprintf(stderr, _("Detected clock going backwards in time.\n"));
                fprintf(stderr, _("Time warp: %d ms\n"), diff);
                exit(1);
            }
    ```
    Clock jumping forward is also possible, and a forward jump of about 2.14 seconds would overflow int32 when expressed in nanoseconds, making the value appear negative. In that case, the code could report a “backwards” clock jump when the actual jump was forwards, which would be misleading.
    
    To make the patch easier to process, I split it into two parts: 0001 fixes the unit in the error message, and 0002 changes the type of diff. If this gets accepted, I would be happy to squash them into one commit.
    
    I should also note that although I noticed this while reading 82c0cb4e672, I do not think this was an oversight of that commit. More likely, because clock drift backwards is rare, this issue has simply gone unnoticed for many years.
    
    Best regards,
    --
    Chao Li (Evan)
    HighGo Software Co., Ltd.
    https://www.highgo.com/
    
    
    
    
    
  2. Re: pg_test_timing: fix unit typo and widen diff type

    Lukas Fittl <lukas@fittl.com> — 2026-04-02T04:17:35Z

    Hi Chao,
    
    On Wed, Apr 1, 2026 at 8:10 PM Chao Li <li.evan.chao@gmail.com> wrote:
    >
    > Hi,
    >
    > This morning, as part of my usual routine, I synced the master branch and read through the recent commits. While reading 82c0cb4e672, I noticed a mistake in an error message. The relevant code is like:
    > ```
    > diff = INSTR_TIME_GET_NANOSEC(diff_time);
    >
    > fprintf(stderr, _("Time warp: %d ms\n"), diff);
    > ```
    >
    > Here, “diff" is in nanoseconds, but the error message prints ms as the unit, which is incorrect.
    
    Good catch!
    
    It looks like the use of nanoseconds for "diff" got introduced last
    year in 0b096e379e6f9bd49 (as you note later in the email, today's
    commit didn't actually change that part), CCing Tom and Hannu as
    authors of that earlier change.
    
    That said, its a bit odd that we were using INSTR_TIME_GET_MICROSEC
    there before that earlier commit, but called it "ms" (i.e.
    milliseconds) in the error message.
    
    >
    > To fix that, I think there are two possible options:
    >
    > 1. Use INSTR_TIME_GET_MILLISEC to get “diff"
    > 2. Change “ms" to “ns" in the error message
    >
    > After reading through the whole file, I think option 2 is the right fix. While doing that, I also noticed another issue.
    >
    > “diff" is currently defined as int32. Although one might think that is enough for a time delta, I believe it should be int64 for two reasons:
    >
    > * INSTR_TIME_GET_NANOSEC() explicitly returns int64:
    > ```
    > #define INSTR_TIME_GET_NANOSEC(t) \
    >                ((int64) (t).ticks)
    > ```
    >
    > * The current code has a sanity check for backward clock drift:
    > ```
    >         /* Did time go backwards? */
    >         if (unlikely(diff < 0))
    >         {
    >             fprintf(stderr, _("Detected clock going backwards in time.\n"));
    >             fprintf(stderr, _("Time warp: %d ms\n"), diff);
    >             exit(1);
    >         }
    > ```
    > Clock jumping forward is also possible, and a forward jump of about 2.14 seconds would overflow int32 when expressed in nanoseconds, making the value appear negative. In that case, the code could report a “backwards” clock jump when the actual jump was forwards, which would be misleading.
    
    I agree it doesn't seem sound to use an int32 for storing the result
    of INSTR_TIME_GET_NANOSEC. It looks like we may also need to adjust
    the call to pg_leftmost_one_pos32 though if we actually accept that
    large a "diff" value, as in your patch.
    
    Maybe we should error out if the diff is larger than an int32, noting
    a positive time drift?
    
    Independently of that, its worth noting we could easily emit the diff
    in a larger unit (micro or milliseconds) for easier interpretation, by
    just calling INSTR_TIME_GET_MICROSEC / INSTR_TIME_GET_MILLISEC on the
    "diff_time" again.
    
    Thanks,
    Lukas
    
    
    --
    Lukas Fittl
    
    
    
    
  3. Re: pg_test_timing: fix unit typo and widen diff type

    Chao Li <li.evan.chao@gmail.com> — 2026-04-02T07:28:48Z

    
    > On Apr 2, 2026, at 12:17, Lukas Fittl <lukas@fittl.com> wrote:
    > 
    > Hi Chao,
    > 
    > On Wed, Apr 1, 2026 at 8:10 PM Chao Li <li.evan.chao@gmail.com> wrote:
    >> 
    >> Hi,
    >> 
    >> This morning, as part of my usual routine, I synced the master branch and read through the recent commits. While reading 82c0cb4e672, I noticed a mistake in an error message. The relevant code is like:
    >> ```
    >> diff = INSTR_TIME_GET_NANOSEC(diff_time);
    >> 
    >> fprintf(stderr, _("Time warp: %d ms\n"), diff);
    >> ```
    >> 
    >> Here, “diff" is in nanoseconds, but the error message prints ms as the unit, which is incorrect.
    > 
    > Good catch!
    > 
    > It looks like the use of nanoseconds for "diff" got introduced last
    > year in 0b096e379e6f9bd49 (as you note later in the email, today's
    > commit didn't actually change that part), CCing Tom and Hannu as
    > authors of that earlier change.
    > 
    > That said, its a bit odd that we were using INSTR_TIME_GET_MICROSEC
    > there before that earlier commit, but called it "ms" (i.e.
    > milliseconds) in the error message.
    > 
    >> 
    >> To fix that, I think there are two possible options:
    >> 
    >> 1. Use INSTR_TIME_GET_MILLISEC to get “diff"
    >> 2. Change “ms" to “ns" in the error message
    >> 
    >> After reading through the whole file, I think option 2 is the right fix. While doing that, I also noticed another issue.
    >> 
    >> “diff" is currently defined as int32. Although one might think that is enough for a time delta, I believe it should be int64 for two reasons:
    >> 
    >> * INSTR_TIME_GET_NANOSEC() explicitly returns int64:
    >> ```
    >> #define INSTR_TIME_GET_NANOSEC(t) \
    >>               ((int64) (t).ticks)
    >> ```
    >> 
    >> * The current code has a sanity check for backward clock drift:
    >> ```
    >>        /* Did time go backwards? */
    >>        if (unlikely(diff < 0))
    >>        {
    >>            fprintf(stderr, _("Detected clock going backwards in time.\n"));
    >>            fprintf(stderr, _("Time warp: %d ms\n"), diff);
    >>            exit(1);
    >>        }
    >> ```
    >> Clock jumping forward is also possible, and a forward jump of about 2.14 seconds would overflow int32 when expressed in nanoseconds, making the value appear negative. In that case, the code could report a “backwards” clock jump when the actual jump was forwards, which would be misleading.
    > 
    > I agree it doesn't seem sound to use an int32 for storing the result
    > of INSTR_TIME_GET_NANOSEC. It looks like we may also need to adjust
    > the call to pg_leftmost_one_pos32 though if we actually accept that
    > large a "diff" value, as in your patch.
    
    You are right. Changed to pg_leftmost_one_pos64 in v2.
    
    > 
    > Maybe we should error out if the diff is larger than an int32, noting
    > a positive time drift?
    
    I agree we should warn/fail upon clock forwards drift. But I doubt int32 is too big (~2.14 seconds), I consider 1 second could be a too big threshold. Let’s wait for more voices on this.
    
    > 
    > Independently of that, its worth noting we could easily emit the diff
    > in a larger unit (micro or milliseconds) for easier interpretation, by
    > just calling INSTR_TIME_GET_MICROSEC / INSTR_TIME_GET_MILLISEC on the
    > "diff_time" again.
    > 
    
    Given the error should rarely happen, I personally feel that might not be super helpful. Also, if the drift is just beyond the threshold, bumping to microsecond or millisecond might print just 0.
    
    PFA v2 - updated 0002 for pg_leftmost_one_pos64.
    
    Best regards,
    --
    Chao Li (Evan)
    HighGo Software Co., Ltd.
    https://www.highgo.com/
    
    
    
    
    
  4. Re: pg_test_timing: fix unit typo and widen diff type

    Chao Li <li.evan.chao@gmail.com> — 2026-04-08T04:06:07Z

    
    > On Apr 2, 2026, at 15:28, Chao Li <li.evan.chao@gmail.com> wrote:
    > 
    > 
    > 
    >> On Apr 2, 2026, at 12:17, Lukas Fittl <lukas@fittl.com> wrote:
    >> 
    >> Hi Chao,
    >> 
    >> On Wed, Apr 1, 2026 at 8:10 PM Chao Li <li.evan.chao@gmail.com> wrote:
    >>> 
    >>> Hi,
    >>> 
    >>> This morning, as part of my usual routine, I synced the master branch and read through the recent commits. While reading 82c0cb4e672, I noticed a mistake in an error message. The relevant code is like:
    >>> ```
    >>> diff = INSTR_TIME_GET_NANOSEC(diff_time);
    >>> 
    >>> fprintf(stderr, _("Time warp: %d ms\n"), diff);
    >>> ```
    >>> 
    >>> Here, “diff" is in nanoseconds, but the error message prints ms as the unit, which is incorrect.
    >> 
    >> Good catch!
    >> 
    >> It looks like the use of nanoseconds for "diff" got introduced last
    >> year in 0b096e379e6f9bd49 (as you note later in the email, today's
    >> commit didn't actually change that part), CCing Tom and Hannu as
    >> authors of that earlier change.
    >> 
    >> That said, its a bit odd that we were using INSTR_TIME_GET_MICROSEC
    >> there before that earlier commit, but called it "ms" (i.e.
    >> milliseconds) in the error message.
    >> 
    >>> 
    >>> To fix that, I think there are two possible options:
    >>> 
    >>> 1. Use INSTR_TIME_GET_MILLISEC to get “diff"
    >>> 2. Change “ms" to “ns" in the error message
    >>> 
    >>> After reading through the whole file, I think option 2 is the right fix. While doing that, I also noticed another issue.
    >>> 
    >>> “diff" is currently defined as int32. Although one might think that is enough for a time delta, I believe it should be int64 for two reasons:
    >>> 
    >>> * INSTR_TIME_GET_NANOSEC() explicitly returns int64:
    >>> ```
    >>> #define INSTR_TIME_GET_NANOSEC(t) \
    >>>              ((int64) (t).ticks)
    >>> ```
    >>> 
    >>> * The current code has a sanity check for backward clock drift:
    >>> ```
    >>>       /* Did time go backwards? */
    >>>       if (unlikely(diff < 0))
    >>>       {
    >>>           fprintf(stderr, _("Detected clock going backwards in time.\n"));
    >>>           fprintf(stderr, _("Time warp: %d ms\n"), diff);
    >>>           exit(1);
    >>>       }
    >>> ```
    >>> Clock jumping forward is also possible, and a forward jump of about 2.14 seconds would overflow int32 when expressed in nanoseconds, making the value appear negative. In that case, the code could report a “backwards” clock jump when the actual jump was forwards, which would be misleading.
    >> 
    >> I agree it doesn't seem sound to use an int32 for storing the result
    >> of INSTR_TIME_GET_NANOSEC. It looks like we may also need to adjust
    >> the call to pg_leftmost_one_pos32 though if we actually accept that
    >> large a "diff" value, as in your patch.
    > 
    > You are right. Changed to pg_leftmost_one_pos64 in v2.
    > 
    >> 
    >> Maybe we should error out if the diff is larger than an int32, noting
    >> a positive time drift?
    > 
    > I agree we should warn/fail upon clock forwards drift. But I doubt int32 is too big (~2.14 seconds), I consider 1 second could be a too big threshold. Let’s wait for more voices on this.
    > 
    >> 
    >> Independently of that, its worth noting we could easily emit the diff
    >> in a larger unit (micro or milliseconds) for easier interpretation, by
    >> just calling INSTR_TIME_GET_MICROSEC / INSTR_TIME_GET_MILLISEC on the
    >> "diff_time" again.
    >> 
    > 
    > Given the error should rarely happen, I personally feel that might not be super helpful. Also, if the drift is just beyond the threshold, bumping to microsecond or millisecond might print just 0.
    > 
    > PFA v2 - updated 0002 for pg_leftmost_one_pos64.
    > 
    > Best regards,
    > --
    > Chao Li (Evan)
    > HighGo Software Co., Ltd.
    > https://www.highgo.com/
    > 
    > 
    > 
    > 
    > <v2-0001-pg_test_timing-fix-unit-in-backward-clock-warning.patch><v2-0002-pg_test_timing-use-int64-for-largest-observed-tim.patch>
    
    PFA v3: Fixed a CI failure.
    
    Best regards,
    --
    Chao Li (Evan)
    HighGo Software Co., Ltd.
    https://www.highgo.com/
    
    
    
    
    
  5. Re:Re: pg_test_timing: fix unit typo and widen diff type

    Xiaopeng Wang <wxp_728@163.com> — 2026-04-09T06:12:39Z

    At 2026-04-08 12:06:07, "Chao Li" <li.evan.chao@gmail.com> wrote:
    >
    >
    >> On Apr 2, 2026, at 15:28, Chao Li <li.evan.chao@gmail.com> wrote:
    >> 
    >> 
    >> 
    >>> On Apr 2, 2026, at 12:17, Lukas Fittl <lukas@fittl.com> wrote:
    >>> 
    >>> Hi Chao,
    >>> 
    >>> On Wed, Apr 1, 2026 at 8:10 PM Chao Li <li.evan.chao@gmail.com> wrote:
    >>>> 
    >>>> Hi,
    >>>> 
    >>>> This morning, as part of my usual routine, I synced the master branch and read through the recent commits. While reading 82c0cb4e672, I noticed a mistake in an error message. The relevant code is like:
    >>>> ```
    >>>> diff = INSTR_TIME_GET_NANOSEC(diff_time);
    >>>> 
    >>>> fprintf(stderr, _("Time warp: %d ms\n"), diff);
    >>>> ```
    >>>> 
    >>>> Here, “diff" is in nanoseconds, but the error message prints ms as the unit, which is incorrect.
    >>> 
    >>> Good catch!
    >>> 
    >>> It looks like the use of nanoseconds for "diff" got introduced last
    >>> year in 0b096e379e6f9bd49 (as you note later in the email, today's
    >>> commit didn't actually change that part), CCing Tom and Hannu as
    >>> authors of that earlier change.
    >>> 
    >>> That said, its a bit odd that we were using INSTR_TIME_GET_MICROSEC
    >>> there before that earlier commit, but called it "ms" (i.e.
    >>> milliseconds) in the error message.
    >>> 
    >>>> 
    >>>> To fix that, I think there are two possible options:
    >>>> 
    >>>> 1. Use INSTR_TIME_GET_MILLISEC to get “diff"
    >>>> 2. Change “ms" to “ns" in the error message
    >>>> 
    >>>> After reading through the whole file, I think option 2 is the right fix. While doing that, I also noticed another issue.
    >>>> 
    >>>> “diff" is currently defined as int32. Although one might think that is enough for a time delta, I believe it should be int64 for two reasons:
    >>>> 
    >>>> * INSTR_TIME_GET_NANOSEC() explicitly returns int64:
    >>>> ```
    >>>> #define INSTR_TIME_GET_NANOSEC(t) \
    >>>>              ((int64) (t).ticks)
    >>>> ```
    >>>> 
    >>>> * The current code has a sanity check for backward clock drift:
    >>>> ```
    >>>>       /* Did time go backwards? */
    >>>>       if (unlikely(diff < 0))
    >>>>       {
    >>>>           fprintf(stderr, _("Detected clock going backwards in time.\n"));
    >>>>           fprintf(stderr, _("Time warp: %d ms\n"), diff);
    >>>>           exit(1);
    >>>>       }
    >>>> ```
    >>>> Clock jumping forward is also possible, and a forward jump of about 2.14 seconds would overflow int32 when expressed in nanoseconds, making the value appear negative. In that case, the code could report a “backwards” clock jump when the actual jump was forwards, which would be misleading.
    >>> 
    >>> I agree it doesn't seem sound to use an int32 for storing the result
    >>> of INSTR_TIME_GET_NANOSEC. It looks like we may also need to adjust
    >>> the call to pg_leftmost_one_pos32 though if we actually accept that
    >>> large a "diff" value, as in your patch.
    >> 
    >> You are right. Changed to pg_leftmost_one_pos64 in v2.
    >> 
    >>> 
    >>> Maybe we should error out if the diff is larger than an int32, noting
    >>> a positive time drift?
    >> 
    >> I agree we should warn/fail upon clock forwards drift. But I doubt int32 is too big (~2.14 seconds), I consider 1 second could be a too big threshold. Let’s wait for more voices on this.
    >> 
    >>> 
    >>> Independently of that, its worth noting we could easily emit the diff
    >>> in a larger unit (micro or milliseconds) for easier interpretation, by
    >>> just calling INSTR_TIME_GET_MICROSEC / INSTR_TIME_GET_MILLISEC on the
    >>> "diff_time" again.
    >>> 
    >> 
    >> Given the error should rarely happen, I personally feel that might not be super helpful. Also, if the drift is just beyond the threshold, bumping to microsecond or millisecond might print just 0.
    >> 
    >> PFA v2 - updated 0002 for pg_leftmost_one_pos64.
    >> 
    >> Best regards,
    >> --
    >> Chao Li (Evan)
    >> HighGo Software Co., Ltd.
    >> https://www.highgo.com/
    >> 
    >> 
    >> 
    >> 
    >> <v2-0001-pg_test_timing-fix-unit-in-backward-clock-warning.patch><v2-0002-pg_test_timing-use-int64-for-largest-observed-tim.patch>
    >
    >PFA v3: Fixed a CI failure.
    >
    >Best regards,
    >--
    >Chao Li (Evan)
    >HighGo Software Co., Ltd.
    >https://www.highgo.com/
    >
    >
    Hi Chao, Lukas,
    Just finished reviewing the v3 patches - looks good to me. The pg_leftmost_one_pos64 fix is spot on, and using INT64_FORMAT for the error message is the right approach.
    Nice catch on the unit mismatch and the int32 overflow issue. Both patches are ready to go from my perspective.
    Cheers,
    Xiaopeng Wang
    
    
    
    
  6. Re: Re: pg_test_timing: fix unit typo and widen diff type

    Fujii Masao <masao.fujii@gmail.com> — 2026-04-22T07:04:16Z

    On Thu, Apr 9, 2026 at 3:13 PM wang.xiao.peng <wxp_728@163.com> wrote:
    > Just finished reviewing the v3 patches - looks good to me. The pg_leftmost_one_pos64 fix is spot on, and using INT64_FORMAT for the error message is the right approach.
    
    Since pg_leftmost_one_pos64() can return up to 63, should the size of
    histogram[] be changed from 32 to 64? If we want to display the full
    histogram[] in the output, max_bit in output() would also need to be set to 63.
    Alternatively, it may be fine to keep max_bit = 31 and show only
    the first 32 histogram entries.
    
    Patch 0001 looks good to me.
    
    Regards,
    
    -- 
    Fujii Masao
    
    
    
    
  7. Re: pg_test_timing: fix unit typo and widen diff type

    Chao Li <li.evan.chao@gmail.com> — 2026-04-22T08:57:32Z

    
    > On Apr 22, 2026, at 15:04, Fujii Masao <masao.fujii@gmail.com> wrote:
    > 
    > On Thu, Apr 9, 2026 at 3:13 PM wang.xiao.peng <wxp_728@163.com> wrote:
    >> Just finished reviewing the v3 patches - looks good to me. The pg_leftmost_one_pos64 fix is spot on, and using INT64_FORMAT for the error message is the right approach.
    > 
    > Since pg_leftmost_one_pos64() can return up to 63, should the size of
    > histogram[] be changed from 32 to 64? If we want to display the full
    > histogram[] in the output, max_bit in output() would also need to be set to 63.
    > Alternatively, it may be fine to keep max_bit = 31 and show only
    > the first 32 histogram entries.
    
    I revisited this patch and think it makes sense to extend histogram[] to 64 entries.
    
    In practice, the higher buckets will almost always remain zero, and output() already omits trailing histogram entries with zero counts, so this change is unlikely to affect the actual runtime output in normal cases.
    
    Accordingly, I also adjusted the minimum width of the first column, since the maximum int64 value has 19 digits, and updated the printf format accordingly.
    
    > 
    > Patch 0001 looks good to me.
    
    Cool.
    
    PFA v4:
    
    * 0001 unchanged from v3.
    * 0002 changed size of histogram[] 64.
    
    Best regards,
    --
    Chao Li (Evan)
    HighGo Software Co., Ltd.
    https://www.highgo.com/
    
    
    
    
    
  8. Re: pg_test_timing: fix unit typo and widen diff type

    Fujii Masao <masao.fujii@gmail.com> — 2026-04-22T16:11:53Z

    On Wed, Apr 22, 2026 at 5:58 PM Chao Li <li.evan.chao@gmail.com> wrote:
    > PFA v4:
    >
    > * 0001 unchanged from v3.
    > * 0002 changed size of histogram[] 64.
    
    Thanks for updating the patch!
    
    Since patch 0001 is a bug fix, it should be backpatched to the supported stable
    branches. However, in v19 the unit should be changed like s/ms/ns, while in v18
    and earlier s/ms/us, since those older branches report the diff in microseconds.
    Right?
    
    Patch 0002 looks more like an improvement than a bug fix, so we should probably
    wait for the next CommitFest before committing it. Thoughts?
    
    Regards,
    
    
    -- 
    Fujii Masao
    
    
    
    
  9. Re: pg_test_timing: fix unit typo and widen diff type

    Chao Li <li.evan.chao@gmail.com> — 2026-04-23T02:02:31Z

    
    > On Apr 23, 2026, at 00:11, Fujii Masao <masao.fujii@gmail.com> wrote:
    > 
    > On Wed, Apr 22, 2026 at 5:58 PM Chao Li <li.evan.chao@gmail.com> wrote:
    >> PFA v4:
    >> 
    >> * 0001 unchanged from v3.
    >> * 0002 changed size of histogram[] 64.
    > 
    > Thanks for updating the patch!
    > 
    > Since patch 0001 is a bug fix, it should be backpatched to the supported stable
    > branches. However, in v19 the unit should be changed like s/ms/ns, while in v18
    > and earlier s/ms/us, since those older branches report the diff in microseconds.
    > Right?
    
    I just checked old branches. Looks like 0001 can be back-patched down to v10. Yes, pre-19, all branches use microsecond, so the back-patch should change “ms” to “us”.
    
    I have helped create back-patch diff files as attached:
    
    * nocfbot.v10-v13.pg_test_timing-backpatch.diff for v10-v13
    * nocfbot.v14-v15.pg_test_timing-backpatch.diff for v14-v15
    * nocfbot.v16-v18.pg_test_timing-backpatch.diff for v16-v18
    
    For v16 to v18, we can make a tiny improvement by replacing “1e9” with a constant macro NS_PER_S. This change has been included in the diff.
    
    I have built and tested on all branches.
    
    > 
    > Patch 0002 looks more like an improvement than a bug fix, so we should probably
    > wait for the next CommitFest before committing it. Thoughts?
    > 
    
    I see 0002 a bit differently. In v19, the unit changed from microseconds to nanoseconds, which introduced a potential overflow: nanoseconds require int64, but the local variable remained int32. So I think this is actually a v19-only bug.
    
    PFA v5: 0001 replaced “1e9” with NS_PER_S; 0002 unchanged.
    
    Best regards,
    --
    Chao Li (Evan)
    HighGo Software Co., Ltd.
    https://www.highgo.com/
    
    
    
    
    
  10. Re: pg_test_timing: fix unit typo and widen diff type

    Fujii Masao <masao.fujii@gmail.com> — 2026-04-24T03:15:50Z

    On Thu, Apr 23, 2026 at 11:03 AM Chao Li <li.evan.chao@gmail.com> wrote:
    > I just checked old branches. Looks like 0001 can be back-patched down to v10. Yes, pre-19, all branches use microsecond, so the back-patch should change “ms” to “us”.
    
    Thanks! So I've pushed this unit change to master and backpatched it to
    all supported branches.
    
    
    > For v16 to v18, we can make a tiny improvement by replacing “1e9” with a constant macro NS_PER_S. This change has been included in the diff.
    
    This change looks good to me. However, we should generally keep changes to
    old stable branches to the minimum required, so I applied this change only
    to master with 0002 patch.
    
    
    > I see 0002 a bit differently. In v19, the unit changed from microseconds to nanoseconds, which introduced a potential overflow: nanoseconds require int64, but the local variable remained int32. So I think this is actually a v19-only bug.
    
    OK, I've pushed 0002 patch. Thanks!
    
    Regards,
    
    -- 
    Fujii Masao
    
    
    
    
  11. Re: pg_test_timing: fix unit typo and widen diff type

    Chao Li <li.evan.chao@gmail.com> — 2026-04-24T03:30:10Z

    
    > On Apr 24, 2026, at 11:15, Fujii Masao <masao.fujii@gmail.com> wrote:
    > 
    > On Thu, Apr 23, 2026 at 11:03 AM Chao Li <li.evan.chao@gmail.com> wrote:
    >> I just checked old branches. Looks like 0001 can be back-patched down to v10. Yes, pre-19, all branches use microsecond, so the back-patch should change “ms” to “us”.
    > 
    > Thanks! So I've pushed this unit change to master and backpatched it to
    > all supported branches.
    > 
    > 
    >> For v16 to v18, we can make a tiny improvement by replacing “1e9” with a constant macro NS_PER_S. This change has been included in the diff.
    > 
    > This change looks good to me. However, we should generally keep changes to
    > old stable branches to the minimum required, so I applied this change only
    > to master with 0002 patch.
    > 
    > 
    >> I see 0002 a bit differently. In v19, the unit changed from microseconds to nanoseconds, which introduced a potential overflow: nanoseconds require int64, but the local variable remained int32. So I think this is actually a v19-only bug.
    > 
    > OK, I've pushed 0002 patch. Thanks!
    > 
    > Regards,
    > 
    > -- 
    > Fujii Masao
    
    Hi Fujii-san, thank you very much for pushing.
    
    Best regards,
    --
    Chao Li (Evan)
    HighGo Software Co., Ltd.
    https://www.highgo.com/