On 2024/09/16 23:30, Anton A. Melnikov wrote:
+1
This idea seems quite tenable to me.

There is a small clarification. Now if there were no skipped restartpoints then
restartpoints_done will be equal to restartpoints_timed + restartpoints_req.
Similar for checkpoints.
So i tried to introduce num_done counter for checkpoints in the patch attached.

Thanks for the patch! I believe this change is targeted for v18. For v17, 
however,
we should update the description of num_timed in the documentation. Thought?
Here's a suggestion:

"Number of scheduled checkpoints due to timeout. Note that checkpoints may be
skipped if the server has been idle since the last one, and this value counts
both completed and skipped checkpoints."

Regarding the patch:
                                if (do_restartpoint)
                                        
PendingCheckpointerStats.restartpoints_performed++;
+                               else
+                                       
PendingCheckpointerStats.num_performed++;

I expected the counter not to be incremented when a checkpoint is skipped,
but in this code, when a checkpoint is skipped, ckpt_performed is set to true,
triggering the counter increment. This seems wrong.


I'm not sure should we include testing for the case when num_done is less than
num_timed + num_requested to the regress tests. I haven't been able to get it 
in a short time yet.

I'm not sure if that test is really necessary...

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION



Reply via email to