> IIUC, if *any* error occurs outside of a deserialize callback, first the "error"
> code will be called, followed by "done" which will then trigger the
> end_extra_stats
> callback that will perform the cleanup.
That is true. But problem is, without an error indication, end_extra_stats(STATS_READ) can only blindly perform cleanup works. As you are providing general purposed callbacks, who knows what scenarios extensions would do, so it’s better to provide more information to callbacks. IMO, letting end_extra_stats() know current situation (normal or failure, even error code) is very meaningful. For example, my extension may want to log “I am forced to quite due to outside error” or “I am done successfully” in end_extra_stats(). Anyway, that’s my own opinion. If you and Michael still consider that’s not a problem, I won’t argue more.
Thanks for explaining. If there is a good use-case to add more detail to the “end” callback, it’s not very obvious yet. Maybe in the future, there will be a convincing reason to do so.
When we hit the clean-up code on any “error”, it should be accompanied by an error log. That is
done in all cases inside pgstat.c, and I expect an extension to log the error as well.
--
Sami Imseih
Amazon Web Services (AWS)
