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

NotSerializableException in AsyncCacheAPI#getOrElseUpdate #140

Closed
tzimisce012 opened this issue Jan 11, 2018 · 10 comments
Closed

NotSerializableException in AsyncCacheAPI#getOrElseUpdate #140

tzimisce012 opened this issue Jan 11, 2018 · 10 comments
Labels

Comments

@tzimisce012
Copy link

Are you filing an issue? Please follow these checklists and provide us all the possible information to speed up the process.

Reporting a bug?

  1. Version of play-redis: 2.0.1
  2. Version of Scala: 2.12.4
  3. Version of Play: 2.6.6
  4. Describe the environment if applies (usually in cloud etc.) Local

So I was working on play-redis 1.6.1 but as soon as I migrated to 2.0.1 I faced two different problems using the method getOrElseUpdate.

When I use this method for the first time, they key is not found on redis, so the method does the update and store in redis before returning the value. The second time I ask for this key, I am getting the following error:

java.io.NotSerializableException: No configured serialization-bindings for class [java.lang.Object]
        at akka.serialization.Serialization.serializerFor(Serialization.scala:238)
        at akka.serialization.Serialization.$anonfun$deserialize$3(Serialization.scala:204)
        at scala.util.Try$.apply(Try.scala:209)
        at akka.serialization.Serialization.deserialize(Serialization.scala:204)
        at play.api.cache.redis.connector.AkkaDecoder.binaryToAnyRef(AkkaSerializer.scala:122)
        at play.api.cache.redis.connector.AkkaDecoder.$anonfun$stringToAnyRef$2(AkkaSerializer.scala:126)
        at scala.Function1.$anonfun$andThen$1(Function1.scala:52)
        at play.api.cache.redis.connector.AkkaDecoder.stringToAnyRef(AkkaSerializer.scala:126)
        at play.api.cache.redis.connector.AkkaDecoder.untypedDecode(AkkaSerializer.scala:113)
        at play.api.cache.redis.connector.AkkaDecoder.decode(AkkaSerializer.scala:97)

The second issue that I have is a problem that was solved on 1.6.1. Again, the java context is lost in getOrElseUpdate with the last version.

@KarelCemus
Copy link
Owner

Hi, I'm sorry to hear that. I'll look into it at weekend and investigate what's going on.

Note: When there are 2 issues, please, open 2 tickets. These seem to be unrelated to each other. The second one is probably just a regression issue.

@KarelCemus KarelCemus added the bug label Jan 11, 2018
@KarelCemus KarelCemus changed the title Migration to 2.0.1 NotSerializableException in getOrElseUpdate Jan 11, 2018
@KarelCemus
Copy link
Owner

KarelCemus commented Jan 14, 2018

I investigated your report and I couldn't reproduce either of the issues.

First, you reported the loosing of the context. I checked the code base and there is still the patch as well as the test preventing the regression issue. Please, check them, if you see any issue in any of them, report it.

Second, I tried the very simple scenario of the getOrElseUpdate method but it seems to work. I also checked the code and there is no obvious reason for the behavior you describe. I tried:

  def index = Action {
    val value = cache.getOrElseUpdate( "ABC", ( ) => {
      println( "getting a string" )
      "message"
    }, 10 )
    Ok( value )
  }

I tried even a custom object instead of the string and it still works.

So, could you provide an example to reproduce the issue? Otherwise, I am unfortunately unable to do more.

And finally just a side note. As you probably know, the error indicates that you are trying to serialize plain java.lang.Object, which is very probably not supported by your serializer. Personally, I'd guess that you are storing some other object and not the one you want.

@tzimisce012
Copy link
Author

Ok, I have created an example on GitHub so you can see the issues that I have.

https://github.com/tzimisce012/redis-test

With this example you will see the error with the deserialization. Be fast with the retries on the request because I put 5 seconds of expiration time.

@tzimisce012
Copy link
Author

Also I have added an example where the context is lost.

@KarelCemus
Copy link
Owner

Great example, I reproduced the serialization issue. Unfortunately, I don't know how to fix it right now. The issue lies in the Java erasure of runtime types and occurs only with Java version of AsyncCacheApi. More specifically, the issue is directly in Play, see the code. Here, Scala.classTag returns Object and I believe that's due to runtime type erasure.

Explanation

When you miss on cache, you compute the actual value and then serialize it, which works as it asks the value itself for its type.

However, when you hit, it considers the ClassTag given by Scala.classTag, which is wrong.

In the previous version of Play, i.e., Play 2.5 and earlier, Play's API didn't support type parameter T and I used a workaround of caching besides the value also its type to be able to deserialize it later. However, since Play 2.6, I use this given type T, which works pretty well with the synchronous API, as the main type is not erased. But oviously, it doesn't work with async API as CompletedStage<T> gets erased to CompletedStage<Object>.

So, it seems the issue is not in play-redis but I have to investigate it more to see how to deal with it.

@KarelCemus
Copy link
Owner

KarelCemus commented Jan 17, 2018

I also reproduced the second issue and it partially relates to the first one. The support of the context is implemented only for Java handler, which is JavaRedis. However, AsyncCacheApi bound to the DefaultAsyncApi in version 2.0.1 uses Scala implementation under the hood, which doesn't support it. On the other hand, DefaultSyncApi uses internally JavaRedis, though with sync version it works.

It explains why I couldn't reproduce the issues. Both these issues partially relates to the Scala implementation. First, there is given wrong classTag, second, it doesn't support Java context.

Surprisingly, the solution is pretty simple. Sync api uses JavaRedis from play-redis library, which also implements play.cache.AsyncCacheApi. I don't know, why the API wasn't bound directly to this implementation and uses the adapter instead. Anyway, JavaRedis workarounds the classTag issue by storing also the type as I explained above and supports the context.

Solution

So to sum it up, one issue is cause by Java type erasure in Play, the second one is the Scala implementation under the Java API.

I change the implementation of AsyncCacheAPI from the wrapper around Scala version to JavaRedis, which already handles both issues.

I'll release the fix as 2.0.2 and will be added also in 2.1.0.

Thanks for your contribution and the example to reproduce it.

@tzimisce012
Copy link
Author

Thanks for your fast solution

@KarelCemus KarelCemus changed the title NotSerializableException in getOrElseUpdate NotSerializableException in AsyncCacheAPI#getOrElseUpdate Jan 17, 2018
@KarelCemus
Copy link
Owner

Released 2.0.2

@tzimisce012
Copy link
Author

is it published already?

@KarelCemus
Copy link
Owner

It takes usually several hours to sync

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants