HomeMesosphereNo notifications. 4 unresolved issues.
  • Search
Differential D301

Increase default expunge timeout tasks
ClosedAll Users
Actions

Authored by timcharper on Dec 8 2016, 10:05 PM.

Details

Reviewers
aquamatthias
jdef
• jasongilanfarr
meichstedt
Commits
rMARATHON99323e6ed85c: Increase default expunge timeout tasks
rMARATHON2bf6fd013532: Increase default expunge timeout tasks
rMARATHON12ca1235a69f: Increase default expunge timeout tasks
Summary

The default expunge time of 6 minutes is too aggressive and was leading
to persistent volumes getting deleted regular maintenance tasks such as
reboot a host.

Also, update text describing the expungeAfter to make sure it's clear
that it will result in volume deletion.

addresses #4603

Test Plan

n/a

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.
timcharper retitled this revision from to Increase default expunge timeout tasks.Dec 8 2016, 10:05 PM
timcharper updated this object.
timcharper edited the test plan for this revision. (Show Details)
timcharper added reviewers: jdef, • jasongilanfarr, meichstedt.
timcharper added a comment.Dec 8 2016, 10:12 PM

@meichstedt is 15 minutes enough for TimeUntilInactive? Or should we go higher? An hour?

jdef added a comment.Dec 9 2016, 12:00 AM

should we use longer defaults for stateful things vs. shorter defaults for stateless things?

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

since we're making changes in here, this description should indicate that inactiveAfterSeconds must be less than expungeAfterSeconds

18

it'd be nice to see a readable translation of this number in the description, like "the default of 604800 seconds translates to 7 days" (or some such)

timcharper added a comment.Dec 9 2016, 12:03 AM

I think using different defaults for resident tasks would be nice; My understanding is @meichstedt votes against it.

timcharper updated this revision to Diff 1171.Dec 9 2016, 12:25 AM

describe default. Clarify that inactiveAfterSeconds is < expungeAfterSeconds

timcharper marked 2 inline comments as done.Dec 9 2016, 12:27 AM

done

jdef accepted this revision.Dec 9 2016, 1:25 AM
This revision is now accepted and ready to land.Dec 9 2016, 1:25 AM
meichstedt accepted this revision.EditedDec 9 2016, 11:32 AM

Thanks. We need to update the docs as well, see https://github.com/mesosphere/marathon/issues/4816

meichstedt updated this revision to Diff 1178.Dec 9 2016, 12:29 PM

Fixed unit test

timcharper added a comment.Dec 9 2016, 4:30 PM

Thank you for fixing the test! @meichstedt

Closed by commit rMARATHON12ca1235a69f: Increase default expunge timeout tasks (authored by timcharper). · Explain WhyDec 9 2016, 4:49 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

Diff 1188

View Options

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

Show First 20 Lines • Show All 750 Lines • ▼ Show 20 Line(s)
751751 },
752752 "unreachableStrategy": {
753753 "type": "object",
754754 "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": {
757757 "inactiveAfterSeconds": {
758758 "type": "integer",
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.",
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. Must be less than expungeAfterSeconds.",
760760 "minimum": 1
761761 },
762762 "expungeAfterSeconds": {
763763 "type": "number",
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.",
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. This value is required to be greater than inactiveAfterSeconds.\n\nIf the instance has any persistent volumes associated with it, then they will be destroyed and associated data will be deleted.\n\nDefault is 7 days (604800 seconds).",
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:
66 inactiveAfterSeconds?:
77 type: integer
88 format: int64
9 default: 300
9 default: 900
1010 minimum: 1
1111 description: |
1212 If an instance is unreachable for longer than inactiveAfter seconds it is marked
jdefUnsubmitted
Done

since we're making changes in here, this description should indicate that inactiveAfterSeconds must be less than expungeAfterSeconds

1313 as inactive. This will trigger a new instance launch. The original task is not
14 expunged yet.
14 expunged yet. Must be less than expungeAfterSeconds.
1515 expungeAfterSeconds?:
1616 type: integer
1717 format: int64
18 default: 600
18 default: 604800
jdefUnsubmitted
Done

it'd be nice to see a readable translation of this number in the description, like "the default of 604800 seconds translates to 7 days" (or some such)

1919 minimum: 1
2020 description: |
21 If an instance is unreachable for longer than unreachableExpungeAfter seconds it will be expunged.
22 That means it will be killed if it ever comes back. Instances are usually marked as
23 unreachable before they are expunged but they don't have to.
21 If an instance is unreachable for longer than unreachableExpungeAfter seconds it will be expunged. That means
22 it will be killed if it ever comes back. Instances are usually marked as unreachable before they are expunged
23 but they don't have to. This value is required to be greater than inactiveAfterSeconds.
24
25 If the instance has any persistent volumes associated with it, then they will be destroyed and associated data
26 will be deleted.
27
28 Default is 7 days (604800 seconds).
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 inactiveAfter: FiniteDuration = UnreachableStrategy.DefaultTimeUntilInactive,
13 expungeAfter: FiniteDuration = UnreachableStrategy.DefaultTimeUntilExpunge)
12 inactiveAfter: FiniteDuration = UnreachableStrategy.DefaultInactiveAfter,
13 expungeAfter: FiniteDuration = UnreachableStrategy.DefaultExpungeAfter)
1414
1515object UnreachableStrategy {
16 val DefaultTimeUntilInactive = 3.minutes
17 val DefaultTimeUntilExpunge = 6.minutes
16 val DefaultInactiveAfter: FiniteDuration = 15.minutes
17 val DefaultExpungeAfter: FiniteDuration = 7.days
1818
1919 implicit val unreachableStrategyValidator = validator[UnreachableStrategy] { strategy =>
2020 strategy.inactiveAfter should be >= 1.second
2121 strategy.inactiveAfter should be < strategy.expungeAfter
2222 }
2323}
View Options

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

Show All 30 Lines
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.inactiveAfter should be(UnreachableStrategy.DefaultTimeUntilInactive)
40 instance.unreachableStrategy.expungeAfter should be(UnreachableStrategy.DefaultTimeUntilExpunge)
39 instance.unreachableStrategy.inactiveAfter should be(UnreachableStrategy.DefaultInactiveAfter)
40 instance.unreachableStrategy.expungeAfter should be(UnreachableStrategy.DefaultExpungeAfter)
4141 }
4242 }
4343}
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.inactiveAfter should be(5.minutes)
18 result.expungeAfter should be(10.minutes)
17 result.inactiveAfter should be(state.UnreachableStrategy.DefaultInactiveAfter)
18 result.expungeAfter should be(state.UnreachableStrategy.DefaultExpungeAfter)
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
2828 raml.inactiveAfterSeconds should be(600)
2929 raml.expungeAfterSeconds should be(1200)
3030 }
3131 }
3232}