Add last_fork_start_time to INFO STATS

[enjoy-binbin]

Sometimes we want to know the last fork start time, this can help
user to analyze the stability since right now fork is still
a costly operation and user can monitor forks by drawing graphs
based on these fork fields.

fork is still a very resource-intensive process, so it makes sense
to record the time each fork is triggered. Adding a last_fork_start_time
field in INFO STATS, to record the last fork start time in us.

[enjoy-binbin]

Currently the time is, a successful fork, when it start, is recorded. We can also record the last failed fork time if needed.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.65%. Comparing base (789a73b) to head (d914f67).
Report is 112 commits behind head on unstable.

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable     #978      +/-   ##
============================================
- Coverage     70.66%   70.65%   -0.02%     
============================================
  Files           114      114              
  Lines         63150    63152       +2     
============================================
- Hits          44624    44619       -5     
- Misses        18526    18533       +7     

Files with missing lines	Coverage Δ
src/server.c	87.71% <100.00%> (+<0.01%)	⬆️
src/server.h	100.00% <ø> (ø)

... and 11 files with indirect coverage changes

[xbasel]

Were these config files and text file committed by accident?

[enjoy-binbin]

Were these config files and text file committed by accident?

yes.. sorry for that, i did not check after push

[xbasel]

Need also to update https://valkey.io/commands/info/

[hwware]

This information looks like helpful.

[hwware]

@valkey-io/core-team Please vote if you are comfortable with this field, Thanks

[madolson]

@hwware @enjoy-binbin I suppose the counter question I have is what is the specific use case here when we have the logs for when it actually started?

[soloestoy]

i'm ok with the new metric, and is it better to add the unit for it?

[madolson]

Not specifically related to this PR, but this info field is confusing. We thought it was how long the fork latest, but apparently it just covers the system call time. This made the info name you proposed a bit odd since it also used fork.

[enjoy-binbin]

yes, it is indeed confusing at the first time. A field show how long the fork latest also will be helpful.

[enjoy-binbin]

I suppose the counter question I have is what is the specific use case here when we have the logs for when it actually started?

We shouldn't recommend relying on logs, should we? Getting from INFO should be easy for clients.

[madolson]

We shouldn't recommend relying on logs, should we? Getting from INFO should be easy for clients.

My concern was that if someone was concerned about a latency event that was 5 minutes ago, and there was a last fork time that was 180 seconds ago, that doesn't really rule out the possibility of a fork that was 5 minutes ago. At some point they will have to rely on logs for anything historical. Adding more info fields just means it's more complex for new operators to figure out what they should be looking at.

[enjoy-binbin]

My concern was that if someone was concerned about a latency event that was 5 minutes ago, and there was a last fork time that was 180 seconds ago, that doesn't really rule out the possibility of a fork that was 5 minutes ago. At some point they will have to rely on logs for anything historical. Adding more info fields just means it's more complex for new operators to figure out what they should be looking at.

It is true. I don't have a good argument about it, if people care the latency, they can look at the LATENCY LATEST command or the logs to figure it out. From my point, this field is more suitable for making graphs. For example, if we record the time of each fork and draw a graph, combined with other stats, we can better see the impact of fork/COW, etc.