vine: changes tasks_running to tasks_on_workers for display tables

[btovar]

Proposed changes

tasks_on_workers is accurate for what the manager knows. tasks_running depends on worker messages that may be stale.

Post-change actions

• make test Run local tests prior to pushing.
• make format Format source code to comply with lint policies. Note that some lint errors can only be resolved manually (e.g., Python)
• make lint Run lint on source code prior to pushing.
• Type Labels Select github labels for the type of this change: bug, enhancement, etc.
• Product Labels Select github labels for the product affected: TaskVine, Makeflow, etc.
• PR RTM Mark your PR as ready to merge.

[dthain]

Hmm, I'm not sure if this has the intended effect. Here is what I see:

1 - The manager knows how many tasks are in the RUNNING state (sent to the worker) by just looking at itable_size(q->running_table). That is generally what we mean by "running" throughout the code.
https://github.com/cooperative-computing-lab/cctools/blob/master/taskvine/src/manager/vine_manager.c#L5177
2 - tasks_on_workers is the sum of tasks in RUNNING and WAITING_RETRIEVAL states.
3 - s->tasks_running is the sum of w->tasks_running which is the (stale) information received from workers about processes actually executing at the worker via asynchronous info tasks_running messages.

Currently # 3 is the thing displayed to users in vine_status. I think you are saying that the user really wants to see # 1. So the more direct solution would be to change vine_get_stats to report itable_size(q->running_table) as the value tasks_running, instead of running around and adding up all the worker values.

What do you think?

[btovar]

Ah, yes, directly looking at q->running_table would be better.

[btovar]

Actually, the pr should be ready. Note that w->current_tasks is only used when asking for the information per worker. For the rest, tasks_on_workers already is set from q->running_table in get_stats.

[dthain]

So instead of changing the status tools to display tasks_on_workers, please change the manager to simply report tasks_running = itable_size(q->running_table) instead of computing tasks_running = SUM(w->tasks_running)

[btovar]

RTM

[dthain]

The changes to vine_manager are fine, but I would like to keep the display fields showing tasks_running as RUNNING.

[btovar]

This pr would make it so that it shows tasks_on_workers. The tasks_running stat has always been a little bit hacky. Do you mean to keep the header RUNNING but show the tasks_on_workers value?

[dthain]

No, I just want the plain meaning used throughout: The reported tasks_running property should reflect the number of tasks in the RUNNING state (which is itable_size(q->running_table)) and show it to the user as RUNNING.

I understand that the meaning of "running" is a little fuzzy because it means "the manager sent it to the worker but the worker may not have received or acted on it yet". but that's true of any property in a distributed system.

[btovar]

Ok, should I remove the tasks_on_workers stat? It is too soon for deprecated stats!

[dthain]

No reason to remove stats reported to the catalog, they may still come in handy, I think.

[btovar]

They will have the same value, as they are both set to the size of q->tasks_running. Is that ok?

[btovar]

Sorry, they are different, my mistake. on_workers also has tasks with results.