debasishg / scala-redis

A scala library for connecting to a redis server, or a cluster of redis nodes using consistent hashing on the client side.
1.02k stars 219 forks source link

using apache pool to v2(latest) #211

Closed masahitojp closed 5 years ago

masahitojp commented 6 years ago

related: https://github.com/debasishg/scala-redis/issues/205

masahitojp commented 6 years ago

Hi there I'd like to update apache pool to 2.x latest! Do you feel about it?

and I have a concern for UnitTest. If you run the unit test locally, even if you run it on the master branch you will see some failures.

I use the following docker image. https://github.com/docker-library/redis/blob/99a06c057297421f9ea46934c342a2fc00644c4f/3.2/Dockerfile

debasishg commented 6 years ago

Thanks for the PR. Let me do some testing over the weekend ..

masahitojp commented 6 years ago

Hi @debasishg I share additional information about unit testing. On my local, the following tests failed. (Other tests are passed.)

PatternsSpec.scala PoolSpec.scala

Here is the log at the time of the test failure. Apparently it seems timed out. (This has the same result with the master branch)

[info] PoolSpec: [info] pool test [info] - should distribute work amongst the clients [info] list load test 1 [info] - should distribute work amongst the clients for 400000 list operations FAILED [info] java.util.concurrent.TimeoutException: Futures timed out after [20 seconds] [info] at scala.concurrent.impl.Promise$DefaultPromise.ready(Promise.scala:223) [info] at scala.concurrent.impl.Promise$DefaultPromise.result(Promise.scala:227) [info] at scala.concurrent.Await$$anonfun$result$1.apply(package.scala:190) [info] at scala.concurrent.BlockContext$DefaultBlockContext$.blockOn(BlockContext.scala:53) [info] at scala.concurrent.Await$.result(package.scala:190) [info] at com.redis.Bench$.load(Bench.scala:39) [info] at com.redis.Bench$.listLoad(Bench.scala:33) [info] at com.redis.PoolSpec$$anonfun$2$$anonfun$apply$mcV$sp$2.apply(PoolSpec.scala:90) [info] at com.redis.PoolSpec$$anonfun$2$$anonfun$apply$mcV$sp$2.apply(PoolSpec.scala:89) [info] at org.scalatest.OutcomeOf$class.outcomeOf(OutcomeOf.scala:85)

masahitojp commented 6 years ago

@debasishg How is your testing going? :smile:

debasishg commented 6 years ago

@masahitojp I am also getting some test failures and future timeouts. May be I will try again with an increased timeout value. But that may not be an ideal solution. Will look at it in details shortly. In the mean time if u have any additional feedback, feel free to let me know.

masahitojp commented 6 years ago

@debasishg sorry to late reply.

May be I will try again with an increased timeout value. But that may not be an ideal solution.

I'll check it soon. Thanks 😄

masahitojp commented 6 years ago

@debasishg I share results

On my local, i succeeded to run and check all UnitTests! When I changed increased timeout value , following UnitTests run and succeeded!

PatternsSpec.scala PoolSpec.scala

and I updated Apache Commons Pool to 2.6.0(latest) https://commons.apache.org/proper/commons-pool/

My change for this PR is now complete. Let me know if you have any questions. Thanks!

debasishg commented 6 years ago

Apologies for being late on this .. too much occupied with regular stuff. Will get back soon ..

masahitojp commented 5 years ago

I fixed confilct.

masahitojp commented 5 years ago

@debasishg Is there something I can do to help you?

debasishg commented 5 years ago

Thanks a lot for the contribution! Apologies it took me a while to get back. Totally swamped with work and conferences ..