Better exposure of LB status in UI #2071
Conversation
| @@ -364,24 +364,6 @@ public SingularityCreateResult savePendingTask(SingularityPendingTask task) { | |||
| return getChildrenAsIdsForParents("getAllTaskIds", paths, taskIdTranscoder); | |||
| } | |||
|
|
|||
| private List<SingularityTaskId> getTaskIds(String root) { | |||
| List<SingularityTaskId> notYetHealthy, | ||
| List<SingularityPendingTaskId> pending, | ||
| List<SingularityTaskId> cleaning) { | ||
| this(healthy, notYetHealthy, pending, cleaning, Collections.emptyList(), Collections.emptyList()); |
There was a problem hiding this comment.
iirc you get an exception if you try to add to an emptyList(), might want to construct new lists to avoid a gotcha if this wasn't intentional
There was a problem hiding this comment.
Was debating that, figured I'd tend towards treating as immutable. In most places in the code we try and do that, and make copies fo lists where appropriate if we are going to build on them
| @@ -45,12 +45,34 @@ const ActiveTasksTable = ({request, requestId, tasksAPI, healthyTaskIds, cleanin | |||
|
|
|||
| const tasksWithHealth = _.map(tasks, (task) => { | |||
There was a problem hiding this comment.
could move this to mapStateToProps for cleanliness, but given that tasks might still be loading there I think it's fine to leave this as is
| .filter(taskManager::isInLoadBalancer) | ||
| .forEach(loadBalanced::add); | ||
| cleaningTaskIds.stream() | ||
| .filter(taskManager::isInLoadBalancer) |
There was a problem hiding this comment.
does the task manager automatically update itself when tasks go in and out of the load balancer?
There was a problem hiding this comment.
yeah, the isInLoadBalancer here is a live call to fetch the state (cached on the leader in memory)
|
🚢 |
The Singularity backend knows if a task has been added to the load balancer or not. We should better expose this in the health icon in the UI to show if a task is currently serving traffic or not. Still debating if this should be a separate column. Didn't want to make the active tasks table too much wider, so for the moment I've tried to see if we can encode this all into the existing health icon in a way that is easy enough to understand