refactored create_complete_config #298
Conversation
| """backoff_seconds represents how many seconds a penalization factor for failing tasks. | ||
| Every time a task fails, Marathon adds this value multiplied by a backoff_factor. | ||
| In PaaSTA we know how many instances a service has, so we adjust the backoff_seconds | ||
| to account for this, which prevents services with large number of instances from | ||
| being penalized more than services with small instance counts. (for example, a service | ||
| with 30 instances will get backed off 10 times faster than a service with 3 instances).""" | ||
| instances = self.get_instances() | ||
| if instances is None: |
There was a problem hiding this comment.
Why is it necessary to pass in instances like this?
There was a problem hiding this comment.
@solarkennedy calling format_marathon_app_dict calls both get_instances and get_backoff_seconds. Since instances might read data from zookeeper this allows us to only query zookeeper
There was a problem hiding this comment.
@solarkennedy another alternative would just be to use min_instances instead of instances here.
There was a problem hiding this comment.
Yea. If this number is going to be dynamically changing a lot, then we can't use it in the hash computation. We have to use something more static. max_instances is actually what we really want
There was a problem hiding this comment.
@solarkennedy this number is actually excluded from the hash calculation. We still want users to be able to change min_instances and max_instances without causing a bounce anyways
There was a problem hiding this comment.
@solarkennedy fixed in #290
This line really should have been a part of that PR anyways
There was a problem hiding this comment.
Can you also fix the docstring ("backoff_seconds represents how many seconds a penalization factor for failing tasks").
mjksmith
force-pushed
the
create_config_rebase
branch
from
e4d5eca to
fb36b35
Compare
|
ship |
refactored create_complete_config
No description provided.