[
https://issues.apache.org/jira/browse/S2GRAPH-257?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Daewon Jeong updated S2GRAPH-257:
---------------------------------
Description:
The SafeUpdateCache Class needs to change the value in another thread when
requesting the value from the cache instance.
With the above operation, the current thread should not be blocked.
However, due to the bug in the line below, the lazy value is evaluated
unintentionally, causing the forground thread to block.
[https://github.com/apache/incubator-s2graph/blob/07a5af39a5a52aad2e61c34d685fe00a6bb8e2cc/s2core/src/main/scala/org/apache/s2graph/core/utils/SafeUpdateCache.scala#L158]
{code:scala}
def withCache[T <: AnyRef](key: String,
broadcast: Boolean,
cacheTTLInSecs: Option[Int] = None,
onEvict: AnyRef => Unit = defaultOnEvict)(op: =>
T): T = {
...
...
if (running) cachedVal
else {
val value = op // Bug on this line: value must be declared as lazy val
Future(value)(executionContext) onComplete {
case Failure(ex) =>
put(key, cachedVal, false)
logger.error(s"withCache update failed: $cacheKey", ex)
case Success(newValue) =>
put(key, newValue, broadcast = (broadcast && newValue !=
cachedVal))
onEvict(cachedVal)
cachedVal match {
case None =>
case _ => logger.info(s"withCache update success: $cacheKey")
}
}
cachedVal
}
}
}
}
{code}
was:
The SafeUpdateCache Class needs to change the value in another thread when
requesting the value from the cache instance.
With the above operation, the current thread should not be blocked.
However, due to the bug in the line below, the lazy value is evaluated
unintentionally, causing the forground thread to block.
[https://github.com/apache/incubator-s2graph/blob/07a5af39a5a52aad2e61c34d685fe00a6bb8e2cc/s2core/src/main/scala/org/apache/s2graph/core/utils/SafeUpdateCache.scala#L158]
{code:scaka}
def withCache[T <: AnyRef](key: String,
broadcast: Boolean,
cacheTTLInSecs: Option[Int] = None,
onEvict: AnyRef => Unit = defaultOnEvict)(op: =>
T): T = {
val cacheKey = toCacheKey(key)
val cachedValWithTs = cache.getIfPresent(cacheKey)
if (cachedValWithTs == null) {
val value = op
// fetch and update cache.
put(key, value, broadcast)
value
} else {
val (_cachedVal, updatedAt, isUpdating) = cachedValWithTs
val cachedVal = _cachedVal.asInstanceOf[T]
val ttl = cacheTTLInSecs.getOrElse(systemTtl)
val isValidCacheVal = toTs() < updatedAt + ttl
if (isValidCacheVal) cachedVal // in cache TTL
else {
val running = isUpdating.getAndSet(true)
if (running) cachedVal
else {
val value = op // <- value should declared lazy val
Future(value)(executionContext) onComplete {
case Failure(ex) =>
put(key, cachedVal, false)
logger.error(s"withCache update failed: $cacheKey", ex)
case Success(newValue) =>
put(key, newValue, broadcast = (broadcast && newValue !=
cachedVal))
onEvict(cachedVal)
cachedVal match {
case None =>
case _ => logger.info(s"withCache update success: $cacheKey")
}
}
cachedVal
}
}
}
}
{code}
> SafeUpdateCache#withCache method incorrect behavior
> ---------------------------------------------------
>
> Key: S2GRAPH-257
> URL: https://issues.apache.org/jira/browse/S2GRAPH-257
> Project: S2Graph
> Issue Type: Bug
> Components: s2core
> Affects Versions: 0.1.0, 0.2.0
> Reporter: Daewon Jeong
> Priority: Major
>
> The SafeUpdateCache Class needs to change the value in another thread when
> requesting the value from the cache instance.
> With the above operation, the current thread should not be blocked.
> However, due to the bug in the line below, the lazy value is evaluated
> unintentionally, causing the forground thread to block.
>
> [https://github.com/apache/incubator-s2graph/blob/07a5af39a5a52aad2e61c34d685fe00a6bb8e2cc/s2core/src/main/scala/org/apache/s2graph/core/utils/SafeUpdateCache.scala#L158]
>
>
> {code:scala}
> def withCache[T <: AnyRef](key: String,
> broadcast: Boolean,
> cacheTTLInSecs: Option[Int] = None,
> onEvict: AnyRef => Unit = defaultOnEvict)(op: =>
> T): T = {
> ...
> ...
> if (running) cachedVal
> else {
> val value = op // Bug on this line: value must be declared as lazy
> val
> Future(value)(executionContext) onComplete {
> case Failure(ex) =>
> put(key, cachedVal, false)
> logger.error(s"withCache update failed: $cacheKey", ex)
> case Success(newValue) =>
> put(key, newValue, broadcast = (broadcast && newValue !=
> cachedVal))
> onEvict(cachedVal)
> cachedVal match {
> case None =>
> case _ => logger.info(s"withCache update success: $cacheKey")
> }
> }
> cachedVal
> }
> }
> }
> }
> {code}
>
>
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
