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

Bumped rediscala to 1.8.2 and Akka 2.5.x #151

Merged
merged 1 commit into from
Mar 22, 2018
Merged

Bumped rediscala to 1.8.2 and Akka 2.5.x #151

merged 1 commit into from
Mar 22, 2018

Conversation

KarelCemus
Copy link
Owner

@KarelCemus KarelCemus commented Feb 27, 2018

Fixes #150

@pandian-egnaroinc I updated the rediscala to version 1.8.2, which uses Akka 2.5.6. However, there are new maintainers so I'm not sure and convinced about its stability. Could you test this snapshot before I merge it to master, please?

I ran the tests within play-redis (517) and all passed, so I hope it should work.

@KarelCemus KarelCemus added this to the 2.0.3 milestone Feb 27, 2018
@pandian-egnaroinc
Copy link

pandian-egnaroinc commented Feb 27, 2018 via email

@pandian-egnaroinc
Copy link

pandian-egnaroinc commented Feb 28, 2018 via email

@KarelCemus
Copy link
Owner Author

KarelCemus commented Feb 28, 2018

Unfortunately, I cannot attach a file here ...

This snapshot is not published anywere.

Usually, people clone the repository a build it from source themselves. Is it a problem?

git clone git@github.com:KarelCemus/play-redis.git --depth=1

cd play-redis

git checkout akka-2.5.x

sbt +package

and then inside the target/scala-${version}/ is the JAR.

Alternatively, you could instead of package do publishLocal and you'll get it installed in your private ivy repository.

@pandian-egnaroinc
Copy link

pandian-egnaroinc commented Feb 28, 2018 via email

@KarelCemus
Copy link
Owner Author

@pandian-egnaroinc Hi, any progress on this PR?

@pandian-egnaroinc
Copy link

pandian-egnaroinc commented Mar 5, 2018 via email

@pandian-egnaroinc
Copy link

pandian-egnaroinc commented Mar 5, 2018 via email

@KarelCemus
Copy link
Owner Author

KarelCemus commented Mar 5, 2018

I cloned and checked out 2.5.x.

You mean branch akka-2.5.x, I hope

  1. Copied the jar to my libs directory.

Sorry, I forgot to suggest using sbt publishLocal to have it automatically published in your local ivy repository. Do it instead of copying, this works better and is reliable.

  1. Compilation mandated for new override function invocationPolicy() for RedisCluster and RedisStandalone derivatives. Defined as below.
    @OverRide
    public String invocationPolicy() { return "lazy"; }

You can use new RedisCluster with RedisDelegatingSettings when overriding the settings to avoid unnecessary implementation of defaults, but your solution also works.

After copying “rediscala_2.12-1.8.2.jar”

Yes, version 2.1.0 uses new version of the connector. That's right.

Otherwise the error is odd. The version 1.8.2 uses akka 2.5.6 so it should be fine. However, it seems you are manually maintaining your dependencies, so there might be the issue. Anyway, are you able to give me some minimal configuration (build.sbt) to reproduce the issue?

@pandian-egnaroinc
Copy link

pandian-egnaroinc commented Mar 5, 2018 via email

@KarelCemus
Copy link
Owner Author

You mean like my hello world but in the cluster mode? I'll try to reproduce ....

@pandian-egnaroinc
Copy link

pandian-egnaroinc commented Mar 5, 2018 via email

@pandian-egnaroinc
Copy link

pandian-egnaroinc commented Mar 5, 2018 via email

@KarelCemus
Copy link
Owner Author

KarelCemus commented Mar 6, 2018

I ran your code and there are several things I noticed:

  • In Module you expose your bindings to redis and ITS, don't do it. Play-redis registers them for you.
  • you have invalid configuration of your cluster, valid cluster has at least 3 masters. I changed your configuration to the configuration of the valid cluster and it works like a charm.

Note that the minimal cluster that works as expected requires to contain at least three master nodes. For your first tests it is strongly suggested to start a six nodes cluster with three masters and three slaves.

For more details look here

For example, I used:

cluster: [        
        { host: 127.0.0.1, port: 7003 }
        { host: 127.0.0.1, port: 7002 }
        { host: 127.0.0.1, port: 7001 }
        { host: 127.0.0.1, port: 7000 }
]

Invalid cluster configuration caused your java.lang.RuntimeException: server not found: no server available exception.

However, although it works, there is still sometimes thrown that NPE at a.a.OneForOneStrategy - null, which is weird. I'll investigate.

@pandian-egnaroinc
Copy link

pandian-egnaroinc commented Mar 6, 2018 via email

@KarelCemus
Copy link
Owner Author

There are 2 issues. First is the cluster configuration and the second is that exception.

Play redis does not implement the protocol, it's implemented by rediscala so for detailed documentation, configuration and settings regarding the connection look there. Also both these issues are issues of the underlying connector, not the play-redis itself.

I actually have a 5 node redis cluster running

I don't know how rediscala validates the cluster configuration, so provide them all and you'll see.

But, Your example mentions only one server for cluster configuration.

That's right, that was only for illustration but I'll fix it.

Redis is able to load balance internally if requests come to a single sentinel and lot of clients are written this way. I thought that is the case with play-redis too.

Sentinel is not exactly the same thing as a cluster. At least not in the terms of rediscala, that is a different mode and play-redis does not support it right now.

Also, I think that the root of the issue could be something else.

Feel free to dig into it, I'm a bit short on time.

@pandian-egnaroinc
Copy link

pandian-egnaroinc commented Mar 6, 2018 via email

@KarelCemus
Copy link
Owner Author

I investigated the issue and it was just as I assumed - two independent issues. The NPE happened due to a possible race condition and a forward reference. See etaty/rediscala#160 and the related PR.

@KarelCemus KarelCemus force-pushed the akka-2.5.x branch 2 times, most recently from bb6392b to 019bee5 Compare March 22, 2018 21:38
- bumped to Scala 2.12.5
- bumped play to 2.6.12
@KarelCemus KarelCemus merged commit 6aa76bd into master Mar 22, 2018
@KarelCemus KarelCemus deleted the akka-2.5.x branch March 22, 2018 21:52
@KarelCemus
Copy link
Owner Author

Migrated to Akka 2.5.x and resolved the NPE in the cluster mode.

@KarelCemus KarelCemus modified the milestones: 2.0.3, 2.1.0 Apr 13, 2018
@KarelCemus KarelCemus mentioned this pull request Aug 21, 2019
2 tasks
KarelCemus added a commit that referenced this pull request Dec 2, 2020
Bumped rediscala to 1.8.2 and Akka 2.5.x
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.

None yet

2 participants