Hello Kyotaro-san,
28.04.2023 04:41, Kyotaro Horiguchi wrote:
> Nonetheless, I'm a bit concerned about making a direct call to
> pgstat_clear_snapshot() from the assign callback, it might be fine for
> now, but I worry that it could an issue later on.
>
> So, how about just settin a trigger that causes a snapshot clearing
> prior to the next use, like the attached?
I'm agree with the change — it's still rather simple and fixes the issue.
Maybe you would also find appropriate to update the comment for
pgstat_get_stat_snapshot_timestamp().
Perhaps add something like?:
Note that the snapshot may be cleared here, if requested.
> As for the test, I can't come up with a better one, but I think the
> comment should explain its intention more clearly.
I'd hesitated to mention a crash in the comment because I've seen
assertion failures only (and no crash with a non-assert build), so I
thought that the test should illustrate the new behavior in the first place.
But I'm not opposed to your comment change, maybe it's more prominent indeed.
Thank you for the fix!
Best regards,
Alexander
