aurora-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Zameer Manji <zma...@apache.org>
Subject Re: [Discussion] Implementing a is_health_check_enabled function OR by checking the presence of an instance of health checker
Date Mon, 26 Sep 2016 21:04:25 GMT
I have no strong preference either way.

On Mon, Sep 26, 2016 at 11:25 AM, Huang Kai <texasred2013@hotmail.com>
wrote:

> Hi folks,
>
> I'm currently blocked on the review https://reviews.apache.org/r/51876.
> I was wondering if you guys can provide some insights into the two proposed
> approaches on RB and help me proceed.
>
> The problem is that the aurora executor needs to determine if it should
> send a TASK_RUNNING message based on whether health check is enabled for an
> assigned task.
>
> Initially, I created an is_health_check_enabled(assigned_task) function
> in task_info.py, and use it in aurora executor. See:
> https://reviews.apache.org/r/51876/diff/3/. However, Maxim raised a valid
> point that is_health_check_enabled has some duplication with the set up of
> health checker in later step. Therefore we should reuse the logic of
> is_health_check_enabled as much as possible in health_checker.
>
> One solution is to create a dedicated function called
> is_health_check_enabled for an assigned_task, and reuse it when we set up a
> health checker. The benefit is better abstraction and ease for test.
>
> The challenge of implementing it is that this function seems a little bit
> heavy-weighted, we have to parse an assigned_task, compute the port map,
> and get health_checker, health_check_config from it as well. One solution I
> can come up with is to store all the computation result(port_map,
> health_checker, health_check_config) in a utility class. So that it can be
> reuse later. But a downside here is that the is_health_check_enabled now
> serves multiple purposes, and the meaning of this function is not clear. It
> should only answer one question: is health check enabled on this task?
>
> A second solution is to check if health check is enabled for an
> assigned_task by checking the presence of a health checker instance. A
> benefit of doing this is that we can set up the necessary health checkers
> and check if health check is enabled in one pass. In this way, we used the
> logic as much as possible and eliminate the duplications. See
> https://reviews.apache.org/r/51876/diff/5/
>
> Could you guys let me know your thought on the two approaches? If no one
> objects to the second solution, I will modify the executor as
> https://reviews.apache.org/r/51876/diff/5/
>
> Best,
>
> Kai
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message