[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2015-02-23 Thread revans2
Github user revans2 commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-75576248
  
+1, sorry it took so long to get in.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2015-02-23 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/storm/pull/365


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2015-02-05 Thread dashengju
Github user dashengju commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-73170761
  
@HeartSaVioR , @ptgoetz 
I agree with committer sponsors only committers can join external's 
committer sponsors.  
I have changed the committer sponsors members in Readme, @ptgoetz , please 
help to review.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2015-02-03 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-72779122
  
@dashengju 
According to #416, we should remove ourselves from committer sponsors to 
let storm-redis module can be merged into master.
Thanks for clarification, @ptgoetz.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2015-01-31 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-72309792
  
I wish to hear additional opinions, and finally see it pulled in master. :)


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2015-01-24 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-71313044
  
I'll address follow-up issue, providing some persist / lookup bolts after 
merged to master.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2015-01-23 Thread revans2
Github user revans2 commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-71273044
  
+1 the changes still look good

Are there any other committers that want to take a look at the code?  I 
would feel more comfortable with more people looking at the code, just because 
it is so large.  If no one wants to take a look I guess I'll just pull it in 
sometime next week.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2015-01-21 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-70936224
  
@revans2 @dashengju 
Just I'm confused about 'Committer Sponsor'. Is it explained from docs? I 
can't find it.
If it doesn't exist, could you explain it?
Maybe it's better to have description about 'Committer Sponsor' to 
BYLAWS.md.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2015-01-21 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-70978681
  
@dashengju 
No, I mean that What is Committer Sponsor and where can I find 
description?.
I think if it is Committer Sponsor then we cannot be included, but if it 
is Committer Sponsor then we can be included.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2015-01-20 Thread revans2
Github user revans2 commented on a diff in the pull request:

https://github.com/apache/storm/pull/365#discussion_r23232733
  
--- Diff: 
external/storm-redis/src/main/java/org/apache/storm/redis/trident/state/RedisClusterMapState.java
 ---
@@ -0,0 +1,295 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * License); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an AS IS BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.storm.redis.trident.state;
+
+import backtype.storm.task.IMetricsContext;
+import backtype.storm.tuple.Values;
+import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import org.apache.storm.redis.util.config.JedisClusterConfig;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import redis.clients.jedis.JedisCluster;
+import storm.trident.state.JSONNonTransactionalSerializer;
+import storm.trident.state.JSONOpaqueSerializer;
+import storm.trident.state.JSONTransactionalSerializer;
+import storm.trident.state.OpaqueValue;
+import storm.trident.state.Serializer;
+import storm.trident.state.State;
+import storm.trident.state.StateFactory;
+import storm.trident.state.StateType;
+import storm.trident.state.TransactionalValue;
+import storm.trident.state.map.CachedMap;
+import storm.trident.state.map.IBackingMap;
+import storm.trident.state.map.MapState;
+import storm.trident.state.map.NonTransactionalMap;
+import storm.trident.state.map.OpaqueMap;
+import storm.trident.state.map.SnapshottableMap;
+import storm.trident.state.map.TransactionalMap;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.EnumMap;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+public class RedisClusterMapStateT implements IBackingMapT {
+private static final Logger logger = 
LoggerFactory.getLogger(RedisClusterMapState.class);
+
+private static final EnumMapStateType, Serializer 
DEFAULT_SERIALIZERS = Maps.newEnumMap(ImmutableMap.of(
+StateType.NON_TRANSACTIONAL, new 
JSONNonTransactionalSerializer(),
+StateType.TRANSACTIONAL, new JSONTransactionalSerializer(),
+StateType.OPAQUE, new JSONOpaqueSerializer()
+));
+
+public static class DefaultKeyFactory implements KeyFactory {
+public String build(ListObject key) {
+if (key.size() != 1)
+throw new RuntimeException(Default KeyFactory does not 
support compound keys);
+return (String) key.get(0);
+}
+};
+
+public static class OptionsT implements Serializable {
+public int localCacheSize = 1000;
+public String globalKey = $REDIS-MAP-STATE-GLOBAL;
+KeyFactory keyFactory = null;
+public SerializerT serializer = null;
+public String hkey = null;
+}
+
+public static interface KeyFactory extends Serializable {
+String build(ListObject key);
+}
+
+/**
+ * OpaqueTransactional for redis-cluster.
+ * */
+public static StateFactory opaque(JedisClusterConfig 
jedisClusterConfig) {
+return opaque(jedisClusterConfig, new Options());
+}
+
+public static StateFactory opaque(JedisClusterConfig 
jedisClusterConfig, String hkey) {
+Options opts = new Options();
+opts.hkey = hkey;
+return opaque(jedisClusterConfig, opts);
+}
+
+public static StateFactory opaque(JedisClusterConfig 
jedisClusterConfig, KeyFactory factory) {
+Options opts = new Options();
+opts.keyFactory = factory;
+return opaque(jedisClusterConfig, opts);
+}
+
+public static StateFactory opaque(JedisClusterConfig 
jedisClusterConfig, OptionsOpaqueValue opts) {
+return new Factory(jedisClusterConfig, StateType.OPAQUE, opts);
+}
+
+/**
+ * 

[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2015-01-20 Thread revans2
Github user revans2 commented on a diff in the pull request:

https://github.com/apache/storm/pull/365#discussion_r23232475
  
--- Diff: external/storm-redis/pom.xml ---
@@ -0,0 +1,108 @@
+?xml version=1.0 encoding=UTF-8?
+!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements.  See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the License); you may not use this file except in compliance with
+ the License.  You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an AS IS BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+--
+project xmlns=http://maven.apache.org/POM/4.0.0; 
xmlns:xsi=http://www.w3.org/2001/XMLSchema-instance; 
xsi:schemaLocation=http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd;
+modelVersion4.0.0/modelVersion
+
+parent
+artifactIdstorm/artifactId
+groupIdorg.apache.storm/groupId
+version0.10.0-SNAPSHOT/version
+relativePath../../pom.xml/relativePath
+/parent
+
+artifactIdstorm-redis/artifactId
+
+developers
+developer
+iddashengju/id
+nameDasheng Ju/name
+emaildashen...@gmail.com/email
+/developer
+developer
+idHeartSaVioR/id
+nameJungtaek Lim/name
+emailkabh...@gmail.com/email
+/developer
+/developers
+
+properties
+jedis.version2.6.2/jedis.version
+/properties
+
+dependencies
+dependency
+groupIdorg.apache.storm/groupId
+artifactIdstorm-core/artifactId
+version${project.version}/version
+scopeprovided/scope
+/dependency
+dependency
+groupIdredis.clients/groupId
+artifactIdjedis/artifactId
+version${jedis.version}/version
+/dependency
+dependency
+groupIdcom.google.guava/groupId
+artifactIdguava/artifactId
+version18.0/version
+/dependency
+/dependencies
+
+!--
+build
+sourceDirectorysrc/sourceDirectory
+resources
+resource
+directory${basedir}/multilang/directory
+/resource
+resource
+directory${basedir}/resources/directory
+/resource
+/resources
+
+plugins
+plugin
+groupIdorg.apache.maven.plugins/groupId
+artifactIdmaven-shade-plugin/artifactId
+version2.2/version
+configuration
+artifactSet
+excludes
+excludejunit:junit/exclude
+excludeorg.slf4j:slf4j-simple/exclude
+excludeorg.slf4j:slf4j-log4j12/exclude
+excludeorg.apache.zookeeper/exclude
+/excludes
+/artifactSet
+/configuration
+executions
+execution
+phasepackage/phase
+goals
+goalshade/goal
+/goals
+configuration
+
finalName${project.artifactId}-${project.version}-with-dependencies/finalName
+/configuration
+/execution
+/executions
+/plugin
+/plugins
+/build
+--
--- End diff --

I assume we can delete these commented parts of the pom?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2015-01-20 Thread HeartSaVioR
Github user HeartSaVioR commented on a diff in the pull request:

https://github.com/apache/storm/pull/365#discussion_r23264315
  
--- Diff: 
external/storm-redis/src/main/java/org/apache/storm/redis/trident/state/RedisClusterMapState.java
 ---
@@ -0,0 +1,295 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * License); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an AS IS BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.storm.redis.trident.state;
+
+import backtype.storm.task.IMetricsContext;
+import backtype.storm.tuple.Values;
+import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import org.apache.storm.redis.util.config.JedisClusterConfig;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import redis.clients.jedis.JedisCluster;
+import storm.trident.state.JSONNonTransactionalSerializer;
+import storm.trident.state.JSONOpaqueSerializer;
+import storm.trident.state.JSONTransactionalSerializer;
+import storm.trident.state.OpaqueValue;
+import storm.trident.state.Serializer;
+import storm.trident.state.State;
+import storm.trident.state.StateFactory;
+import storm.trident.state.StateType;
+import storm.trident.state.TransactionalValue;
+import storm.trident.state.map.CachedMap;
+import storm.trident.state.map.IBackingMap;
+import storm.trident.state.map.MapState;
+import storm.trident.state.map.NonTransactionalMap;
+import storm.trident.state.map.OpaqueMap;
+import storm.trident.state.map.SnapshottableMap;
+import storm.trident.state.map.TransactionalMap;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.EnumMap;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+public class RedisClusterMapStateT implements IBackingMapT {
+private static final Logger logger = 
LoggerFactory.getLogger(RedisClusterMapState.class);
+
+private static final EnumMapStateType, Serializer 
DEFAULT_SERIALIZERS = Maps.newEnumMap(ImmutableMap.of(
+StateType.NON_TRANSACTIONAL, new 
JSONNonTransactionalSerializer(),
+StateType.TRANSACTIONAL, new JSONTransactionalSerializer(),
+StateType.OPAQUE, new JSONOpaqueSerializer()
+));
+
+public static class DefaultKeyFactory implements KeyFactory {
+public String build(ListObject key) {
+if (key.size() != 1)
+throw new RuntimeException(Default KeyFactory does not 
support compound keys);
+return (String) key.get(0);
+}
+};
+
+public static class OptionsT implements Serializable {
+public int localCacheSize = 1000;
+public String globalKey = $REDIS-MAP-STATE-GLOBAL;
+KeyFactory keyFactory = null;
+public SerializerT serializer = null;
+public String hkey = null;
+}
+
+public static interface KeyFactory extends Serializable {
+String build(ListObject key);
+}
+
+/**
+ * OpaqueTransactional for redis-cluster.
+ * */
+public static StateFactory opaque(JedisClusterConfig 
jedisClusterConfig) {
+return opaque(jedisClusterConfig, new Options());
+}
+
+public static StateFactory opaque(JedisClusterConfig 
jedisClusterConfig, String hkey) {
+Options opts = new Options();
+opts.hkey = hkey;
+return opaque(jedisClusterConfig, opts);
+}
+
+public static StateFactory opaque(JedisClusterConfig 
jedisClusterConfig, KeyFactory factory) {
+Options opts = new Options();
+opts.keyFactory = factory;
+return opaque(jedisClusterConfig, opts);
+}
+
+public static StateFactory opaque(JedisClusterConfig 
jedisClusterConfig, OptionsOpaqueValue opts) {
+return new Factory(jedisClusterConfig, StateType.OPAQUE, opts);
+}
+
+/**
+ 

[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2015-01-20 Thread Parth-Brahmbhatt
Github user Parth-Brahmbhatt commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-70752983
  
@HeartSaVioR and @dashengju Thanks for the patch and I think overall the 
code looks good.

I am little confused about the core bolt. You have a trident state and 
query function but in the core implementation you just provide an abstract 
bolt. Any reason why the core functionality can not be extended to support the 
basic persist and lookup features out of the box? You already have a mapper 
interface for trident and instead of taking the tridentuple as input you could 
change the type to ITuple so it will be able to support both trident states and 
core bolt. It is ok to leave this implementation as is and file a jira to add 
the core persist and lookup bolts later or document why it is not a good idea 
to do so.

Also I think the README should be more detailed. you probably want to 
document that users trying to use core features needs to extend 
AbstractRedisBolt and how they should be using methods from the parent class. 
The trident section should cover the query functions and your probably want to 
remove ShardedRedis related classes from README as they don't exist.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2015-01-20 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-70755347
  
@Parth-Brahmbhatt 
Thanks for review!

First of all, let's start about abstract bolt.
https://github.com/apache/storm/pull/365#issuecomment-68332013 
You can see the matrix I've commented. 
Redis provides so many data types so we need to decide what data types we 
support or not.
I thought JedisCommands is easy and convenient enough to use so users would 
no problem to extend abstract bolt and make his/her own bolt.

But I'm also thinking it is really better for users to force making bolt, 
instead of providing some simple bolts. It could be already inconvenient to use 
in point of users' view.
@revans2 @dashengju How do you think about?

Btw, persist/lookup bolts for some datatypes can be added at any time.
Also, changing mapper to support normal bolt and trident seems great. 
We can check it now, and provide some bolts based on it if it works 
smoothly.
Thanks for your help!

Then let's talk about README.md.
Describing usage of AbstractBolt to README.md is already done, please see 
AbstractRedisBolt usage.
I agree we should remove description related to SharededJedis. I'll take 
care of it.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2015-01-20 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-70752747
  
@dashengju 
For your information, Jedis team is discussing about Jedis 3.0, and 
ShardedJedis could be replaced to JedisCluster for reducing code complexity.
https://groups.google.com/d/msg/jedis_redis/7ZJPFp_qTY0/ISHqEPZlAnwJ


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2015-01-20 Thread HeartSaVioR
Github user HeartSaVioR commented on a diff in the pull request:

https://github.com/apache/storm/pull/365#discussion_r23262904
  
--- Diff: external/storm-redis/pom.xml ---
@@ -0,0 +1,108 @@
+?xml version=1.0 encoding=UTF-8?
+!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements.  See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the License); you may not use this file except in compliance with
+ the License.  You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an AS IS BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+--
+project xmlns=http://maven.apache.org/POM/4.0.0; 
xmlns:xsi=http://www.w3.org/2001/XMLSchema-instance; 
xsi:schemaLocation=http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd;
+modelVersion4.0.0/modelVersion
+
+parent
+artifactIdstorm/artifactId
+groupIdorg.apache.storm/groupId
+version0.10.0-SNAPSHOT/version
+relativePath../../pom.xml/relativePath
+/parent
+
+artifactIdstorm-redis/artifactId
+
+developers
+developer
+iddashengju/id
+nameDasheng Ju/name
+emaildashen...@gmail.com/email
+/developer
+developer
+idHeartSaVioR/id
+nameJungtaek Lim/name
+emailkabh...@gmail.com/email
+/developer
+/developers
+
+properties
+jedis.version2.6.2/jedis.version
+/properties
+
+dependencies
+dependency
+groupIdorg.apache.storm/groupId
+artifactIdstorm-core/artifactId
+version${project.version}/version
+scopeprovided/scope
+/dependency
+dependency
+groupIdredis.clients/groupId
+artifactIdjedis/artifactId
+version${jedis.version}/version
+/dependency
+dependency
+groupIdcom.google.guava/groupId
+artifactIdguava/artifactId
+version18.0/version
+/dependency
+/dependencies
+
+!--
+build
+sourceDirectorysrc/sourceDirectory
+resources
+resource
+directory${basedir}/multilang/directory
+/resource
+resource
+directory${basedir}/resources/directory
+/resource
+/resources
+
+plugins
+plugin
+groupIdorg.apache.maven.plugins/groupId
+artifactIdmaven-shade-plugin/artifactId
+version2.2/version
+configuration
+artifactSet
+excludes
+excludejunit:junit/exclude
+excludeorg.slf4j:slf4j-simple/exclude
+excludeorg.slf4j:slf4j-log4j12/exclude
+excludeorg.apache.zookeeper/exclude
+/excludes
+/artifactSet
+/configuration
+executions
+execution
+phasepackage/phase
+goals
+goalshade/goal
+/goals
+configuration
+
finalName${project.artifactId}-${project.version}-with-dependencies/finalName
+/configuration
+/execution
+/executions
+/plugin
+/plugins
+/build
+--
--- End diff --

@revans2 So do I. I can take care of it and send PR to @dashengju 's repo.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2015-01-20 Thread dashengju
Github user dashengju commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-70777681
  
@revans2 ,  I have added you to the committer. thanks.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2015-01-19 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-70575248
  
@harshach @revans2 @ptgoetz 
I'd like to hear your thoughts about this.
Anything including code reviews, thought about Storm + Redis, and so on is 
very appreciated.
Thanks in advance!


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2015-01-05 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-68803852
  
@dashengju Great! Thanks for applying.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2015-01-04 Thread dashengju
Github user dashengju commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-68626207
  
@HeartSaVioR  and I have added abstract bolt  and trident state 
implementations, we also add some test cases, and modify the readme usage.
Anyone can help review?



---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2015-01-04 Thread HeartSaVioR
Github user HeartSaVioR commented on a diff in the pull request:

https://github.com/apache/storm/pull/365#discussion_r22443235
  
--- Diff: 
external/storm-redis/src/main/java/org/apache/storm/redis/trident/state/RedisClusterStateUpdater.java
 ---
@@ -0,0 +1,76 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * License); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an AS IS BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.storm.redis.trident.state;
+
+import org.apache.storm.redis.trident.mapper.TridentTupleMapper;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import redis.clients.jedis.JedisCluster;
+import storm.trident.operation.TridentCollector;
+import storm.trident.state.BaseStateUpdater;
+import storm.trident.tuple.TridentTuple;
+
+import java.util.List;
+
+public class RedisClusterStateUpdater extends 
BaseStateUpdaterRedisClusterState {
+private static final Logger logger = 
LoggerFactory.getLogger(RedisClusterState.class);
+
+private static final long DEFAULT_EXPIRE_INTERVAL_MS = 8640;
+
+private final String redisKeyPrefix;
+private final TridentTupleMapper tupleMapper;
+private final long expireIntervalMs;
+
+public RedisClusterStateUpdater(String redisKeyPrefix, 
TridentTupleMapper tupleMapper, long expireIntervalMs) {
+this.redisKeyPrefix = redisKeyPrefix;
+this.tupleMapper = tupleMapper;
+if (expireIntervalMs  0) {
+this.expireIntervalMs = expireIntervalMs;
+} else {
+this.expireIntervalMs = DEFAULT_EXPIRE_INTERVAL_MS;
+}
+}
+
+@Override
+public void updateState(RedisClusterState redisClusterState, 
ListTridentTuple inputs,
+TridentCollector collector) {
+long expireAt = System.currentTimeMillis() + expireIntervalMs;
+
+JedisCluster jedisCluster = null;
+try {
+jedisCluster = redisClusterState.getJedisCluster();
+for (TridentTuple input : inputs) {
+String key = 
this.tupleMapper.getKeyFromTridentTuple(input);
+String redisKey = key;
+if (redisKeyPrefix != null  redisKeyPrefix.length()  0) 
{
+redisKey = redisKeyPrefix + redisKey;
+}
+String value = 
this.tupleMapper.getValueFromTridentTuple(input);
+
+logger.debug(update key[ + key + ] redisKey[ + 
redisKey + ] value[ + value + ]);
+
+jedisCluster.set(redisKey, value);
--- End diff --

Just a note, we can use set command with ex parameter or setex command to 
reduce round-trip.
http://redis.io/commands/set


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2015-01-04 Thread HeartSaVioR
Github user HeartSaVioR commented on a diff in the pull request:

https://github.com/apache/storm/pull/365#discussion_r22443267
  
--- Diff: 
external/storm-redis/src/main/java/org/apache/storm/redis/trident/state/RedisMapState.java
 ---
@@ -0,0 +1,327 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * License); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an AS IS BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.storm.redis.trident.state;
+
+import backtype.storm.task.IMetricsContext;
+import backtype.storm.tuple.Values;
+import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
+import org.apache.storm.redis.util.config.JedisPoolConfig;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import redis.clients.jedis.Jedis;
+import redis.clients.jedis.JedisPool;
+import redis.clients.jedis.Pipeline;
+import storm.trident.state.JSONNonTransactionalSerializer;
+import storm.trident.state.JSONOpaqueSerializer;
+import storm.trident.state.JSONTransactionalSerializer;
+import storm.trident.state.OpaqueValue;
+import storm.trident.state.Serializer;
+import storm.trident.state.State;
+import storm.trident.state.StateFactory;
+import storm.trident.state.StateType;
+import storm.trident.state.TransactionalValue;
+import storm.trident.state.map.CachedMap;
+import storm.trident.state.map.IBackingMap;
+import storm.trident.state.map.MapState;
+import storm.trident.state.map.NonTransactionalMap;
+import storm.trident.state.map.OpaqueMap;
+import storm.trident.state.map.SnapshottableMap;
+import storm.trident.state.map.TransactionalMap;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.EnumMap;
+import java.util.List;
+import java.util.Map;
+
+public class RedisMapStateT implements IBackingMapT {
+private static final Logger logger = 
LoggerFactory.getLogger(RedisMapState.class);
+
+private static final EnumMapStateType, Serializer 
DEFAULT_SERIALIZERS = Maps.newEnumMap(ImmutableMap.of(
+StateType.NON_TRANSACTIONAL, new 
JSONNonTransactionalSerializer(),
+StateType.TRANSACTIONAL, new JSONTransactionalSerializer(),
+StateType.OPAQUE, new JSONOpaqueSerializer()
+));
+
+public static class DefaultKeyFactory implements KeyFactory {
+public String build(ListObject key) {
+if (key.size() != 1)
+throw new RuntimeException(Default KeyFactory does not 
support compound keys);
+return (String) key.get(0);
+}
+};
+
+public static class OptionsT implements Serializable {
+public int localCacheSize = 1000;
+public String globalKey = $REDIS-MAP-STATE-GLOBAL;
+KeyFactory keyFactory = null;
+public SerializerT serializer = null;
+public String hkey = null;
+}
+
+public static interface KeyFactory extends Serializable {
+String build(ListObject key);
+}
+
+/**
+ * OpaqueTransactional for redis.
+ * */
+public static StateFactory opaque(JedisPoolConfig jedisPoolConfig) {
+return opaque(jedisPoolConfig, new Options());
+}
+
+public static StateFactory opaque(JedisPoolConfig jedisPoolConfig, 
String hkey) {
+Options opts = new Options();
+opts.hkey = hkey;
+return opaque(jedisPoolConfig, opts);
+}
+
+public static StateFactory opaque(JedisPoolConfig jedisPoolConfig,  
KeyFactory factory) {
+Options opts = new Options();
+opts.keyFactory = factory;
+return opaque(jedisPoolConfig, opts);
+}
+
+public static StateFactory opaque(JedisPoolConfig jedisPoolConfig, 
OptionsOpaqueValue opts) {
+return new Factory(jedisPoolConfig, StateType.OPAQUE, opts);
+}
+
+/**
+ * Transactional for redis.
+ * */
+public static 

[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2015-01-04 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-68652310
  
@dashengju 
Though there're some points to reduce round-trip to Redis, looks good to 
me. Great job!
Seems like we're ready to review our effort to Storm community and finally 
merge into Storm Project.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2014-12-31 Thread dashengju
Github user dashengju commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-68431816
  
@harshach , we have added tests for both topology and trident.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2014-12-31 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-68431920
  
@dashengju 
JedisCommands is for single key. 
JedisCluster supports multi keys operation but all keys should be in one 
slot (so it should be handled differently from Jedis), and ShardedJedis doesn't 
support multi keys operation.
That's what I suggest we can use JedisCommands and unify all things into 
one.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2014-12-31 Thread dashengju
Github user dashengju commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-68434697
  
@HeartSaVioR  
If JedisCommands is designed for single key operation, It is useful for 
RedisState, because we provide the JedisCommands interface to user and they can 
unify all things into one.  It is not suitable for RedisMapState, because 
MapState do not expose JedisCommands interface to user. 



---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2014-12-31 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-68437760
  
@dashengju 
If we cannot tolerate single key operations restriction, there're some ways 
to settle.

1. check type and convert, from usage of Jedis 
We can check type (as you did) and convert JedisCommands to Jedis if 
available.

pros. It's simplest way
cons. Both handling Jedis and original (ex. Trident State) logic got mixed.

2. introduce CommandExecutor
We can introduce CommandExecutor, and passes Jedis/JedisCluster into it.
CommandExecutor implements and exposes available operations, such as get / 
set / mget / mset / hset / hget / etc.
(ex. If Jedis is passed, CommandExecutor.mget() calls actual mget(), but if 
JedisCluster is passed, CommandExecutor.mget() runs loop and calls get() for 
each key. )

pros. We can separate handling Jedis and original logic. 
cons. We have to implement additional component, which adds complexity. 

3. Drop support for ShardedJedis, JedisCluster
We can drop support for ShardedJedis and JedisCluster, and postpone this 
issue.

pros. There're no limit to use Jedis directly. We can use Pipeline if we 
need higher speed and requests could be batched.
cons. State repository is restricted to one instance of Redis.

Please add any alternative approaches or let me hear your opinions. :D

Surely anyone can participate this discussion.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2014-12-31 Thread dashengju
Github user dashengju commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-68447314
  
The users of our trident state, they may have a redis or have a redis 
cluster.  normally, they will not have both.
When they have a redis, they want to use the Jedis client for more 
function, such as mget/mset. they do not care about JedisCluster.
When they have a redis cluster, they can just use the JedisCluster client 
for single operation.

As redis bolt, because it handle one tuple one time, we provide 
JedisCommands is Ok.

As trident state, how about we support more implementations?
1) TridentState/TridentMapState,  which provide JedisCommands interface 
both for single redis or  redis cluster.
2) JedisTridentState/JedisTridentMapState, which provide Jedis interface 
just for single redis.
3) JedisClusterTridentState/JedisClusterTridentMapState, which provide 
JedisCluster interface, just for redis cluster. 
4) ShardedJedisTridentState/ShardedJedisTridentMapState, which provide 
ShardedJedis interface.

Users can select what they need. And storm-redis can go along with 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2014-12-31 Thread dashengju
Github user dashengju commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-68447798
  
may be support 1) 2) 4) is ok


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2014-12-31 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-68448629
  
@dashengju
Supporting 2, 3, 4 seems OK. When we need 2 and/or 3, and/or 4 separately, 
I don't think we need to maintain 1. 
1 is just for unifying.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2014-12-31 Thread dashengju
Github user dashengju commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-68449366
  
ok,I will add 2) 3) 4) those days.
@HeartSaVioR  Happy New Year :)


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2014-12-31 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-68450496
  
@dashengju Thanks! Happy new year!


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2014-12-30 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-68341375
  
@dashengju It would be great to move test codes to src/test/java.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2014-12-30 Thread dashengju
Github user dashengju commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-68346400
  
@HeartSaVioR  I have merged your PR, thanks. And I have added Apache 
licenses to all files.

I have found JedisCommands interface suitable for trident state, and I will 
change my code. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2014-12-30 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-68426972
  
@dashengju Thanks for applying unification on Jedis / JedisCluster into 
Trident implementation!


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2014-12-29 Thread dashengju
Github user dashengju commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-68244243
  
@darionyaphet , I think storm-redis can support redis and redis-cluster 
both.  we can also add redis-sharding solution to trident state. 

@HeartSaVioR , yes, I think storm-redis is first accepted, then we can work 
on the same place to add more modules to storm-redis.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2014-12-29 Thread harshach
Github user harshach commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-68267817
  
@dashengju thanks for sharing this. I would like to see unit tests on the 
code.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2014-12-29 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-68322020
  
@dashengju Hey, it's better to collaborate with you by requesting PR to 
merge.
I posted small PR to your fork just now.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2014-12-29 Thread dashengju
Github user dashengju commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-68324052
  
@harshach , @Parth-Brahmbhatt , @HeartSaVioR  
I have implemented a few functions in current PR.  
As @HeartSaVioR says, it's better to collaborate with each other  to make 
this PR to merge. You are welcome to make PR to my branch.
I will try to add support for core storm bolt, and add unit test for those 
code.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2014-12-29 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-68324419
  
@dashengju Oh, actually I'm sketching features for supporting core storm 
bolt.
Could you wait for me to share sketches and let me implement it, or do you 
want your own implementation?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2014-12-29 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-68324513
  
My sketch would be similar to storm-hbase, but because of various Redis 
data types, supporting types needs design before implementing.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2014-12-29 Thread dashengju
Github user dashengju commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-68325551
  
@HeartSaVioR , ok , you can responsible for core storm bolt.  
And I will add more data types for trident state and unit tests.

next time commit,  I will add you to the Committer Sponsors for storm-redis 
component.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2014-12-29 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-68325672
  
@dashengju Thanks for concerning!
I'll share a sketch about what it looks like (maybe similar to storm-hbase) 
and what types it can support.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2014-12-29 Thread darionyaphet
Github user darionyaphet commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-68327749
  
redis publish/subscribe is supported ?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2014-12-29 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-68330414
  
@darionyaphet 
Hello.
For spout, it isn't acceptable for reliable data source because during 
failure it'll lose some tuples.
(Actually Redis cannot act as reliable message queue, so I think it isn't 
good to implement spout for Redis.)
For bolt, every bolt should react based on tuple from spout or other bolt, 
so pub/sub seems not fit this case.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2014-12-29 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-68332013
  
Here're data types and commands I'm trying to support at first time.

Type | Read | Write
--- | --- | ---
STRING | GET (key) | SET (key, value), INCRBY (key, incr)
HASH | HGET (key, field) | HSET (key, field, value)
LIST | LPOP (key) | RPUSH (key, value)
SET |  | SADD (key, member)
SORTED SET | ZSCORE (key, member) | ZADD (key, score, member), ZINCRBY 
(key, incr, member)
HLL (HyperLogLog) | PFCOUNT (key) | PFADD (key, element)

There're so many candidate list, but we don't need to care all things 
because users may use Jedis easily.
There're also many commands which retrieves subset of values (by range, 
all), but it's good to start with essential and expand if we think it would be 
great to have.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2014-12-28 Thread HeartSaVioR
Github user HeartSaVioR commented on the pull request:

https://github.com/apache/storm/pull/365#issuecomment-68235170
  
Since Redis Cluster has some limitations, it seems better to implement it 
with Redis itself (and Jedis) unless state uses a big memory.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---