Use inline func to fix deadlock

This commit is contained in:
Harry Zhang 2018-03-08 16:01:54 -08:00
parent 4e5901f947
commit 5cc841a337

View File

@ -430,10 +430,6 @@ func podFitsOnNode(
var ( var (
eCacheAvailable bool eCacheAvailable bool
failedPredicates []algorithm.PredicateFailureReason failedPredicates []algorithm.PredicateFailureReason
invalid bool
fit bool
reasons []algorithm.PredicateFailureReason
err error
) )
predicateResults := make(map[string]HostPredicate) predicateResults := make(map[string]HostPredicate)
@ -469,24 +465,37 @@ func podFitsOnNode(
// when pods are nominated or their nominations change. // when pods are nominated or their nominations change.
eCacheAvailable = equivCacheInfo != nil && !podsAdded eCacheAvailable = equivCacheInfo != nil && !podsAdded
for _, predicateKey := range predicates.Ordering() { for _, predicateKey := range predicates.Ordering() {
var (
fit bool
reasons []algorithm.PredicateFailureReason
err error
)
//TODO (yastij) : compute average predicate restrictiveness to export it as Prometheus metric //TODO (yastij) : compute average predicate restrictiveness to export it as Prometheus metric
if predicate, exist := predicateFuncs[predicateKey]; exist { if predicate, exist := predicateFuncs[predicateKey]; exist {
// Use an in-line function to guarantee invocation of ecache.Unlock()
// when the in-line function returns.
func() {
var invalid bool
if eCacheAvailable { if eCacheAvailable {
// Lock ecache here to avoid a race condition against cache invalidation invoked // Lock ecache here to avoid a race condition against cache invalidation invoked
// in event handlers. This race has existed despite locks in eCache implementation. // in event handlers. This race has existed despite locks in equivClassCacheimplementation.
ecache.Lock() ecache.Lock()
defer ecache.Unlock()
// PredicateWithECache will return its cached predicate results. // PredicateWithECache will return its cached predicate results.
fit, reasons, invalid = ecache.PredicateWithECache(pod.GetName(), info.Node().GetName(), predicateKey, equivCacheInfo.hash, false) fit, reasons, invalid = ecache.PredicateWithECache(
pod.GetName(), info.Node().GetName(),
predicateKey, equivCacheInfo.hash, false)
} }
if !eCacheAvailable || invalid { if !eCacheAvailable || invalid {
// we need to execute predicate functions since equivalence cache does not work // we need to execute predicate functions since equivalence cache does not work
fit, reasons, err = predicate(pod, metaToUse, nodeInfoToUse) fit, reasons, err = predicate(pod, metaToUse, nodeInfoToUse)
if err != nil { if err != nil {
return false, []algorithm.PredicateFailureReason{}, err return
} }
if eCacheAvailable { if eCacheAvailable {
// Store data to update eCache after this loop. // Store data to update equivClassCacheafter this loop.
if res, exists := predicateResults[predicateKey]; exists { if res, exists := predicateResults[predicateKey]; exists {
res.Fit = res.Fit && fit res.Fit = res.Fit && fit
res.FailReasons = append(res.FailReasons, reasons...) res.FailReasons = append(res.FailReasons, reasons...)
@ -495,12 +504,15 @@ func podFitsOnNode(
predicateResults[predicateKey] = HostPredicate{Fit: fit, FailReasons: reasons} predicateResults[predicateKey] = HostPredicate{Fit: fit, FailReasons: reasons}
} }
result := predicateResults[predicateKey] result := predicateResults[predicateKey]
ecache.UpdateCachedPredicateItem(pod.GetName(), info.Node().GetName(), predicateKey, result.Fit, result.FailReasons, equivCacheInfo.hash, false) ecache.UpdateCachedPredicateItem(
pod.GetName(), info.Node().GetName(),
predicateKey, result.Fit, result.FailReasons, equivCacheInfo.hash, false)
} }
} }
}()
if eCacheAvailable { if err != nil {
ecache.Unlock() return false, []algorithm.PredicateFailureReason{}, err
} }
if !fit { if !fit {
@ -508,7 +520,9 @@ func podFitsOnNode(
failedPredicates = append(failedPredicates, reasons...) failedPredicates = append(failedPredicates, reasons...)
// if alwaysCheckAllPredicates is false, short circuit all predicates when one predicate fails. // if alwaysCheckAllPredicates is false, short circuit all predicates when one predicate fails.
if !alwaysCheckAllPredicates { if !alwaysCheckAllPredicates {
glog.V(5).Infoln("since alwaysCheckAllPredicates has not been set, the predicate evaluation is short circuited and there are chances of other predicates failing as well.") glog.V(5).Infoln("since alwaysCheckAllPredicates has not been set, the predicate" +
"evaluation is short circuited and there are chances" +
"of other predicates failing as well.")
break break
} }
} }