[Serve] Deflake test_metrics

[GeneDer]

Why are these changes needed?

Split out test_metrics to run on it's own so the metrics will not be polluted by other tests.

Related issue number

Closes #45843

Checks

• 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 added any new APIs to the API Reference. For example, if I added a
    method in Tune, I've added it in doc/source/tune/api/ under the
    corresponding .rst file.
• 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 :(

[GeneDer]

Tests 5 runs (29452, 29455, 29457, 29459, 29461) and not seeing any more failures with this change

[edoakes]

@GeneDer can you explain a little more about what the issue was an how this solves it? And also why is this specific to windows?

[GeneDer]

@GeneDer can you explain a little more about what the issue was an how this solves it? And also why is this specific to windows?

This test was running in windows before, there is no change on that.

I think the main issue is there are some race conditions between different tests running in this build and polluting the metrics, so often time we see this test failing (timed out with condition never met).

Also after I factor this test out, we are seeing those unexpected kwarg error, so those get fixed as well.

[edoakes]

@GeneDer can you explain a little more about what the issue was an how this solves it? And also why is this specific to windows?

This test was running in windows before, there is no change on that.

I think the main issue is there are some race conditions between different tests running in this build and polluting the metrics, so often time we see this test failing (timed out with condition never met).

Also after I factor this test out, we are seeing those unexpected kwarg error, so those get fixed as well.

We should be able to set up the fixtures properly to clear all of the expected metrics. We rely on metrics in quite a few tests, so it's probably better to nail down the root cause there

[GeneDer]

@edoakes I refactored the call to clean up metrics between tests into fixture and call on each of the tests here. Can you PTAL?

[edoakes]

Hm shouldn't this be sufficient on its own? The prometheus endpoint is the raylet, so if ray is shut down between runs there should be no state sharing.

What am I missing?

[GeneDer]

My expectation is there are something that's not cleaned up in between those tests. And in fact adding those calls seems to helped. Now that thinking through it again maybe just adding some sleep in between will also help the same way and maybe the issue is serve and/or ray wasn't complete shutdown before the next test starts? 🤔 Let me do some more experiments

[edoakes]

we should not add any sleeps -- if we need to wait for anything to clean up, then explicitly wait for the cleanup to happen

sleeps are what make things flaky in the first place

[GeneDer]

Close for now, will dig into this deeper when we have more bandwidth. There still seemed to have some port binding issues with this change