This is an automated email from the ASF dual-hosted git repository. chetanm pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/incubator-openwhisk.git
The following commit(s) were added to refs/heads/master by this push: new 536ce94 Cache empty auth results to reduce db load (#4104) 536ce94 is described below commit 536ce94af92a9b72ae1b82b16242d2c8177478b0 Author: Martin Henke <martin.he...@web.de> AuthorDate: Thu Nov 15 06:49:37 2018 +0100 Cache empty auth results to reduce db load (#4104) cache empty results to avoid performance hits by calling webactions repeatedly. Also configure a fixed size for identity cache to ensure it does not grow unbounded with too many negative entries. * limit size of auth cache * Simplify logic to create the cache Co-Authored-By: mhenke1 <martin.he...@web.de> --- .../MultipleReadersSingleWriterCache.scala | 49 ++++++++++------------ .../apache/openwhisk/core/entity/Identity.scala | 37 +++++++++------- .../ArtifactStoreSubjectQueryBehaviors.scala | 16 +++++++ 3 files changed, 58 insertions(+), 44 deletions(-) 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 1b4e46c..7de4f3b 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 7f3d0fd..f1833aa 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 d3b1763..fda51a6 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()