cloudfoundry / diego-notes

Diego Notes
Apache License 2.0
23 stars 7 forks source link

Better LRP Deduping #24

Closed onsi closed 9 years ago

onsi commented 9 years ago

A proposal to improve how we dedupe multiple LRPs based on feedback from our performance tests:

https://github.com/pivotal-cf-experimental/diego-dev-notes/blob/master/proposals/better-lrp-deduping.md

fraenkel commented 9 years ago

Booo... Given the choice of an old or a new container, one would always prefer new. Over time processes get old. Now I realize you are trying to solve a different issue, but its not always the preferred behavior.

Why would we need the Container-Creation-Time when we have AllocatedAt on the Container already? I realize they aren't the same thing but...

We could also see which indexes are running and ignore auctions for those indexes. We only count because that is what we needed but we have all the info to do something better/smarter.

onsi commented 9 years ago

@fraenkel I did consider most of these but didn't want to propose something too complicated. Happy to amend that if we'd like. At some level, of course, once we fix the underlying problem (spurious disappearances) this becomes less important:

Given the choice of an old or a new container, one would always prefer new. Over time processes get old. Now I realize you are trying to solve a different issue, but its not always the preferred behavior.

Yes, but this isn't the case of "the user has requested a new container" rather "we goofed and have two containers". With that said it might make sense to introduce an upper threshold here (though this complicates things somewhat and is kind of arbitrary). Something like: "If I have duplicates I'm going to prefer the older one unless the newer one has been around for > X minutes". (where X is.... 5?) Why? Well if the old one has been missing for a while then maybe there's something actually going on with that Cell that we should worry about? Again: kind of arbitrary and not particularly convincing.

Why would we need the Container-Creation-Time when we have AllocatedAt on the Container already? I realize they aren't the same thing but...

We have AllocatedAt on the container but not on the ActualLRP. We'd need it on the ActualLRP. And, yes, if we had AllocatedAt we wouldn't need Container-Creation-Time.

We could also see which indexes are running and ignore auctions for those indexes. We only count because that is what we needed but we have all the info to do something better/smarter.

I do agree with this and thought of including it. Truth is it wouldn't solve the case where the Cell hadn't returned by the time the auction ran -- but it would help in cases where the Cell was in fact back. It also wouldn't be too hard to add. I'm happy to make this part of the proposal if folks like. I confess, it feels a little weird to me for the Auctioneer to concern itself with this: instead of just doing as he's told he's now also double-checking... but I can get over that :)

fraenkel commented 9 years ago

Can we just hold off until we identify the root cause?

There are many ways to solve this most of which do not require us to add any data. The Rep has access to the Allocation Time and the Since in the ActualLRP should reflect the Running state switch time.

emalm commented 9 years ago

I'm also wary of adding more fields to the BBS models and more complicated logic around contention for LRP survival, especially until we determine the root cause for the brief cell disappearances, and I too am concerned about the system being set up to prefer older containers. Perhaps we should consider more general mechanisms for rebalancing the container load, as this would help the system attain a more balanced distribution after a genuine cell crash or after evacuation in addition to this case.

sykesm commented 9 years ago

I also think looking at general rebalancing mechanisms makes a lot of sense. There are a number of recovery scenarios and rolling update scenarios that would benefit from it.

onsi commented 9 years ago

Y'all are convincing me. Looking at rebalancing makes sense (I can work on a proposal to give us a starting point).

Are any of the ideas that have come up in this proposal of value? What about having the auctioneer not schedule ActualLRPs that it sees are already running? That would have also helped in the cases we saw.

onsi commented 9 years ago

I'm going to kill this one for now. Will add something about rebalancing in the future.