cbickel commented on a change in pull request #2338: Prepare cache to change it 
by configuration
URL: 
https://github.com/apache/incubator-openwhisk/pull/2338#discussion_r129035769
 
 

 ##########
 File path: 
common/scala/src/main/scala/whisk/core/database/MultipleReadersSingleWriterCache.scala
 ##########
 @@ -146,138 +142,127 @@ trait MultipleReadersSingleWriterCache[W, Winfo] {
         override def toString = s"tid ${transid.meta.id}, state ${state.get}"
     }
 
-    /**
-     * This method posts a delete to the backing store, and either directly 
invalidates the cache entry
-     * or informs any outstanding transaction that it must invalidate the 
cache on completion.
-     */
-    protected def cacheInvalidate[R](key: Any, invalidator: => Future[R])(
+    def cacheInvalidate[R](key: Any, invalidator: => Future[R])(
         implicit ec: ExecutionContext, transid: TransactionId, logger: 
Logging): Future[R] = {
-        if (cacheEnabled) {
-            logger.info(this, s"invalidating $key on delete")
-
-            // try inserting our desired entry...
-            val desiredEntry = Entry(transid, InvalidateInProgress, None)
-            cache(key)(desiredEntry) flatMap { actualEntry =>
-                // ... and see what we get back
-                val currentState = actualEntry.state.get
-
-                currentState match {
-                    case Cached =>
-                        // nobody owns the entry, forcefully grab ownership
-                        // note: if a new cache lookup is received while
-                        // the invalidator has not yet completed (and hence 
the actual entry
-                        // removed from the cache), such lookup operations 
will still be able
-                        // to return the value that is cached, and this is 
acceptable (under
-                        // the eventual consistency model) as long as such 
lookups do not
-                        // mutate the state of the cache to violate the 
invalidation that is
-                        // about to occur (this is eventually consistent and 
NOT sequentially
-                        // consistent since the cache lookup and the setting 
of the
-                        // InvalidateInProgress bit are not atomic
+        logger.info(this, s"invalidating $key on delete")
+
+        // try inserting our desired entry...
+        val desiredEntry = Entry(transid, InvalidateInProgress, None)
+        cache(key)(desiredEntry) flatMap { actualEntry =>
+            // ... and see what we get back
+            val currentState = actualEntry.state.get
+
+            currentState match {
+                case Cached =>
+                    // nobody owns the entry, forcefully grab ownership
+                    // note: if a new cache lookup is received while
+                    // the invalidator has not yet completed (and hence the 
actual entry
+                    // removed from the cache), such lookup operations will 
still be able
+                    // to return the value that is cached, and this is 
acceptable (under
+                    // the eventual consistency model) as long as such lookups 
do not
+                    // mutate the state of the cache to violate the 
invalidation that is
+                    // about to occur (this is eventually consistent and NOT 
sequentially
+                    // consistent since the cache lookup and the setting of the
+                    // InvalidateInProgress bit are not atomic
+                    invalidateEntryAfter(invalidator, key, actualEntry)
+
+                case ReadInProgress | WriteInProgress =>
+                    if (actualEntry.trySet(currentState, InvalidateWhenDone)) {
+                        // then the pre-existing owner will take care of the 
invalidation
+                        invalidator
+                    } else {
+                        // the pre-existing reader or writer finished and so 
must
+                        // explicitly invalidate here
                         invalidateEntryAfter(invalidator, key, actualEntry)
+                    }
 
-                    case ReadInProgress | WriteInProgress =>
-                        if (actualEntry.trySet(currentState, 
InvalidateWhenDone)) {
-                            // then the pre-existing owner will take care of 
the invalidation
-                            invalidator
-                        } else {
-                            // the pre-existing reader or writer finished and 
so must
-                            // explicitly invalidate here
-                            invalidateEntryAfter(invalidator, key, actualEntry)
-                        }
-
-                    case InvalidateInProgress =>
-                        if (actualEntry == desiredEntry) {
-                            // we own the entry, so we are responsible for 
cleaning it up
-                            invalidateEntryAfter(invalidator, key, actualEntry)
-                        } else {
-                            // someone else requested an invalidation already
-                            invalidator
-                        }
-
-                    case InvalidateWhenDone =>
-                        // a pre-existing owner will take care of the 
invalidation
+                case InvalidateInProgress =>
+                    if (actualEntry == desiredEntry) {
+                        // we own the entry, so we are responsible for 
cleaning it up
+                        invalidateEntryAfter(invalidator, key, actualEntry)
+                    } else {
+                        // someone else requested an invalidation already
                         invalidator
-                }
+                    }
+
+                case InvalidateWhenDone =>
+                    // a pre-existing owner will take care of the invalidation
+                    invalidator
             }
-        } else invalidator // not caching
+        }
     }
 
-    /**
-     * This method may initiate a read from the backing store, and potentially 
stores the result in the cache.
-     */
-    protected def cacheLookup[Wsuper >: W](key: Any, generator: => Future[W], 
fromCache: Boolean = cacheEnabled)(
-        implicit ec: ExecutionContext, transid: TransactionId, logger: 
Logging): Future[W] = {
-        if (fromCache) {
-            val promise = Promise[W] // this promise completes with the 
generator value
-
-            // try inserting our desired entry...
-            val desiredEntry = Entry(transid, ReadInProgress, 
Some(promise.future))
-            cache(key)(desiredEntry) flatMap { actualEntry =>
-                // ... and see what we get back
-
-                actualEntry.state.get match {
-                    case Cached =>
-                        logger.debug(this, "cached read")
-                        makeNoteOfCacheHit(key)
-                        actualEntry.unpack
-
-                    case ReadInProgress =>
-                        if (actualEntry == desiredEntry) {
-                            logger.debug(this, "read initiated");
-                            makeNoteOfCacheMiss(key)
-                            // updating the cache with the new value is done 
in the listener
-                            // and will complete unless an invalidation 
request or an intervening
-                            // write occur in the meantime
-                            listenForReadDone(key, actualEntry, generator, 
promise)
-                            actualEntry.unpack
-                        } else {
-                            logger.debug(this, "coalesced read")
-                            makeNoteOfCacheHit(key)
-                            actualEntry.unpack
+    def cacheLookup[Wsub <: W](key: Any, generator: => Future[Wsub])(
+        implicit ec: ExecutionContext, transid: TransactionId, logger: 
Logging, mw: Manifest[Wsub]): Future[Wsub] = {
+        val promise = Promise[W] // this promise completes with the generator 
value
+
+        // try inserting our desired entry...
+        val desiredEntry = Entry(transid, ReadInProgress, Some(promise.future))
+        cache(key)(desiredEntry) flatMap { actualEntry =>
+            // ... and see what we get back
+
+            actualEntry.state.get match {
+                case Cached =>
+                    logger.debug(this, "cached read")
+                    makeNoteOfCacheHit(key)
+                    actualEntry.unpack.map { entry =>
+                        if (entry.getClass != mw.runtimeClass) {
+                            throw DocumentTypeMismatchException(s"document 
type ${entry.getClass} did not match expected type ${mw.runtimeClass}.")
                         }
+                        entry.asInstanceOf[Wsub]
 
 Review comment:
   As the Cache is a cache for all WhiskEntities, there are Rules, Triggers and 
Actions in one Cache.
   On getting an element by id, we should be sure, that the element the cache 
found has the expected type.
 
----------------------------------------------------------------
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

Reply via email to