[core][state] Task Backend - reduce lock contention on debug stats / metric recording on counters.

[rickyyx]

Signed-off-by: rickyyx [email protected]

Why are these changes needed?

When GcsTaskManager is busy processing task events, it is not supposed to slow down the GCS. However, we previously have mutexes protecting some of the counter states. So the main io service/thread will get blocked when trying to acquire locks to print debug states + record metrics + add telemetry data.

Global stats: 196276 total (5 active)
Queueing time: mean = 5.255 ms, max = 4.545 s, min = -0.000 s, total = 1031.389 s
Execution time:  mean = 295.864 us, total = 58.071 s
Event stats:
....
        GCSServer.deadline_timer.debug_state_dump - 85 total (1 active), CPU time: mean = 521.750 ms, total = 44.349 s
        GCSServer.deadline_timer.debug_state_event_stats_print - 15 total (1 active, 1 running), CPU time: mean = 404.255 ms, total = 6.064 s
....

This PR

1. introduced a thread-safe wrapper on CounterMap, such that modifying and reading various debug counters will have minimal lock contentions. Also merged the count by task type for telemetry into the counter map. This way, we will not need to acquire locks at various places.
2. With access to counters thread-safe now, we could also remove the mutex locks on the GcsTaskManagerStorage since it's now thread-safe (only accessed from its dedicated io thread)

Related issue number

Closes #32202

Checks

https://buildkite.com/ray-project/release-tests-pr/builds/27896#_

• I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
• I've run scripts/format.sh to lint the changes in this PR.
• I've included any doc changes needed for https://docs.ray.io/en/master/.
• I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
• Testing Strategy
  • Unit tests
  • Release tests
  • This PR is not tested :(

[cadedaniel]

Can we get this reviewed and merged ASAP? This is on the critical path for a release test run and handoff to product.

[rickyyx]

Fixing the tsan test.

[rickyyx]

Together with changes from #32251 it is still rather slow on the stress_test_many_tasks test if I increased the buffer size to 1M and keep the batch size as 10k.

Trying a batch size of 1k to see if it helps.

[rkooo567]

I think this looks good now, but for the maintainability perspective, I think we should make the whole class thread-safe (IMO partially thread-safe class like this is the worst for maintainability). E.g., if someone touches states within Stop, OnJobFinished, or the storage class, it can cause random issues that are difficult to debug.

So let's merge it first and make another PR to make the whole class thread-safe.

[rkooo567]

ping me when tests pass

[rickyyx]

I guess making the whole class thread-safe is not hard. I could add a bunch of mutexes there.

But yeah, will open in another PR for this.