Add heartbeat capability

[kpamnany]

PR Description

Telemetry for Julia's threads.

This capability is build-time enabled by setting JL_HEARTBEAT_THREAD in src/options.h, which causes Julia to start a heartbeat thread that quickly goes to sleep.

Control heartbeats with jl_heartbeat_enable(heartbeat_s, n_reports, reset_reporting_after_s). Heartbeats are disabled if heartbeat_s <= 0. Otherwise, you must call jl_heartbeat() at least one time every heartbeat_s. If this does not happen, the heartbeat thread will call jl_print_task_backtraces(0). If jl_heartbeat() continues to not be called, we double the reporting interval up to a maximum of 60 times the heartbeat_s and report up to n_reports times altogether. Once jl_heartbeat() is called again, the number of reports is reset immediately but we only reset the reporting interval back to heartbeat_s after reset_reporting_after_s of steady jl_heartbeat()s are observed.

Checklist

Requirements for merging:

• [ ] I have opened an issue or PR upstream on JuliaLang/julia:
• I have created an issue to upstream this feature: https://relationalai.atlassian.net/browse/RAI-17788
• I have removed the port-to-* labels that don't apply.
• I have opened a PR on raicode to test these changes: https://github.com/RelationalAI/raicode/pull/15844

[NHDaly]

This mostly LGTM! I think it needs a couple tests?

Also, should we open this as a PR upstream? This seems like a useful feature for anyone who is experiencing process hangs and/or deadlocks, not just for us, yeah?

[NHDaly]

My main remaining question is that this should probably also be opened upstream, right?
Also, what happened to the PR checkboxes? Can you add those back?

[kpamnany]

Yes, I think we should open this upstream, but we should work out all the kinks beforehand. There might be tweaks we apply to this to make it more useful/effective over the next weeks.

[NHDaly]

Makes sense. Please file an internal tracking issue then, so we don't forget, and put that in place of the first checkbox. And attend to the second checkbox.

Assuming those are addressed and you add a bunch of comments, this LGTM i think!

[kpamnany]

More comments added. Issue to upstream this created and linked.

Please review the sleep duration computation I've changed in these lines? Previously, the code unsafely assumed that tchb < 1e9. This assumption is now removed.

Also added a reporting interval backoff as suggested.

[kpamnany]

I spent some time trying to figure out how to test this. The challenge is that heartbeats are time driven and all timing sensitive tests are flaky in CI. Would love any ideas.

[NHDaly]

I spent some time trying to figure out how to test this. The challenge is that heartbeats are time driven and all timing sensitive tests are flaky in CI. Would love any ideas.

The standard way to deal with time-sensitive approaches is to mock out the timing element, and step the system forward in the tests.

There are basically two or three main variants that I've seen on that:

• mockable / test-injectable timing system
  • At google, any class that wanted to anything based on time would depend on some Timing or Clock interface, and so to instantiate one, you'd have pass in a Time object. In production, this would be a normal Time implementation, but in tests, you could use a MockTime instance, which lets you control the time from your tests.
• Insert mockable "pause points" specifically for testing, and have the tests override those mocks.
  • That's what we do in RAI. You can look at the v2-api tests for an example of this, where we have mockable pause points in the transaction processing, and we use a channel to set up synchronization between the client and server through those mocked pause points.
• (variant on the above) If your system already has some synchronization interaction, mock out those to drive the tests.
  • So if the system waits for a client before proceeding at some points, you can mock out the client and control the system that way.

I'm not sure which of those would be most applicable in this situation. Notably, Julia's codebase does not employ any of these in its current test suites, so the infrastructure isn't there to easily build upon it.

[NHDaly]

One way to mock out the timing would be: instead of calling jl_hrtime() directly, you could call it through a global function pointer which defaults to jl_hrtime. Then the tests could update that global function pointer to point to a mocked timing function, which calls a julia callback instead, and from there you can now do whatever you want to step time forward.

[kpamnany]

As discussed, I've changed the configuration and behavior of the heartbeat thread; see here.

Additionally, I've hardened the interface so that two consecutive jl_heartbeat_enable() calls don't break things.

Here's the log of the benchmark.micro.query_evaluation job with the heartbeat thread enabled; there are quite a few heartbeat losses -- task backtraces printed. There seem to be only two types:

• do_bench runs on thread 1 and occupies it completely; an artifact of how the benchmark is written. But it isn't clear why the second interactive thread doesn't pick up and run the server metrics task; it seems to be asleep.
• A thread is compiling (deep in LLVM) and numerous other threads are blocked on the codegen_lock.

Would love to try and get this complete and merged tomorrow.

[NHDaly]

Really neat to hear you're seeing this in action!

• But it isn't clear why the second interactive thread doesn't pick up and run the server metrics task; it seems to be asleep

Is this because thread 1 isn't able to drive the event loop while it's running the do_bench function?

[NHDaly]

LGTM, thanks Kiran! 🎉
Can you make sure all the open threads are resolved before merging? Otherwise LGTM

[Drvi]

I've ported the PR to all +RAI branches