[core] telemetry for oom occurences #30472

clarng · 2022-11-18T14:54:31Z

Signed-off-by: Clarence Ng clarence.wyng@gmail.com

Why are these changes needed?

track how often oom triggers in the wild

Related issue number

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 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 :(

Signed-off-by: Clarence Ng <clarence.wyng@gmail.com>

python/ray/_private/usage/usage_lib.py

Signed-off-by: Clarence Ng <clarence.wyng@gmail.com>

jjyao · 2022-11-18T16:39:37Z

cc @thomasdesr for security review.

Signed-off-by: Clarence Ng <clarence.wyng@gmail.com>

jjyao

LGTM. I'll follow up with #30427.

cc @iycheng for the gcs side change.

python/ray/_private/usage/usage_lib.py

python/ray/tests/test_usage_stats.py

Signed-off-by: Clarence Ng <clarence.wyng@gmail.com>

…elemtrics Signed-off-by: Clarence Ng <clarence.wyng@gmail.com>

ericl · 2022-11-18T23:51:57Z

Will defer to @iycheng for merge.

Signed-off-by: Clarence Ng <clarence.wyng@gmail.com>

fishbone · 2022-11-21T02:06:51Z

src/ray/gcs/gcs_server/gcs_kv_manager.h

@@ -138,7 +138,7 @@ class RedisInternalKV : public InternalKVInterface {
 /// This implementation class of `InternalKVHandler`.
 class GcsInternalKVManager : public rpc::InternalKVHandler {
 public:
-  explicit GcsInternalKVManager(std::unique_ptr<InternalKVInterface> kv_instance)


Any reason to make it a shared ptr? If it's not shared across threads, we shouldn't do this. kv_instance is belonged to GcsInternalKVManager only which makes things simpler.

fishbone · 2022-11-21T02:15:00Z

I feel we shouldn't use kv instance here directly for stats. We should use general APIs provided by GCS (GlobalStateAcessor/GcsClient). I'm thinking about Raylet/CoreWorker which want to store the stats and we probably don't want to have different approaches to record the data.

There is protocol for telemetry and now we have already got python version where core logical is written in py. I'd think in the future, it's going to be very hard to maintain (the same protocol in py/java/gcs/coreworker/raylet).

clarng · 2022-11-21T02:31:49Z

I feel we shouldn't use kv instance here directly for stats. We should use general APIs provided by GCS (GlobalStateAcessor/GcsClient). I'm thinking about Raylet/CoreWorker which want to store the stats and we probably don't want to have different approaches to record the data.

There is protocol for telemetry and now we have already got python version where core logical is written in py. I'd think in the future, it's going to be very hard to maintain (the same protocol in py/java/gcs/coreworker/raylet).

We added a TODO to do the the way you suggested (gcs client). The reason it is a TODO right now to integrate with gcs client is because it is crashing / hang when we try to use it directly - we are not sure if it is because we are calling gcs from gcs. Jiajun was looking into this last Friday.

The hope is to get something functional and migrate to the gcs client approach as soon as we figured out what was going on that prevented it.

fishbone · 2022-11-21T04:36:54Z

@clarng that sgtm then! Thanks for the explanation! The only comment is the unique_ptr/shared_ptr.

rkooo567 · 2022-11-21T08:47:52Z

Lmk when tests pass!

clarng · 2022-11-21T16:02:46Z

Lmk when tests pass!

Looks like it is passing now, some failures look unrelated (not worse than master)

@pcmoritz

We added some basic Ray core metrics: num of actors created and num of pgs created. To achieve so, this PR introduced 3 changes (potentially we can split them): Move the usage_lib.py TagKey into usage.proto, and also added @pcmoritz / @thomasdesr as code owner. This makes the future change to telemetry easier to audit; and also allows us use telemetry across different languages. Introduced a Gcs/C++ version of usage_lib (GcsUsgageReporter). Its implementation is similar to [core] telemetry for oom occurences #30472. This make it easier to add telemetry from GCS. Added PlacementGroup and Actor telemetry code which collects the number of actors/pgs created during the lifetime of the cluster.

track how often oom triggers in the wild Signed-off-by: Clarence Ng <clarence.wyng@gmail.com> Signed-off-by: Weichen Xu <weichen.xu@databricks.com>

@pcmoritz

We added some basic Ray core metrics: num of actors created and num of pgs created. To achieve so, this PR introduced 3 changes (potentially we can split them): Move the usage_lib.py TagKey into usage.proto, and also added @pcmoritz / @thomasdesr as code owner. This makes the future change to telemetry easier to audit; and also allows us use telemetry across different languages. Introduced a Gcs/C++ version of usage_lib (GcsUsgageReporter). Its implementation is similar to [core] telemetry for oom occurences ray-project#30472. This make it easier to add telemetry from GCS. Added PlacementGroup and Actor telemetry code which collects the number of actors/pgs created during the lifetime of the cluster. Signed-off-by: Weichen Xu <weichen.xu@databricks.com>

@pcmoritz

We added some basic Ray core metrics: num of actors created and num of pgs created. To achieve so, this PR introduced 3 changes (potentially we can split them): Move the usage_lib.py TagKey into usage.proto, and also added @pcmoritz / @thomasdesr as code owner. This makes the future change to telemetry easier to audit; and also allows us use telemetry across different languages. Introduced a Gcs/C++ version of usage_lib (GcsUsgageReporter). Its implementation is similar to [core] telemetry for oom occurences #30472. This make it easier to add telemetry from GCS. Added PlacementGroup and Actor telemetry code which collects the number of actors/pgs created during the lifetime of the cluster.

@pcmoritz

We added some basic Ray core metrics: num of actors created and num of pgs created. To achieve so, this PR introduced 3 changes (potentially we can split them): Move the usage_lib.py TagKey into usage.proto, and also added @pcmoritz / @thomasdesr as code owner. This makes the future change to telemetry easier to audit; and also allows us use telemetry across different languages. Introduced a Gcs/C++ version of usage_lib (GcsUsgageReporter). Its implementation is similar to [core] telemetry for oom occurences ray-project#30472. This make it easier to add telemetry from GCS. Added PlacementGroup and Actor telemetry code which collects the number of actors/pgs created during the lifetime of the cluster. Signed-off-by: tmynn <hovhannes.tamoyan@gmail.com>

clarng added 4 commits November 18, 2022 06:53

[core] telemetry for oom occurences

34b6724

Signed-off-by: Clarence Ng <clarence.wyng@gmail.com>

[core] telemetry for oom occurences

6f0d6b6

Signed-off-by: Clarence Ng <clarence.wyng@gmail.com>

[core] telemetry for oom occurences

d16cd6a

Signed-off-by: Clarence Ng <clarence.wyng@gmail.com>

[core] telemetry for oom occurences

f25d517

Signed-off-by: Clarence Ng <clarence.wyng@gmail.com>

clarng marked this pull request as ready for review November 18, 2022 15:07

clarng requested review from ericl, richardliaw, rkooo567 and jjyao as code owners November 18, 2022 15:07

clarng assigned ericl, scv119 and jjyao Nov 18, 2022

jjyao reviewed Nov 18, 2022

View reviewed changes

python/ray/_private/usage/usage_lib.py Show resolved Hide resolved

python/ray/_private/usage/usage_lib.py Show resolved Hide resolved

[core] telemetry for oom occurences

119978f

Signed-off-by: Clarence Ng <clarence.wyng@gmail.com>

clarng added 3 commits November 18, 2022 12:01

[core] throttle and improve oom logging message

5202012

Signed-off-by: Clarence Ng <clarence.wyng@gmail.com>

[core] telemetry for oom occurences

f99b770

Signed-off-by: Clarence Ng <clarence.wyng@gmail.com>

[core] telemetry for oom occurences

e48d7a6

Signed-off-by: Clarence Ng <clarence.wyng@gmail.com>

jjyao approved these changes Nov 18, 2022

View reviewed changes

python/ray/_private/usage/usage_lib.py Show resolved Hide resolved

python/ray/tests/test_usage_stats.py Show resolved Hide resolved

clarng added 2 commits November 18, 2022 15:06

[core] telemetry for oom occurences

4d00996

Signed-off-by: Clarence Ng <clarence.wyng@gmail.com>

Merge branch 'master' of https://github.com/ray-project/ray into oomt…

edb17b7

…elemtrics Signed-off-by: Clarence Ng <clarence.wyng@gmail.com>

ericl approved these changes Nov 18, 2022

View reviewed changes

ericl assigned fishbone and unassigned ericl Nov 18, 2022

[core] telemetry for oom occurences

88015a8

Signed-off-by: Clarence Ng <clarence.wyng@gmail.com>

fishbone reviewed Nov 21, 2022

View reviewed changes

scv119 mentioned this pull request Nov 21, 2022

[Core][Telemetry] Add basic core telemetries #30520

Merged

7 tasks

clarng added the tests-ok The tagger certifies test failures are unrelated and assumes personal liability. label Nov 21, 2022

clarng mentioned this pull request Nov 21, 2022

[core] use usage_reporter for worker crash count in telemetry #30534

Closed

scv119 merged commit 5fa4132 into ray-project:master Nov 21, 2022

clarng mentioned this pull request Nov 23, 2022

[core] telemetry for when memory monitor is enabled #29152

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[core] telemetry for oom occurences #30472

[core] telemetry for oom occurences #30472

clarng commented Nov 18, 2022

jjyao commented Nov 18, 2022

jjyao left a comment

ericl commented Nov 18, 2022

fishbone Nov 21, 2022

fishbone commented Nov 21, 2022

clarng commented Nov 21, 2022

fishbone commented Nov 21, 2022 •

edited

Loading

rkooo567 commented Nov 21, 2022

clarng commented Nov 21, 2022 •

edited

Loading

[core] telemetry for oom occurences #30472

[core] telemetry for oom occurences #30472

Conversation

clarng commented Nov 18, 2022

Why are these changes needed?

Related issue number

Checks

jjyao commented Nov 18, 2022

jjyao left a comment

Choose a reason for hiding this comment

ericl commented Nov 18, 2022

fishbone Nov 21, 2022

Choose a reason for hiding this comment

fishbone commented Nov 21, 2022

clarng commented Nov 21, 2022

fishbone commented Nov 21, 2022 • edited Loading

rkooo567 commented Nov 21, 2022

clarng commented Nov 21, 2022 • edited Loading

fishbone commented Nov 21, 2022 •

edited

Loading

clarng commented Nov 21, 2022 •

edited

Loading