HomeMesosphereNo notifications. 4 unresolved issues.
  • Search
Differential D600

Raise lock on a run spec for any deployment.
ClosedAll Users
Actions

Authored by jeschkies on Mar 13 2017, 4:25 PM.

Details

Reviewers
zen-dog
unterstein
jenkins
• jasongilanfarr
Commits
rMARATHON94b29c31bd0a: Raise lock on a run spec for any deployment.
JIRA Issues
JIRA MARATHON-2114 GroupDeployIntegrationTest is flaky
JIRA MARATHON-7040 Conflicting Deployment Without Force Removes RunSpec Lock
Summary

This finally closes MARATHON-2114. The deployment of v3 in the group deploy
integration test fails and lowers the lock on the run spec. However, the lock
has not been increased before thus removing the lock completly.

Test Plan

integration-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.
jeschkies created this revision.Mar 13 2017, 4:25 PM
unterstein accepted this revision.Mar 13 2017, 4:29 PM
This revision is now accepted and ready to land.Mar 13 2017, 4:29 PM
zen-dog accepted this revision.Mar 13 2017, 4:30 PM

Thx for catching this!

jenkins requested changes to this revision.Mar 13 2017, 4:31 PM
This revision now requires changes to proceed.Mar 13 2017, 4:31 PM
jdef added a subscriber: jdef.Mar 13 2017, 4:39 PM
jdef added inline comments.
src/main/scala/mesosphere/marathon/MarathonSchedulerActor.scala
246

should we actually log a warning if locks goes negative?

jenkins accepted this revision.Mar 13 2017, 4:56 PM

(ﾉ◕ヮ◕)ﾉ*: It's Green :*ヽ(◕ヮ◕ヽ)

This revision is now accepted and ready to land.Mar 13 2017, 4:56 PM
jeschkies marked an inline comment as done.Mar 13 2017, 5:01 PM
jeschkies added inline comments.
src/main/scala/mesosphere/marathon/MarathonSchedulerActor.scala
246

Seems locks cannot be negative since we remove it from lockedRunSpecs before.

jeschkies marked 2 inline comments as done.Mar 13 2017, 5:01 PM
jeschkies added 2 JIRA issue(s): MARATHON-7040, MARATHON-2114.Mar 13 2017, 5:14 PM
• jasongilanfarr accepted this revision.Mar 13 2017, 5:29 PM
• jasongilanfarr added a subscriber: • jasongilanfarr.

Seems reasonable to me.

Closed by commit rMARATHON94b29c31bd0a: Raise lock on a run spec for any deployment. (authored by jeschkies). · Explain WhyMar 13 2017, 6:16 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

Diff 2236

View Options

src/main/scala/mesosphere/marathon/MarathonSchedulerActor.scala

Show First 20 Lines • Show All 237 Lines • ▼ Show 20 Line(s)
238238 conflicts.isEmpty
239239 }
240240
241241 def removeLocks(runSpecIds: Set[PathId]): Unit = runSpecIds.foreach(removeLock)
242242 def removeLock(runSpecId: PathId): Unit = {
243243 if (lockedRunSpecs.contains(runSpecId)) {
244244 val locks = lockedRunSpecs(runSpecId) - 1
245245 if (locks <= 0) lockedRunSpecs -= runSpecId else lockedRunSpecs(runSpecId) -= 1
246 logger.debug(s"Removed lock for run spec: id=$runSpecId locks=$locks lockedRunSpec=$lockedRunSpecs")
jdefUnsubmitted

should we actually log a warning if locks goes negative?

jeschkiesAuthorUnsubmitted

Seems locks cannot be negative since we remove it from lockedRunSpecs before.

246247 }
247248 }
248249
249250 def addLocks(runSpecIds: Set[PathId]): Unit = runSpecIds.foreach(addLock)
250 def addLock(runSpecId: PathId): Unit = lockedRunSpecs(runSpecId) += 1
251 def addLock(runSpecId: PathId): Unit = {
252 lockedRunSpecs(runSpecId) += 1
253 logger.debug(s"Added to lock for run spec: id=$runSpecId locks=${lockedRunSpecs(runSpecId)} lockedRunSpec=$lockedRunSpecs")
254 }
251255
252256 /**
253257 * Tries to acquire the lock for the given runSpecId.
254258 * If it succeeds it executes the given function,
255259 * otherwise the result will contain an AppLockedException.
256260 */
257261 def withLockFor[A](runSpecId: PathId)(f: => A): Try[A] =
258262 withLockFor(Set(runSpecId))(f)
259263
260264 // there has to be a better way...
261265 @SuppressWarnings(Array("OptionGet"))
262266 def driver: SchedulerDriver = marathonSchedulerDriverHolder.driver.get
263267
264268 def deploy(origSender: ActorRef, cmd: Deploy): Unit = {
265269 val plan = cmd.plan
266270 val runSpecIds = plan.affectedRunSpecIds
267271
268 // If there are no conflicting locks or the deployment is forced we lock passed runSpecIds.
272 // We raise the lock in any case for a deployment attempt.
269273 // Afterwards the deployment plan is sent to DeploymentManager. It will take care of cancelling
270274 // conflicting deployments, scheduling new one or (if there were conflicts but the deployment
271 // is not forced) send to the original sender and AppLockedException with conflicting deployments.
275 // is not forced) send to the original sender an AppLockedException with conflicting deployments.
272276 //
273277 // If a deployment is forced (and there exists an old one):
274278 // - the new deployment will be started
275279 // - the old deployment will be cancelled and release all claimed locks
276 // - only in this case, one RunSpec can have 2 locks
277 if (noConflictsWith(runSpecIds) || cmd.force) {
280 //
281 // In the case of a DeploymentFinished or DeploymentFailed we lower the lock again. See the receiving methods
278282 addLocks(runSpecIds)
279 }
280283
281284 deploymentManager.start(plan, cmd.force, origSender).onComplete{
282285 case Success(_) => self ! DeploymentFinished(plan)
283286 case Failure(t) => self ! DeploymentFailed(plan, t)
284287 }
285288 }
286289
287290 def deploymentSuccess(plan: DeploymentPlan): Unit = {
▲ Show 20 Lines • Show All 261 Lines • Show Last 20 Lines