You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by darionyaphet <gi...@git.apache.org> on 2016/06/02 15:02:09 UTC
[GitHub] storm pull request #1459: [STORM-1875]Separate Jedis/JedisCluster Config
GitHub user darionyaphet opened a pull request:
https://github.com/apache/storm/pull/1459
[STORM-1875]Separate Jedis/JedisCluster Config
[STORM-1875](https://issues.apache.org/jira/browse/STORM-1875)
Separate Jedis / JedisCluster to provide full operations for each environment to users .
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/darionyaphet/storm STORM-1875
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/1459.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1459
----
commit 493c28b0b5a9c416632b0048e18a2826535f5c38
Author: darionyaphet <da...@gmail.com>
Date: 2016-06-02T15:00:29Z
separate Jedis/JedisCluster config
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #1459: [STORM-1875]Separate Jedis/JedisCluster Config
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/1459
No for predefined operations (StoreBolt, LookupBolt) but we can still support full features for AbstractRedisBolt and let users extend this class.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #1459: [STORM-1875]Separate Jedis/JedisCluster Config
Posted by darionyaphet <gi...@git.apache.org>.
Github user darionyaphet commented on the issue:
https://github.com/apache/storm/pull/1459
`Storm-Redis` have support multiple keys operation ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #1459: [STORM-1875]Separate Jedis/JedisCluster Config
Posted by darionyaphet <gi...@git.apache.org>.
Github user darionyaphet commented on the issue:
https://github.com/apache/storm/pull/1459
OK , I will try to separating classes :D
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #1459: [STORM-1875]Separate Jedis/JedisCluster Config
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/1459
The problem is actually not simple. The blocking point is JedisCommands interface, not config.
JedisCommands exposes operations which use only single key. Since some multiple keys operations of Redis is not suitable for Redis Cluster (operation is available but not making sense), Jedis and JedisCluster takes different interfaces, which blocks us to provide unified interface from storm-redis side.
One simple way is separating existing classes to RedisLookupBolt / RedisClusterLookupBolt, RedisStoreBolt / RedisClusterStoreBol, as State/MapState did, but it incurs backward incompatibility if users use RedisLookupBolt / RedisStoreBolt to access Redis Cluster.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #1459: [STORM-1875]Separate Jedis/JedisCluster Config
Posted by darionyaphet <gi...@git.apache.org>.
Github user darionyaphet commented on the issue:
https://github.com/apache/storm/pull/1459
@darionyaphet
This is Jedis class diagram when Jedis was 2.5.0. At that time JedisCluster doesn't support multiple keys operation so now it becomes more complicated.
https://lh5.googleusercontent.com/-kMPsNHe6t2I/U0Dn01HxIoI/AAAAAAAAt4U/U8I0j570rI4/s3200/Jedis+Class+Diagram+without+fields_methods.jpg
DISCLAIMER: I'm also a committer of Jedis. :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #1459: [STORM-1875]Separate Jedis/JedisCluster Config
Posted by darionyaphet <gi...@git.apache.org>.
Github user darionyaphet commented on the issue:
https://github.com/apache/storm/pull/1459
Both `Jedis` and `JedisCluster ` have implemented `JedisCommands` interface . So I think we can use `JedisCommands` calling the same method on single redis instance and redis cluster .
Just use `JedisConfig` to control instance `JedisContainer` or `JedisCommandsInstanceContainer` maybe a good idea and support two difference interface is useless :P
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #1459: [STORM-1875]Separate Jedis/JedisCluster Config
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/1459
One other way without breaking compatibility but not beauty is adding some utility methods to AbstractRedisBolt like,
```
public JedisCluster asJedisCluster(JedisCommands instance) throws IllegalArgumentException
public JedisCluster asJedis(JedisCommands instance) throws IllegalArgumentException
```
and let users convert their JedisCommands instance to actual type before using full features.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #1459: [STORM-1875]Separate Jedis/JedisCluster Config
Posted by darionyaphet <gi...@git.apache.org>.
Github user darionyaphet commented on the issue:
https://github.com/apache/storm/pull/1459
@HeartSaVioR Good ! I will try to split into two class . BTW Do you have any idea to unified `Jedis` and `JedisCluster` interface ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #1459: [STORM-1875]Separate Jedis/JedisCluster Config
Posted by darionyaphet <gi...@git.apache.org>.
Github user darionyaphet closed the pull request at:
https://github.com/apache/storm/pull/1459
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---