chetanmeh closed pull request #4104: Cache empty auth results to reduce db load
URL: https://github.com/apache/incubator-openwhisk/pull/4104
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/common/scala/src/main/scala/org/apache/openwhisk/core/database/MultipleReadersSingleWriterCache.scala
b/common/scala/src/main/scala/org/apache/openwhisk/core/database/MultipleReadersSingleWriterCache.scala
index 1b4e46c910..7de4f3b692 100644
---
a/common/scala/src/main/scala/org/apache/openwhisk/core/database/MultipleReadersSingleWriterCache.scala
+++
b/common/scala/src/main/scala/org/apache/openwhisk/core/database/MultipleReadersSingleWriterCache.scala
@@ -17,18 +17,16 @@
package org.apache.openwhisk.core.database
-import java.util.concurrent.TimeUnit
import java.util.concurrent.atomic.AtomicReference
+import java.util.concurrent.{ConcurrentMap, TimeUnit}
-import scala.concurrent.{ExecutionContext, Future, Promise}
-import scala.util.Failure
-import scala.util.Success
import com.github.benmanes.caffeine.cache.Caffeine
-import org.apache.openwhisk.common.Logging
-import org.apache.openwhisk.common.LoggingMarkers
-import org.apache.openwhisk.common.TransactionId
+import org.apache.openwhisk.common.{Logging, LoggingMarkers, TransactionId}
import org.apache.openwhisk.core.entity.CacheKey
+import scala.concurrent.{ExecutionContext, Future, Promise}
+import scala.util.{Failure, Success}
+
/**
* A cache that allows multiple readers, but only a single writer, at
* a time. It will make a best effort attempt to coalesce reads, but
@@ -90,12 +88,13 @@ case object AccessTime extends EvictionPolicy
case object WriteTime extends EvictionPolicy
trait MultipleReadersSingleWriterCache[W, Winfo] {
- import MultipleReadersSingleWriterCache._
import MultipleReadersSingleWriterCache.State._
+ import MultipleReadersSingleWriterCache._
/** Subclasses: Toggle this to enable/disable caching for your entity type.
*/
protected val cacheEnabled = true
protected val evictionPolicy: EvictionPolicy = AccessTime
+ protected val fixedCacheSize = 0
private object Entry {
def apply(transid: TransactionId, state: State, value: Option[Future[W]]):
Entry = {
@@ -445,25 +444,19 @@ trait MultipleReadersSingleWriterCache[W, Winfo] {
}
/** This is the backing store. */
- private lazy val cache: ConcurrentMapBackedCache[Entry] = evictionPolicy
match {
- case AccessTime =>
- new ConcurrentMapBackedCache(
- Caffeine
- .newBuilder()
- .asInstanceOf[Caffeine[Any, Future[Entry]]]
- .expireAfterAccess(5, TimeUnit.MINUTES)
- .softValues()
- .build()
- .asMap())
-
- case _ =>
- new ConcurrentMapBackedCache(
- Caffeine
- .newBuilder()
- .asInstanceOf[Caffeine[Any, Future[Entry]]]
- .expireAfterWrite(5, TimeUnit.MINUTES)
- .softValues()
- .build()
- .asMap())
+ private lazy val cache: ConcurrentMapBackedCache[Entry] = createCache()
+
+ private def createCache() = {
+ val b = Caffeine
+ .newBuilder()
+ .softValues()
+
+ evictionPolicy match {
+ case AccessTime => b.expireAfterAccess(5, TimeUnit.MINUTES)
+ case _ => b.expireAfterWrite(5, TimeUnit.MINUTES)
+ }
+
+ if (fixedCacheSize > 0) b.maximumSize(fixedCacheSize)
+ new
ConcurrentMapBackedCache(b.build().asMap().asInstanceOf[ConcurrentMap[Any,
Future[Entry]]])
}
}
diff --git
a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Identity.scala
b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Identity.scala
index 7f3d0fd32a..f1833aa37f 100644
---
a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Identity.scala
+++
b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/Identity.scala
@@ -17,17 +17,19 @@
package org.apache.openwhisk.core.entity
+import org.apache.openwhisk.common.{Logging, TransactionId}
+import org.apache.openwhisk.core.database.{
+ MultipleReadersSingleWriterCache,
+ NoDocumentException,
+ StaleParameter,
+ WriteTime
+}
+import org.apache.openwhisk.core.entitlement.Privilege
+import org.apache.openwhisk.core.entity.types.AuthStore
+import spray.json._
+
import scala.concurrent.Future
import scala.util.Try
-import spray.json._
-import types.AuthStore
-import org.apache.openwhisk.common.Logging
-import org.apache.openwhisk.common.TransactionId
-import org.apache.openwhisk.core.database.MultipleReadersSingleWriterCache
-import org.apache.openwhisk.core.database.NoDocumentException
-import org.apache.openwhisk.core.database.StaleParameter
-import org.apache.openwhisk.core.database.WriteTime
-import org.apache.openwhisk.core.entitlement.Privilege
case class UserLimits(invocationsPerMinute: Option[Int] = None,
concurrentInvocations: Option[Int] = None,
@@ -50,12 +52,15 @@ protected[core] case class Identity(subject: Subject,
rights: Set[Privilege],
limits: UserLimits = UserLimits())
-object Identity extends MultipleReadersSingleWriterCache[Identity, DocInfo]
with DefaultJsonProtocol {
+object Identity extends MultipleReadersSingleWriterCache[Option[Identity],
DocInfo] with DefaultJsonProtocol {
private val viewName = "subjects/identities"
override val cacheEnabled = true
override val evictionPolicy = WriteTime
+ // upper bound for the auth cache to prevent memory pollution by sending
+ // malicious namespace patterns
+ override val fixedCacheSize = 100000
implicit val serdes = jsonFormat5(Identity.apply)
@@ -75,16 +80,16 @@ object Identity extends
MultipleReadersSingleWriterCache[Identity, DocInfo] with
list(datastore, List(ns), limit = 1) map { list =>
list.length match {
case 1 =>
- rowToIdentity(list.head, ns)
+ Some(rowToIdentity(list.head, ns))
case 0 =>
logger.info(this, s"$viewName[$namespace] does not exist")
- throw new NoDocumentException("namespace does not exist")
+ None
case _ =>
logger.error(this, s"$viewName[$namespace] is not unique")
throw new IllegalStateException("namespace is not unique")
}
}
- })
+ }).map(_.getOrElse(throw new NoDocumentException("namespace does not
exist")))
}
def get(datastore: AuthStore, authkey: BasicAuthenticationAuthKey)(
@@ -97,16 +102,16 @@ object Identity extends
MultipleReadersSingleWriterCache[Identity, DocInfo] with
list(datastore, List(authkey.uuid.asString, authkey.key.asString)) map
{ list =>
list.length match {
case 1 =>
- rowToIdentity(list.head, authkey.uuid.asString)
+ Some(rowToIdentity(list.head, authkey.uuid.asString))
case 0 =>
logger.info(this, s"$viewName[${authkey.uuid}] does not exist")
- throw new NoDocumentException("uuid does not exist")
+ None
case _ =>
logger.error(this, s"$viewName[${authkey.uuid}] is not unique")
throw new IllegalStateException("uuid is not unique")
}
}
- })
+ }).map(_.getOrElse(throw new NoDocumentException("namespace does not
exist")))
}
def list(datastore: AuthStore, key: List[Any], limit: Int = 2)(
diff --git
a/tests/src/test/scala/org/apache/openwhisk/core/database/test/behavior/ArtifactStoreSubjectQueryBehaviors.scala
b/tests/src/test/scala/org/apache/openwhisk/core/database/test/behavior/ArtifactStoreSubjectQueryBehaviors.scala
index d3b1763993..fda51a695d 100644
---
a/tests/src/test/scala/org/apache/openwhisk/core/database/test/behavior/ArtifactStoreSubjectQueryBehaviors.scala
+++
b/tests/src/test/scala/org/apache/openwhisk/core/database/test/behavior/ArtifactStoreSubjectQueryBehaviors.scala
@@ -82,6 +82,22 @@ trait ArtifactStoreSubjectQueryBehaviors extends
ArtifactStoreBehaviorBase {
Identity.get(authStore, ak1).failed.futureValue shouldBe
a[NoDocumentException]
}
+ it should "should throw NoDocumentException for non existing namespaces" in {
+ implicit val tid: TransactionId = transid()
+ val nonExistingNamesSpace = "nonExistingNamesSpace"
+ Identity.get(authStore,
EntityName(nonExistingNamesSpace)).failed.futureValue shouldBe
a[NoDocumentException]
+ }
+
+ it should "should throw NoDocumentException for non existing authKeys" in {
+ implicit val tid: TransactionId = transid()
+ val nonExistingUUID = "nonExistingUUID"
+ val nonExistingSecret = "nonExistingSecret"
+ Identity
+ .get(authStore, BasicAuthenticationAuthKey(UUID(nonExistingUUID),
Secret()))
+ .failed
+ .futureValue shouldBe a[NoDocumentException]
+ }
+
it should "find subject having multiple namespaces" in {
implicit val tid: TransactionId = transid()
val uuid1 = UUID()
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
