[Dashboard]Don't set node actors when node_id of actor is Nil

[WangTaoTheTonic]

Why are these changes needed?

When actor is not ALIVE, normally it's raylet id is not set when published. The raylet id would be Nil("ffffffffffffffffffffffffffffffffffffffffffffffffffffffff", aka "f" * 56).

We should not update node_actors when the raylet id is Nil.

Related issue number

Checks

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

[rkooo567]

Doesn't this mean we cannot get dead actors information?

[WangTaoTheTonic]

Doesn't this mean we cannot get dead actors information?

We can still get dead actors, no matter before or after this change, as even an actor is dead, its raylet id still is valid(not empty).

This pr fix bug that when a new actor coming, it carries empty raylet id because it has not been allocated yet. Then the dashboard will put information of this actor into node_actors with an empty node id key, which means all new actors would be put into that map unnecessarily, causing memory leak.

[fyrestone]

It's better to set a const in stats_collector_consts.py

NIL_NODE_ID = ray.NodeID.nil().hex()

then check if node_id != stats_collector_consts.NIL_NODE_ID.

[WangTaoTheTonic]

Sounds like a good idea!

[fyrestone]

Can you add this check to GetAllActorInfo processing logic, too? Then, the assertion could be added to the test_stats_collector.py:

# Start a cluster and a actor, use the resource constraints to prevent the actor from being alive
response = requests.get(webui_url + "/test/dump?key=node_actors")
response.raise_for_status()
result = response.json()
assert stats_collector_consts.NIL_NODE_ID not in result["data"]["node_actors"]

[fyrestone]

Doesn't this mean we cannot get dead actors information?

We can still get dead actors, no matter before or after this change, as even an actor is dead, its raylet id still is valid(not empty).

This pr fix bug that when a new actor coming, it carries empty raylet id because it has not been allocated yet. Then the dashboard will put information of this actor into node_actors with an empty node id key, which means all new actors would be put into that map unnecessarily, causing memory leak.

Good catch! The value of DataSource.node_actors[NIL_NODE_ID] will be leaking in such case. Actually, none of the actor info will be deleted from Redis, so this is a leaky system. But we can avoid the leak of DataSource.node_actors[NIL_NODE_ID].

[rkooo567]

Ping me after addressing @fyrestone's comment!

[rkooo567]

I will merge upon his approval

[fyrestone]

LGTM

[WangTaoTheTonic]

@rkooo567 It's ok to go!