You are viewing a plain text version of this content. The canonical link for it is here.

Posted to issues@kvrocks.apache.org by "infdahai (via GitHub)" <gi...@apache.org> on 2023/05/22 16:03:14 UTC

[GitHub] [incubator-kvrocks] infdahai opened a new issue, #1466: consider to replace system_clock with steady_clock

infdahai opened a new issue, #1466:
URL: https://github.com/apache/incubator-kvrocks/issues/1466

### Search before asking

- [X] I had searched in the [issues](https://github.com/apache/incubator-kvrocks/issues) and found no similar issues.

### Motivation

A repetitive question about #983.

https://github.com/apache/incubator-kvrocks/blob/635dd311e5d07007826ae5338eae4cefb88ccc55/src/common/time_util.h#L27-L32

[system_clock from cpp ref](https://en.cppreference.com/w/cpp/chrono/system_clock) is a wall clock and [wall clock from cpp ref](https://en.cppreference.com/w/cpp/chrono/steady_clock) is a monotonic clock.
I find this function is widely used in scenarios related to `ttl`. Maybe the monotonic one is better because the wall clock exists the problem of time adjustment and we only want a precise time interval.

A related discussion from [stackoverflow](https://stackoverflow.com/questions/31552193/difference-between-steady-clock-vs-system-clock).
>As opposed to the system_clock, which is not monotonic (i.e. the time can decrease if, say, the user changes the time on the host machine.)

### Solution

_No response_

### Are you willing to submit a PR?

- [ ] I'm willing to submit a PR!

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kvrocks] infdahai closed issue #1466: consider to replace system_clock with steady_clock

Posted by "infdahai (via GitHub)" <gi...@apache.org>.

infdahai closed issue #1466: consider to replace system_clock with steady_clock
URL: https://github.com/apache/incubator-kvrocks/issues/1466


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kvrocks] mapleFU commented on issue #1466: consider to replace system_clock with steady_clock

Posted by "mapleFU (via GitHub)" <gi...@apache.org>.

mapleFU commented on issue #1466:
URL: https://github.com/apache/incubator-kvrocks/issues/1466#issuecomment-1557550092

   https://github.com/apache/incubator-kvrocks/pull/1038#issuecomment-1289048482
   
   Seems we've talked about it here.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kvrocks] infdahai commented on issue #1466: consider to replace system_clock with steady_clock

Posted by "infdahai (via GitHub)" <gi...@apache.org>.

infdahai commented on issue #1466:
URL: https://github.com/apache/incubator-kvrocks/issues/1466#issuecomment-1557631895

   The more specific dicussion about the clock is placed [here in redis](https://github.com/redis/redis/pull/7644#issue-676476847).
   
   >We need to call ustime() because there is a cached timestamp which matches the wall-clock. This is set at the beginning of every command and is used to support various functions which need this recent wall-clock time.
   We also need to time the execution of the current command by measuring time before/after invoking the command. Measuring duration is something that we'd normally perform using the monotonic clock (not wall-clock).
   The logic is something like this:
   ```
   set cached_ustime = ustime()   // wall clock
   
   set command_start = getMonotonic()
   execute command
   set command_duration = getMonotonic() - command_start
   ```
   
   from https://github.com/redis/redis/pull/10502#issuecomment-1091972167
   
   So we don't need this issue currently in other word.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org