Monotonic time

[pboling]

Important background: Monotonic time is the "correct" way to calculate elapsed time in Ruby:
https://blog.dnsimple.com/2018/03/elapsed-time-with-ruby-the-right-way/

• Adds started_at based on the monotonic clock which is guaranteed to always move forward (processor uptime)
• Fixes created_at / timestamp based on Process.clock_gettime(Process::CLOCK_REALTIME), to avoid Time.now stubbing & Timecop issues
• Fixes I am experiencing race condition while using SimpleCov with Parallel_tests #559 (I think)
• Fixes Race Condition in parallel_tests integration: an outdated report may be written #815 (I think)
• Fixes some spec failures in the cucumber suite, due to a bug in Rails, where it failed to require "logger".
• Fixes an out of order condition which results in incompability with turbo_tests - an "almost drop-in" replacement/enhancement of parallel_tests) (see: Simplecov support serpapi/turbo_tests#24) (though there are other major blockers preventing compatibility with turbo_tests)

I do not understand the rubocop failure in CI, because when I run locally there are no violations:

❯ bundle exec rubocop -a
Inspecting 111 files
...............................................................................................................

111 files inspected, no offenses detected

Same result when run with bundle exec rake rubocop as it is in CI.

CC @ilyazub

[pboling]

This is now ready for review @amatsuda @PragTob

[pboling]

Note: the rubocop failure will be fixed once @simi's work in #1131 is merged, and thus it will be a completely green build!
Ping @amatsuda @PragTob.

[sferik]

Thanks for the PR.

I added some minor concerns that I left comments about but I have a bigger concern with the core approach: monotonic time doesn’t work across processes.

Monotonic clock values are per-process. They represent something like the process’s view of system uptime and have no shared epoch between processes. The started_at value gets serialized to .resultset.json and then read back in time_since_result_creation by a different process (the one doing the merge). When that process computes current_monotonic - stored_started_at, the two values come from different processes with different monotonic origins, so the result is meaningless. This could cause valid results to be incorrectly expired or stale results to be kept.

The cross-process merge path is actually one place where wall clock time is the right choice, since it’s a clock with a shared epoch. If the concern is Time.now being stubbed by Timecop, switching created_at to use Process.clock_gettime(Process::CLOCK_REALTIME), solves that without needing monotonic time.

Would you consider a version of this PR scoped to just that change?

[sferik]

These should be attr_reader, since there’s no reason to mutate them after construction.

[sferik]

This environment variable is introduced but not documented, tested, and doesn’t guard against "false" being truthy.

[sferik]

This looks like a local artifact that shouldn’t be committed.

[pboling]

Probably should be updated to not be a pre-release version of bundler!

[sferik]

It has been update to 4.0.9 in the main branch. Feel free to rebase.

[pboling]

@sferik

monotonic time doesn’t work across processes.

I just looked into this a bit deeper, because it surprised me to hear it. What you say is true of some programming languages, including notably Python. It is not generally true of Ruby, nor of system level OS calls in POSIX systems.

• System-Wide by Default: On all modern POSIX-compliant systems (Linux, macOS, BSD), CLOCK_MONOTONIC is system-wide. Ruby simply wraps the OS system call, meaning Process A and Process B will receive the same time value at the same moment.
• Shared Epoch: The "origin" (time zero) is system-wide, and is typically the system boot time. Because both processes share this same origin, comparing values between them is valid.
• In the Ruby documentation this is hedged, for a valid, albeit perhaps edge case, reason, because on non-POSIX compliant systems the origin may vary from the POSIX standard, like old versions of Windows.

Exceptions:

• If simplecov timings are to be compared across different servers, then the monotonic clock values would indeed lose all significance.

I don't know if this changes your view. If it doesn't, then I'm happy to switch it back to wall clock - though I expect it could reintroduce the race condition I built this to work around.

Perhaps another option would be to allow the user to configure the timer either way. Then cross-server setups have meaningful timings. Worth noting that if a suite is run across servers it almost certainly implies long running suites. On the other hand, if a suite runs on a single server in different processes there is a decent chance of them executing extremely fast, increasing the liklihood of running into the classic issues with wall clock time math.

What do you think of supporting both?

[pboling]

Found this interesting documentation:

Note: Time calculations on all platforms and languages are sensitive to changes to the system clock. To alleviate the potential problems associated with changing the system clock while an application is running, most modern operating systems provide a monotonic clock that operates independently of the system clock. A monotonic clock cannot be used to determine human-friendly clock times. A monotonic clock is used exclusively for calculating time intervals. Not all Ruby platforms provide access to an operating system monotonic clock. On these platforms a pure-Ruby monotonic clock will be used as a fallback. An operating system monotonic clock is both faster and more reliable than the pure-Ruby implementation. The pure-Ruby implementation should be fast and reliable enough for most non-realtime operations. At this time the common Ruby platforms that provide access to an operating system monotonic clock are MRI 2.1 and above and JRuby (all versions).

https://ruby-concurrency.github.io/concurrent-ruby/master/Concurrent/ScheduledTask.html#:~:text=Note:,loosely%20based%20on%20Java's%20ScheduledExecutorService.

[sferik]

I stand corrected on monotonic time. I’m not sure why I had a mistaken understanding of how it worked. Thank you for bringing me up-to-speed.

Despite being right about that, I still feel like this pull request is over-engineered for the actual problem. If the main goal is to avoid Timecop interference, Process.clock_gettime(Process::CLOCK_REALTIME) is a one-line fix that doesn’t require a new class, a new serialized field, fallback logic, etc. I believe just that change would provide 90% of the benefit with none of the complexity. What if just started with that?

Note, I don’t believe this change would resolve #559 or #815. I believe those issues are caused by a process synchronization race condition in the parallel_tests integration. Here is what I think is happening:

1. Multiple test processes finish at different times.
2. Each process reads .resultset.json, merges what’s there with its own results, and writes the merged report.
3. The process that checks coverage thresholds (via at_exit) may read the resultset before other processes finish writing their results to it.

[pboling]

I'll submit a new PR with the simpler approach!