HomeMesosphereNo notifications. 4 unresolved issues.
  • Search
Differential D296

Rename unreachable strategy parameters to avoid API stutter.
ClosedAll Users
Actions

Authored by jeschkies on Dec 8 2016, 2:23 PM.

Details

Reviewers
jdef
meichstedt
Commits
rMARATHONbd6e69020596: Rename unreachable strategy parameters to avoid API stutter.
rMARATHON3af58c657998: Rename unreachable strategy parameters to avoid API stutter.
rMARATHON61f3671be737: Rename unreachable strategy parameters to avoid API stutter.
rMARATHON1dc1a68f0131: Rename unreachable strategy parameters to avoid API stutter.
Summary

This renames unreachableInactiveAfter and
unreachbleExpungeAfter to inactiveAfter and expungeAfter to avoid
API stutter such as unreachableStrategy.unreachableInactiveAfter.

Closes #4794.

Test Plan

unit-test

Diff Detail

Repository
rMARATHON marathon
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
Changes from before your most recent comment are hidden. Show Older Changes
jeschkies added a comment.Dec 8 2016, 3:37 PM

@janisz could you try an sbt clean before.

janisz added a comment.Dec 8 2016, 3:41 PM

My bad. sbt clean resolved the issue.

janisz added a comment.Dec 8 2016, 4:34 PM

Can't run Marathon with this commit due to flood of following exception:

Caused by: play.api.libs.json.JsResultException: JsResultException(errors:List((/unreachableStrategy/inactiveAfter,List(ValidationError(List(error.path.missing),WrappedArray()))), (/unreachableStrategy/expungeAfter,List(ValidationError(List(error.path.missing),WrappedArray())))))
meichstedt added a comment.Dec 9 2016, 5:20 PM

@janisz Your marathon state seems to contain instances with an unreachableStrategy including the previous Json representation (different property names). Instances are serialized to JSON and stored as a byte string; Unfortunately, we do not have a migration from snapshot to snapshot in place so a state that was created with a snapshot previous to this one cannot be used :|

Revision Contents

Diff 1153

View Options

docs/docs/rest-api/public/api/v2/schema/AppDefinition.json

Show First 20 Lines • Show All 745 Lines • ▼ Show 20 Line(s)
746746 "killSelection": {
747747 "type": "string",
748748 "enum": ["YoungestFirst", "OldestFirst"],
749749 "default": "YoungestFirst",
750750 "description": "Defines which instance should be killed first in case of e.g. rescaling."
751751 },
752752 "unreachableStrategy": {
753753 "type": "object",
754 "description": "Define handling for unreachable instances. Given `unreachableInactiveAfter = 60` and `unreachableExpungeAfter = 120`, an instance will be expunged if it has been unreachable for more than 120 seconds or a second instance is started if it has been unreachable for more than 60 seconds.",
754 "description": "Define handling for unreachable instances. Given `inactiveAfter = 60` and `expungeAfter = 120`, an instance will be expunged if it has been unreachable for more than 120 seconds or a second instance is started if it has been unreachable for more than 60 seconds.",
755755 "additionalProperties": false,
756756 "properties": {
757 "unreachableInactiveAfterSeconds": {
757 "inactiveAfterSeconds": {
758758 "type": "integer",
759 "description": "If an instance is unreachable for longer than unreachableInactiveAfter seconds it is marked as inactive. This will trigger a new instance launch. The original task is not expunged yet.",
759 "description": "If an instance is unreachable for longer than inactiveAfter seconds it is marked as inactive. This will trigger a new instance launch. The original task is not expunged yet.",
760760 "minimum": 1
761761 },
762 "unreachableExpungeAfterSeconds": {
762 "expungeAfterSeconds": {
763763 "type": "number",
764 "description": "If an instance is unreachable for longer than unreachableExpungeAfter seconds it will be expunged. That means it will be killed if it ever comes back. Instances are usually marked as unreachable before they are expunged but they don't have to.",
764 "description": "If an instance is unreachable for longer than expungeAfter seconds it will be expunged. That means it will be killed if it ever comes back. Instances are usually marked as unreachable before they are expunged but they don't have to.",
765765 "minimum": 2
766766 }
767767 }
768768 }
769769 },
770770 "required": [
771771 "id"
772772 ],
773773 "type": "object"
774774}
View Options

docs/docs/rest-api/public/api/v2/types/unreachableStrategy.raml

11#%RAML 1.0 Library
22types:
33 UnreachableStrategy:
44 type: object
55 properties:
6 unreachableInactiveAfterSeconds?:
6 inactiveAfterSeconds?:
77 type: integer
88 format: int64
99 default: 300
1010 minimum: 1
1111 description: |
12 If an instance is unreachable for longer than unreachableInactiveAfter seconds it is marked
12 If an instance is unreachable for longer than inactiveAfter seconds it is marked
1313 as inactive. This will trigger a new instance launch. The original task is not
1414 expunged yet.
15 unreachableExpungeAfterSeconds?:
15 expungeAfterSeconds?:
1616 type: integer
1717 format: int64
1818 default: 600
1919 minimum: 1
2020 description: |
2121 If an instance is unreachable for longer than unreachableExpungeAfter seconds it will be expunged.
2222 That means it will be killed if it ever comes back. Instances are usually marked as
2323 unreachable before they are expunged but they don't have to.
View Options

src/main/scala/mesosphere/marathon/core/instance/Instance.scala

Show First 20 Lines • Show All 162 Lines • ▼ Show 20 Line(s)
163163 override def version: Timestamp = runSpecVersion
164164
165165 override def hostname: String = agentInfo.host
166166
167167 override def attributes: Seq[Attribute] = agentInfo.attributes
168168
169169 private[instance] def updatedInstance(updatedTask: Task, now: Timestamp): Instance = {
170170 val updatedTasks = tasksMap.updated(updatedTask.taskId, updatedTask)
171 copy(tasksMap = updatedTasks, state = Instance.InstanceState(Some(state), updatedTasks, now, unreachableStrategy.unreachableInactiveAfter))
171 copy(tasksMap = updatedTasks, state = Instance.InstanceState(Some(state), updatedTasks, now, unreachableStrategy.inactiveAfter))
172172 }
173173}
174174
175175@SuppressWarnings(Array("DuplicateImport"))
176176object Instance {
177177
178178 import mesosphere.marathon.api.v2.json.Formats.TimestampFormat
179179
▲ Show 20 Lines • Show All 311 Lines • Show Last 20 Lines
View Options

src/main/scala/mesosphere/marathon/core/task/jobs/impl/ExpungeOverdueLostTasksActor.scala

Show All 33 Lines
3434
3535 /**
3636 * @return instances that have been UnreachableInactive for more than half of [[mesosphere.marathon.core.task.jobs.TaskJobsConfig.taskLostExpungeGC]] millis.
3737 */
3838 def filterOverdueUnreachableInactive(instances: Map[PathId, SpecInstances], now: Timestamp) =
3939 instances.values.flatMap(_.instances)
4040 .withFilter(_.isUnreachableInactive)
4141 .withFilter { instance =>
42 val unreachableExpungeAfter = instance.unreachableStrategy.unreachableExpungeAfter
42 val unreachableExpungeAfter = instance.unreachableStrategy.expungeAfter
4343 instance.tasksMap.valuesIterator.exists(_.isUnreachableExpired(now, unreachableExpungeAfter))
4444 }
4545}
4646
4747class ExpungeOverdueLostTasksActor(
4848 val clock: Clock,
4949 val config: TaskJobsConfig,
5050 instanceTracker: InstanceTracker,
Show All 35 Lines
View Options

src/main/scala/mesosphere/marathon/raml/UnreachableStrategyConversion.scala

11package mesosphere.marathon
22package raml
33
44import scala.concurrent.duration._
55
66/**
77 * Conversion from [[mesosphere.marathon.state.UnreachableStrategy]] to [[mesosphere.marathon.raml.UnreachableStrategy]].
88 */
99trait UnreachableStrategyConversion {
1010
11 implicit val ramlUnreachableStrategyRead = Reads[UnreachableStrategy, state.UnreachableStrategy] { handling =>
11 implicit val ramlUnreachableStrategyRead = Reads[UnreachableStrategy, state.UnreachableStrategy] { strategy =>
1212 state.UnreachableStrategy(
13 unreachableInactiveAfter = handling.unreachableInactiveAfterSeconds.seconds,
14 unreachableExpungeAfter = handling.unreachableExpungeAfterSeconds.seconds)
13 inactiveAfter = strategy.inactiveAfterSeconds.seconds,
14 expungeAfter = strategy.expungeAfterSeconds.seconds)
1515 }
1616
17 implicit val ramlUnreachableStrategyWrite = Writes[state.UnreachableStrategy, UnreachableStrategy]{ handling =>
17 implicit val ramlUnreachableStrategyWrite = Writes[state.UnreachableStrategy, UnreachableStrategy]{ strategy =>
1818 UnreachableStrategy(
19 unreachableInactiveAfterSeconds = handling.unreachableInactiveAfter.toSeconds,
20 unreachableExpungeAfterSeconds = handling.unreachableExpungeAfter.toSeconds)
19 inactiveAfterSeconds = strategy.inactiveAfter.toSeconds,
20 expungeAfterSeconds = strategy.expungeAfter.toSeconds)
2121 }
2222}
2323
2424object UnreachableStrategyConversion extends UnreachableStrategyConversion
View Options

src/main/scala/mesosphere/marathon/state/UnreachableStrategy.scala

11package mesosphere.marathon
22package state
33
44import com.wix.accord.dsl._
55
66import scala.concurrent.duration._
77
88/**
99 * Defines the time outs for unreachable tasks.
1010 */
1111case class UnreachableStrategy(
12 unreachableInactiveAfter: FiniteDuration = UnreachableStrategy.DefaultTimeUntilInactive,
13 unreachableExpungeAfter: FiniteDuration = UnreachableStrategy.DefaultTimeUntilExpunge)
12 inactiveAfter: FiniteDuration = UnreachableStrategy.DefaultTimeUntilInactive,
13 expungeAfter: FiniteDuration = UnreachableStrategy.DefaultTimeUntilExpunge)
1414
1515object UnreachableStrategy {
1616 val DefaultTimeUntilInactive = 3.minutes
1717 val DefaultTimeUntilExpunge = 6.minutes
1818
1919 implicit val unreachableStrategyValidator = validator[UnreachableStrategy] { strategy =>
20 strategy.unreachableInactiveAfter should be >= 1.second
21 strategy.unreachableInactiveAfter should be < strategy.unreachableExpungeAfter
20 strategy.inactiveAfter should be >= 1.second
21 strategy.inactiveAfter should be < strategy.expungeAfter
2222 }
2323}
View Options

src/test/scala/mesosphere/marathon/api/v2/json/AppDefinitionFormatsTest.scala

Show First 20 Lines • Show All 443 Lines • ▼ Show 20 Line(s)
444444 (json \ "secrets" \ "secret3" \ "source").as[String] should equal("/foo2")
445445 }
446446
447447 test("FromJSON should parse unreachable instance strategy") {
448448 val appDef = Json.parse(
449449 """{
450450 | "id": "test",
451451 | "unreachableStrategy": {
452 | "unreachableInactiveAfterSeconds": 600,
453 | "unreachableExpungeAfterSeconds": 1200
452 | "inactiveAfterSeconds": 600,
453 | "expungeAfterSeconds": 1200
454454 | }
455455 |}""".stripMargin).as[AppDefinition]
456456
457 appDef.unreachableStrategy.unreachableInactiveAfter should be(10.minutes)
458 appDef.unreachableStrategy.unreachableExpungeAfter should be(20.minutes)
457 appDef.unreachableStrategy.inactiveAfter should be(10.minutes)
458 appDef.unreachableStrategy.expungeAfter should be(20.minutes)
459459 }
460460
461461 test("ToJSON should serialize unreachable instance strategy") {
462462 val strategy = UnreachableStrategy(6.minutes, 12.minutes)
463463 val appDef = AppDefinition(id = PathId("test"), unreachableStrategy = strategy)
464464
465465 val json = Json.toJson(appDef)
466466
467 (json \ "unreachableStrategy" \ "unreachableInactiveAfterSeconds").as[Long] should be(360)
468 (json \ "unreachableStrategy" \ "unreachableExpungeAfterSeconds").as[Long] should be(720)
467 (json \ "unreachableStrategy" \ "inactiveAfterSeconds").as[Long] should be(360)
468 (json \ "unreachableStrategy" \ "expungeAfterSeconds").as[Long] should be(720)
469469 }
470470
471471 test("FromJSON should parse kill selection") {
472472 val appDef = Json.parse(
473473 """{
474474 | "id": "test",
475475 | "killSelection": "YoungestFirst"
476476 |}""".stripMargin).as[AppDefinition]
Show All 23 Lines
View Options

src/test/scala/mesosphere/marathon/core/instance/InstanceFormatTest.scala

11package mesosphere.marathon
22package core.instance
33
44import mesosphere.UnitTest
55import mesosphere.marathon.state.UnreachableStrategy
66import play.api.libs.json._
77
88import scala.concurrent.duration._
99
1010class InstanceFormatTest extends UnitTest {
1111
1212 import Instance._
1313
1414 "Instance.unreachableStrategyFormat" should {
1515 "parse a proper JSON" in {
16 val json = Json.parse("""{ "unreachableInactiveAfter": 1, "unreachableExpungeAfter": 2 }""")
17 json.as[UnreachableStrategy].unreachableInactiveAfter should be(1.second)
18 json.as[UnreachableStrategy].unreachableExpungeAfter should be(2.seconds)
16 val json = Json.parse("""{ "inactiveAfter": 1, "expungeAfter": 2 }""")
17 json.as[UnreachableStrategy].inactiveAfter should be(1.second)
18 json.as[UnreachableStrategy].expungeAfter should be(2.seconds)
1919 }
2020
2121 "not parse a JSON with empty fields" in {
2222 val json = Json.parse("""{ "unreachableExpungeAfter": 2 }""")
2323 a[JsResultException] should be thrownBy { json.as[UnreachableStrategy] }
2424 }
2525
2626 }
2727
2828 "Instance.instanceFormat" should {
2929 "fill UnreachableStrategy with defaults if empty" in {
3030 val json = Json.parse(
3131 """{ "instanceId": { "idString": "app.instance-1337" },
3232 | "tasksMap": {},
3333 | "runSpecVersion": "2015-01-01",
3434 | "agentInfo": { "host": "localhost", "attributes": [] },
3535 | "state": { "since": "2015-01-01", "condition": { "str": "Running" } }
3636 |}""".stripMargin)
3737 val instance = json.as[Instance]
3838
39 instance.unreachableStrategy.unreachableInactiveAfter should be(UnreachableStrategy.DefaultTimeUntilInactive)
40 instance.unreachableStrategy.unreachableExpungeAfter should be(UnreachableStrategy.DefaultTimeUntilExpunge)
39 instance.unreachableStrategy.inactiveAfter should be(UnreachableStrategy.DefaultTimeUntilInactive)
40 instance.unreachableStrategy.expungeAfter should be(UnreachableStrategy.DefaultTimeUntilExpunge)
4141 }
4242 }
4343}
View Options

src/test/scala/mesosphere/marathon/core/instance/InstanceUpdateTest.scala

Show First 20 Lines • Show All 64 Lines • ▼ Show 20 Line(s)
6565 }
6666
6767 }
6868
6969 "updated to an expired unreachable" should {
7070
7171 val f = new Fixture
7272
73 val unreachableInactiveAfter = f.instance.unreachableStrategy.unreachableInactiveAfter
73 val unreachableInactiveAfter = f.instance.unreachableStrategy.inactiveAfter
7474 val newMesosStatus = MesosTaskStatusTestHelper.unreachable(f.taskId, since = f.clock.now())
7575
7676 // Forward time to expire unreachable status
7777 f.clock += unreachableInactiveAfter + 1.minute
7878
7979 val operation = InstanceUpdateOperation.MesosUpdate(f.instance, newMesosStatus, f.clock.now())
8080
8181 val result = f.instance.update(operation)
▲ Show 20 Lines • Show All 122 Lines • ▼ Show 20 Line(s)
204204
205205 val f = new Fixture
206206
207207 // Setup unreachable instance with a unreachable task
208208 val mesosTaskStatus = MesosTaskStatusTestHelper.unreachable(f.taskId, since = f.clock.now())
209209 val unreachableStatus = f.taskStatus.copy(startedAt = None, condition = Condition.Unreachable, mesosStatus = Some(mesosTaskStatus))
210210 val unreachableTask = f.task.copy(status = unreachableStatus)
211211 val unreachableState = f.instanceState.copy(condition = Condition.Unreachable)
212 val unreachableStrategy = UnreachableStrategy(unreachableInactiveAfter = 30.minutes, unreachableExpungeAfter = 1.hour)
212 val unreachableStrategy = UnreachableStrategy(inactiveAfter = 30.minutes, expungeAfter = 1.hour)
213213 val unreachableInstance = f.instance.copy(
214214 tasksMap = Map(f.taskId -> unreachableTask),
215215 state = unreachableState,
216216 unreachableStrategy = unreachableStrategy)
217217
218218 // Move time forward
219219 f.clock += 5.minutes
220220
Show All 11 Lines
232232
233233 val f = new Fixture
234234
235235 // Setup unreachable instance with a unreachable task
236236 val mesosTaskStatus = MesosTaskStatusTestHelper.unreachable(f.taskId, since = f.clock.now())
237237 val unreachableStatus = f.taskStatus.copy(startedAt = None, condition = Condition.Unreachable, mesosStatus = Some(mesosTaskStatus))
238238 val unreachableTask = f.task.copy(status = unreachableStatus)
239239 val unreachableInactiveState = f.instanceState.copy(condition = Condition.UnreachableInactive)
240 val unreachableStrategy = UnreachableStrategy(unreachableInactiveAfter = 1.minute, unreachableExpungeAfter = 1.hour)
240 val unreachableStrategy = UnreachableStrategy(inactiveAfter = 1.minute, expungeAfter = 1.hour)
241241 val unreachableInactiveInstance = f.instance.copy(
242242 tasksMap = Map(f.taskId -> unreachableTask),
243243 state = unreachableInactiveState,
244244 unreachableStrategy = unreachableStrategy)
245245
246246 // Move time forward
247247 f.clock += 5.minutes
248248
▲ Show 20 Lines • Show All 82 Lines • Show Last 20 Lines
View Options

src/test/scala/mesosphere/marathon/core/task/jobs/ExpungeOverdueLostTasksActorTest.scala

Show First 20 Lines • Show All 57 Lines • ▼ Show 20 Line(s)
5858 }
5959
6060 // format: OFF
6161 // Different task configuration with startedAt, status since and condition values. Expunge indicates whether an
6262 // expunge is expected or not.
6363 val taskCases = Table(
6464 ("name", "startedAt", "since", "condition", "expunge"),
6565 ("running", Timestamp.zero, Timestamp.zero, Condition.Running, false ),
66 ("expired inactive", Timestamp.zero, f.clock.now - f.strategy.unreachableExpungeAfter - 1.minute, Condition.UnreachableInactive, true ),
67 ("unreachable", Timestamp.zero, f.clock.now - f.strategy.unreachableInactiveAfter, Condition.Unreachable, false )
66 ("expired inactive", Timestamp.zero, f.clock.now - f.strategy.expungeAfter - 1.minute, Condition.UnreachableInactive, true ),
67 ("unreachable", Timestamp.zero, f.clock.now - f.strategy.inactiveAfter, Condition.Unreachable, false )
6868 )
6969 // format: ON
7070
7171 forAll(taskCases) { (name: String, startedAt: Timestamp, since: Timestamp, condition: Condition, expunge: Boolean) =>
7272 s"filtering $name task since $since" should {
7373 val instance: Instance = (condition match {
7474 case Condition.Unreachable => TestInstanceBuilder.newBuilder("/unreachable".toPath).addTaskUnreachable(since = since).getInstance()
7575 case Condition.UnreachableInactive => TestInstanceBuilder.newBuilder("/unreachable".toPath).addTaskUnreachableInactive(since = since).getInstance()
▲ Show 20 Lines • Show All 121 Lines • Show Last 20 Lines
View Options

src/test/scala/mesosphere/marathon/raml/UnreachableStrategyConversionTest.scala

11package mesosphere.marathon
22package raml
33
44import mesosphere.marathon.state
55import mesosphere.UnitTest
66
77import scala.concurrent.duration._
88
99class UnreachableStrategyConversionTest extends UnitTest {
1010
1111 "UnreachableStrategyConversion" should {
1212 "read from RAML" in {
1313 val raml = UnreachableStrategy()
1414
1515 val result: state.UnreachableStrategy = UnreachableStrategyConversion.ramlUnreachableStrategyRead(raml)
1616
17 result.unreachableInactiveAfter should be(5.minutes)
18 result.unreachableExpungeAfter should be(10.minutes)
17 result.inactiveAfter should be(5.minutes)
18 result.expungeAfter should be(10.minutes)
1919 }
2020 }
2121
2222 it should {
2323 "write to RAML" in {
2424 val strategy = state.UnreachableStrategy(10.minutes, 20.minutes)
2525
2626 val raml: UnreachableStrategy = UnreachableStrategyConversion.ramlUnreachableStrategyWrite(strategy)
2727
28 raml.unreachableInactiveAfterSeconds should be(600)
29 raml.unreachableExpungeAfterSeconds should be(1200)
28 raml.inactiveAfterSeconds should be(600)
29 raml.expungeAfterSeconds should be(1200)
3030 }
3131 }
3232}
View Options

src/test/scala/mesosphere/marathon/state/UnreachableStrategyTest.scala

11package mesosphere.marathon
22package state
33
44import mesosphere.UnitTest
55import com.wix.accord.scalatest.ResultMatchers
66
77import scala.concurrent.duration._
88
99class UnreachableStrategyTest extends UnitTest with ResultMatchers {
1010
1111 "UnreachableStrategy.unreachableStrategyValidator" should {
1212 "validate default strategy" in {
1313 val strategy = UnreachableStrategy()
1414 UnreachableStrategy.unreachableStrategyValidator(strategy) shouldBe aSuccess
1515 }
1616
17 "validate with other paramters successfully" in {
17 "validate with other parameters successfully" in {
1818 val strategy = UnreachableStrategy(13.minutes, 37.minutes)
1919 UnreachableStrategy.unreachableStrategyValidator(strategy) shouldBe aSuccess
2020 }
2121
2222 "fail with invalid time until inactive" in {
23 val strategy = UnreachableStrategy(unreachableInactiveAfter = 0.second)
24 UnreachableStrategy.unreachableStrategyValidator(strategy) should failWith("unreachableInactiveAfter" -> "got 0 seconds, expected 1 second or more")
23 val strategy = UnreachableStrategy(inactiveAfter = 0.second)
24 UnreachableStrategy.unreachableStrategyValidator(strategy) should failWith("inactiveAfter" -> "got 0 seconds, expected 1 second or more")
2525 }
2626
2727 "fail when time until expunge is smaller" in {
28 val strategy = UnreachableStrategy(unreachableInactiveAfter = 2.seconds, unreachableExpungeAfter = 1.second)
29 UnreachableStrategy.unreachableStrategyValidator(strategy) should failWith("unreachableInactiveAfter" -> "got 2 seconds, expected less than 1 second")
28 val strategy = UnreachableStrategy(inactiveAfter = 2.seconds, expungeAfter = 1.second)
29 UnreachableStrategy.unreachableStrategyValidator(strategy) should failWith("inactiveAfter" -> "got 2 seconds, expected less than 1 second")
3030 }
3131
3232 "fail when time until expunge is equal to time until inactive" in {
33 val strategy = UnreachableStrategy(unreachableInactiveAfter = 2.seconds, unreachableExpungeAfter = 2.seconds)
34 UnreachableStrategy.unreachableStrategyValidator(strategy) should failWith("unreachableInactiveAfter" -> "got 2 seconds, expected less than 2 seconds")
33 val strategy = UnreachableStrategy(inactiveAfter = 2.seconds, expungeAfter = 2.seconds)
34 UnreachableStrategy.unreachableStrategyValidator(strategy) should failWith("inactiveAfter" -> "got 2 seconds, expected less than 2 seconds")
3535 }
3636 }
3737}