Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix FutureAsyncCache #2162

Merged
merged 7 commits into from
Jun 11, 2021
Merged

Fix FutureAsyncCache #2162

merged 7 commits into from
Jun 11, 2021

Conversation

kitlangton
Copy link
Member

@kitlangton kitlangton commented Jun 9, 2021

The ZIO Async Cache never actually cached anything. So we removed it for now, potentially eventually replacing it with zio-cache. For now we simply replaced it with the AsyncFutureCache, fixing some correctness issues there as well. Previously, it was unsafely casting the ExecutionContext to an Executor, which happened to work with what was being passed in before, but broke when the ExecutionContext was ZIO's internal Executor.

deusaquilus
deusaquilus approved these changes Jun 9, 2021
@zio zio deleted a comment from kitlangton Jun 9, 2021
@zio zio deleted a comment from kitlangton Jun 9, 2021
@deusaquilus
Copy link
Collaborator

Note that the andThen part in this prepareAsync (after the asScala) was always wrong:

trait AsyncFutureCache { this: CassandraSession =>
  lazy val asyncCache = new PrepareStatementCache[Future[PreparedStatement]](preparedStatementCacheSize)
  def prepareAsync(cql: String)(implicit executionContext: ExecutionContext): Future[BoundStatement] =
    asyncCache(cql)(stmt => session.prepareAsync(stmt).asScala andThen {
      case Failure(_) => asyncCache.invalidate(stmt)
    }).map(_.bind())
}

The problem here is that the result of session.prepareAsync(stmt).asScala is not a promise, it's a future so the partial function it's being fed into here always fails (i.e. since Future[PreparedStatement] is never a scala.util.Failure.
That means that this cache has not been working in the first place.

@deusaquilus deusaquilus merged commit aa2a2fc into master Jun 11, 2021
@deusaquilus deusaquilus deleted the fix-async-future-cache branch June 11, 2021 05:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants