twitter / storehaus

Storehaus is a library that makes it easy to work with asynchronous key value stores
Apache License 2.0
464 stars 86 forks source link

ReadableStore.find calls the next store even if the current store finds a valid value #287

Closed pankajroark closed 8 years ago

pankajroark commented 8 years ago

We ran into this, the primary store was a remote cache and second store was db. Even when the value was found in the remote cache the db was still being called. On investigation I found this being an issue in ReadableStore.find and ultimately in FutureOps.find.

The root cause is that pattern match on a stream evaluates both head and tail. So in: def find[T](futures: Stream[Future[T]])(pred: T => Boolean): Future[T] = futures match { case Stream.Empty => Future.exception(new RuntimeException("Empty iterator in FutureOps.find")) case last #:: Stream.Empty => last case next #:: rest => next.filter(pred).rescue { case _: Throwable => find(rest)(pred) } }

when the second case is hit i.e. " case last #:: Stream.Empty => last" it evaluates the tail, resulting in a call to the second store. Even the third patten match would have had the same issue but the issue happens even before. Since scala stream evaluates an element only once it doesn't result in an additional call on the third pattern match, otherwise there would have been two extra calls.

I'll send a pull request shortly with a test and the fix.

pankajroark commented 8 years ago

Pull request for the fix: https://github.com/twitter/storehaus/pull/288