[jira] [Commented] (STORM-1934) Race condition between sync-supervisor and sync-processes raises several strange issues

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1934?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15356511#comment-15356511
 ] 

ASF GitHub Bot commented on STORM-1934:
---

Github user satishd commented on the issue:

https://github.com/apache/storm/pull/1528
  
@HeartSaVioR You may want to use `(declare rm-topo-files)` in the start so 
that you do not need to move `defn sync-processes` for handling forward 
references. This may make it easy to review the code. 


> Race condition between sync-supervisor and sync-processes raises several 
> strange issues
> ---
>
> Key: STORM-1934
> URL: https://issues.apache.org/jira/browse/STORM-1934
> Project: Apache Storm
>  Issue Type: Bug
>  Components: storm-core
>Affects Versions: 1.0.0, 2.0.0, 1.0.1
>Reporter: Jungtaek Lim
>Assignee: Jungtaek Lim
>Priority: Critical
>
> There're some strange issues including STORM-1933 and others (which I will 
> file an issue soon) which are related to race condition in supervisor.
> As I mentioned to STORM-1933, basically sync-supervisor relies on zk 
> assignment, and sync-processes relies on local assignment and local workers 
> directory, but in fact sync-supervisor also access local state and take some 
> actions which affects sync-processes. And also Satish left the comment to 
> STORM-1933 describing other issue related to race condition and idea to fix 
> this which is same page on me.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[GitHub] storm issue #1528: STORM-1934 Fix race condition between sync-supervisor and...

2016-06-29 Thread satishd
Github user satishd commented on the issue:

https://github.com/apache/storm/pull/1528
  
@HeartSaVioR You may want to use `(declare rm-topo-files)` in the start so 
that you do not need to move `defn sync-processes` for handling forward 
references. This may make it easy to review 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.
---


[jira] [Commented] (STORM-1934) Race condition between sync-supervisor and sync-processes raises several strange issues

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1934?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15356362#comment-15356362
 ] 

ASF GitHub Bot commented on STORM-1934:
---

Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/1528
  
Added some comments. I'll squash once reviewing is done.


> Race condition between sync-supervisor and sync-processes raises several 
> strange issues
> ---
>
> Key: STORM-1934
> URL: https://issues.apache.org/jira/browse/STORM-1934
> Project: Apache Storm
>  Issue Type: Bug
>  Components: storm-core
>Affects Versions: 1.0.0, 2.0.0, 1.0.1
>Reporter: Jungtaek Lim
>Assignee: Jungtaek Lim
>Priority: Critical
>
> There're some strange issues including STORM-1933 and others (which I will 
> file an issue soon) which are related to race condition in supervisor.
> As I mentioned to STORM-1933, basically sync-supervisor relies on zk 
> assignment, and sync-processes relies on local assignment and local workers 
> directory, but in fact sync-supervisor also access local state and take some 
> actions which affects sync-processes. And also Satish left the comment to 
> STORM-1933 describing other issue related to race condition and idea to fix 
> this which is same page on me.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[GitHub] storm issue #1528: STORM-1934 Fix race condition between sync-supervisor and...

2016-06-29 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/1528
  
Added some comments. I'll squash once reviewing is done.


---
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.
---


[jira] [Commented] (STORM-1879) Supervisor may not shut down workers cleanly

2016-06-29 Thread Jungtaek Lim (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1879?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15356337#comment-15356337
 ] 

Jungtaek Lim commented on STORM-1879:
-

[~Srdo] [~nico.meyer] [~kevinconaway] [~fogetti]
Sorry to mention so lately, but could anyone apply my patch to see it resolves 
what you're affected?
Please mention me with your storm version if you want to receive modified core 
jar instead of building your own.
Thanks in advance.

> Supervisor may not shut down workers cleanly
> 
>
> Key: STORM-1879
> URL: https://issues.apache.org/jira/browse/STORM-1879
> Project: Apache Storm
>  Issue Type: Bug
>  Components: storm-core
>Affects Versions: 1.0.1
>Reporter: Stig Rohde Døssing
> Attachments: fix_missing_worker_pid.patch, nimbus-supervisor.zip, 
> supervisor.log
>
>
> We've run into a strange issue with a zombie worker process. It looks like 
> the worker pid file somehow got deleted without the worker process shutting 
> down. This causes the supervisor to try repeatedly to kill the worker 
> unsuccessfully, and means multiple workers may be assigned to the same port. 
> The worker root folder sticks around because the worker is still heartbeating 
> to it.
> It may or may not be related that we've seen Nimbus occasionally enter an 
> infinite loop of printing logs similar to the below.
> {code}
> 2016-05-19 14:55:14.196 o.a.s.b.BlobStoreUtils [ERROR] Could not update the 
> blob with keyZendeskTicketTopology-5-1463647641-stormconf.ser
> 2016-05-19 14:55:14.210 o.a.s.b.BlobStoreUtils [ERROR] Could not update the 
> blob with keyZendeskTicketTopology-5-1463647641-stormcode.ser
> 2016-05-19 14:55:14.218 o.a.s.b.BlobStoreUtils [ERROR] Could not update the 
> blob with keyZendeskTicketTopology-5-1463647641-stormconf.ser
> 2016-05-19 14:55:14.256 o.a.s.b.BlobStoreUtils [ERROR] Could not update the 
> blob with keyZendeskTicketTopology-5-1463647641-stormcode.ser
> 2016-05-19 14:55:14.273 o.a.s.b.BlobStoreUtils [ERROR] Could not update the 
> blob with keyZendeskTicketTopology-5-1463647641-stormcode.ser
> 2016-05-19 14:55:14.316 o.a.s.b.BlobStoreUtils [ERROR] Could not update the 
> blob with keyZendeskTicketTopology-5-1463647641-stormconf.ser
> {code}
> Which continues until Nimbus is rebooted. We also see repeating blocks 
> similar to the logs below.
> {code}
> 2016-06-02 07:45:03.656 o.a.s.d.nimbus [INFO] Cleaning up 
> ZendeskTicketTopology-127-1464780171
> 2016-06-02 07:45:04.132 o.a.s.d.nimbus [INFO] 
> ExceptionKeyNotFoundException(msg:ZendeskTicketTopology-127-1464780171-stormjar.jar)
> 2016-06-02 07:45:04.144 o.a.s.d.nimbus [INFO] 
> ExceptionKeyNotFoundException(msg:ZendeskTicketTopology-127-1464780171-stormconf.ser)
> 2016-06-02 07:45:04.155 o.a.s.d.nimbus [INFO] 
> ExceptionKeyNotFoundException(msg:ZendeskTicketTopology-127-1464780171-stormcode.ser)
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


Re: [VOTE] Release Apache Storm 1.0.2 (rc2)

2016-06-29 Thread P. Taylor Goetz
+1 That's what I was holding off on. 

I'll cancel. 

> On Jun 29, 2016, at 5:10 PM, Jungtaek Lim  wrote:
> 
> Seems like we forgot this vote.
> 
> By the way, I found the critical issue from supervisor which raises various
> issues which are reported by users.
> https://issues.apache.org/jira/browse/STORM-1934
> (Patch is in reviewing)
> 
> IMO this should be included to bugfix release ASAP, so I'd rather ask
> ourselves to cancel the vote and trigger new RC ASAP.
> 
> What do you think?
> 
> Thanks,
> Jungtaek Lim (HeartSaVioR)
> 
> 2016년 6월 21일 (화) 오전 12:18, P. Taylor Goetz 님이 작성:
> 
>> +1 (binding)
>> 
>> - Verified build from source archive with `mvn clean install -P all-tests`
>> - Checked LICENSE and NOTICE files
>> - Deployed to a small cluster and tested a variety of topologies.
>> 
>> -Taylor
>> 
>>> On Jun 10, 2016, at 3:26 PM, P. Taylor Goetz  wrote:
>>> 
>>> This is a call to vote on releasing Apache Storm 1.0.2 (rc2)
>>> 
>>> Full list of changes in this release:
>> https://git-wip-us.apache.org/repos/asf?p=storm.git;a=blob_plain;f=CHANGELOG.md;hb=ea6202b9a2398e9fbed718ff15f3d31f7f52b437
>>> 
>>> The tag/commit to be voted upon is v1.0.2:
>> https://git-wip-us.apache.org/repos/asf?p=storm.git;a=tree;h=a73edfdf6ca18ac8d273cf44c7b68cc53f303edd;hb=ea6202b9a2398e9fbed718ff15f3d31f7f52b437
>>> 
>>> The source archive being voted upon can be found here:
>> https://dist.apache.org/repos/dist/dev/storm/apache-storm-1.0.2-rc2/apache-storm-1.0.2-src.tar.gz
>>> 
>>> Other release files, signatures and digests can be found here:
>>> 
>>> https://dist.apache.org/repos/dist/dev/storm/apache-storm-1.0.2-rc2/
>>> 
>>> The release artifacts are signed with the following key:
>> https://git-wip-us.apache.org/repos/asf?p=storm.git;a=blob_plain;f=KEYS;hb=22b832708295fa2c15c4f3c70ac0d2bc6fded4bd
>>> 
>>> The Nexus staging repository for this release is:
>>> 
>>> https://repository.apache.org/content/repositories/orgapachestorm-1036
>>> 
>>> Please vote on releasing this package as Apache Storm 1.0.2.
>>> 
>>> When voting, please list the actions taken to verify the release.
>>> 
>>> This vote will be open for at least 72 hours.
>>> 
>>> [ ] +1 Release this package as Apache Storm 1.0.2
>>> [ ]  0 No opinion
>>> [ ] -1 Do not release this package because...
>>> 
>>> Thanks to everyone who contributed to this release.
>>> 
>>> -Taylor
>> 
>> 


[jira] [Commented] (STORM-1919) Introduce FilterBolt on storm-redis

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1919?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15356231#comment-15356231
 ] 

ASF GitHub Bot commented on STORM-1919:
---

Github user HeartSaVioR commented on a diff in the pull request:

https://github.com/apache/storm/pull/1517#discussion_r69050854
  
--- Diff: 
external/storm-redis/src/test/java/org/apache/storm/redis/topology/WhitelistWordCount.java
 ---
@@ -0,0 +1,155 @@
+/**
+ * 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.topology;
+
+import org.apache.storm.Config;
+import org.apache.storm.LocalCluster;
+import org.apache.storm.StormSubmitter;
+import org.apache.storm.redis.bolt.RedisFilterBolt;
+import org.apache.storm.redis.common.config.JedisPoolConfig;
+import org.apache.storm.redis.common.mapper.RedisDataTypeDescription;
+import org.apache.storm.redis.common.mapper.RedisFilterMapper;
+import org.apache.storm.task.OutputCollector;
+import org.apache.storm.task.TopologyContext;
+import org.apache.storm.topology.OutputFieldsDeclarer;
+import org.apache.storm.topology.TopologyBuilder;
+import org.apache.storm.topology.base.BaseRichBolt;
+import org.apache.storm.tuple.Fields;
+import org.apache.storm.tuple.ITuple;
+import org.apache.storm.tuple.Tuple;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Map;
+import java.util.Random;
+
+public class WhitelistWordCount {
--- End diff --

@abhishekagarwal87 
I'm OK for that, but I'd prefer to get this out of scope for pull request.
This needs discussion to have a generic rule across all external modules 
rather than fix for only this module.
Could you initiate the discussion to dev@ or file an issue? Thanks in 
advance!


> Introduce FilterBolt on storm-redis
> ---
>
> Key: STORM-1919
> URL: https://issues.apache.org/jira/browse/STORM-1919
> Project: Apache Storm
>  Issue Type: New Feature
>  Components: storm-redis
>Reporter: Jungtaek Lim
>Assignee: Jungtaek Lim
>
> While discussing about STORM-1880, it would be better to have FilterBolt 
> explicitly instead of letting users set up their lookup mapper to act as 
> filter.
> There's other benefit here: we can use exists / hexists on STRING / HASH 
> datatype instead of retrieving actual value which reduces execution time / 
> latency from Redis side.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[GitHub] storm pull request #1517: STORM-1919 Introduce FilterBolt on storm-redis

2016-06-29 Thread HeartSaVioR
Github user HeartSaVioR commented on a diff in the pull request:

https://github.com/apache/storm/pull/1517#discussion_r69050854
  
--- Diff: 
external/storm-redis/src/test/java/org/apache/storm/redis/topology/WhitelistWordCount.java
 ---
@@ -0,0 +1,155 @@
+/**
+ * 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.topology;
+
+import org.apache.storm.Config;
+import org.apache.storm.LocalCluster;
+import org.apache.storm.StormSubmitter;
+import org.apache.storm.redis.bolt.RedisFilterBolt;
+import org.apache.storm.redis.common.config.JedisPoolConfig;
+import org.apache.storm.redis.common.mapper.RedisDataTypeDescription;
+import org.apache.storm.redis.common.mapper.RedisFilterMapper;
+import org.apache.storm.task.OutputCollector;
+import org.apache.storm.task.TopologyContext;
+import org.apache.storm.topology.OutputFieldsDeclarer;
+import org.apache.storm.topology.TopologyBuilder;
+import org.apache.storm.topology.base.BaseRichBolt;
+import org.apache.storm.tuple.Fields;
+import org.apache.storm.tuple.ITuple;
+import org.apache.storm.tuple.Tuple;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Map;
+import java.util.Random;
+
+public class WhitelistWordCount {
--- End diff --

@abhishekagarwal87 
I'm OK for that, but I'd prefer to get this out of scope for pull request.
This needs discussion to have a generic rule across all external modules 
rather than fix for only this module.
Could you initiate the discussion to dev@ or file an issue? 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.
---


Re: [VOTE] Release Apache Storm 1.0.2 (rc2)

2016-06-29 Thread Jungtaek Lim
Seems like we forgot this vote.

By the way, I found the critical issue from supervisor which raises various
issues which are reported by users.
https://issues.apache.org/jira/browse/STORM-1934
(Patch is in reviewing)

IMO this should be included to bugfix release ASAP, so I'd rather ask
ourselves to cancel the vote and trigger new RC ASAP.

What do you think?

Thanks,
Jungtaek Lim (HeartSaVioR)

2016년 6월 21일 (화) 오전 12:18, P. Taylor Goetz 님이 작성:

> +1 (binding)
>
> - Verified build from source archive with `mvn clean install -P all-tests`
> - Checked LICENSE and NOTICE files
> - Deployed to a small cluster and tested a variety of topologies.
>
> -Taylor
>
> > On Jun 10, 2016, at 3:26 PM, P. Taylor Goetz  wrote:
> >
> > This is a call to vote on releasing Apache Storm 1.0.2 (rc2)
> >
> > Full list of changes in this release:
> >
> >
> https://git-wip-us.apache.org/repos/asf?p=storm.git;a=blob_plain;f=CHANGELOG.md;hb=ea6202b9a2398e9fbed718ff15f3d31f7f52b437
> >
> > The tag/commit to be voted upon is v1.0.2:
> >
> >
> https://git-wip-us.apache.org/repos/asf?p=storm.git;a=tree;h=a73edfdf6ca18ac8d273cf44c7b68cc53f303edd;hb=ea6202b9a2398e9fbed718ff15f3d31f7f52b437
> >
> > The source archive being voted upon can be found here:
> >
> >
> https://dist.apache.org/repos/dist/dev/storm/apache-storm-1.0.2-rc2/apache-storm-1.0.2-src.tar.gz
> >
> > Other release files, signatures and digests can be found here:
> >
> > https://dist.apache.org/repos/dist/dev/storm/apache-storm-1.0.2-rc2/
> >
> > The release artifacts are signed with the following key:
> >
> >
> https://git-wip-us.apache.org/repos/asf?p=storm.git;a=blob_plain;f=KEYS;hb=22b832708295fa2c15c4f3c70ac0d2bc6fded4bd
> >
> > The Nexus staging repository for this release is:
> >
> > https://repository.apache.org/content/repositories/orgapachestorm-1036
> >
> > Please vote on releasing this package as Apache Storm 1.0.2.
> >
> > When voting, please list the actions taken to verify the release.
> >
> > This vote will be open for at least 72 hours.
> >
> > [ ] +1 Release this package as Apache Storm 1.0.2
> > [ ]  0 No opinion
> > [ ] -1 Do not release this package because...
> >
> > Thanks to everyone who contributed to this release.
> >
> > -Taylor
>
>


[jira] [Commented] (STORM-1936) Support default value for WindowedBolt

2016-06-29 Thread Matthias J. Sax (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1936?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15355572#comment-15355572
 ] 

Matthias J. Sax commented on STORM-1936:


[~darion] Can you elaborate a little more, what this is about? Was there a 
discussion on the mailing list, that I did miss?

> Support default value for WindowedBolt
> --
>
> Key: STORM-1936
> URL: https://issues.apache.org/jira/browse/STORM-1936
> Project: Apache Storm
>  Issue Type: New Feature
>  Components: storm-core
>Reporter: darion yaphet
>Assignee: darion yaphet
>




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Created] (STORM-1936) Support default value for WindowedBolt

2016-06-29 Thread darion yaphet (JIRA)
darion yaphet created STORM-1936:


 Summary: Support default value for WindowedBolt
 Key: STORM-1936
 URL: https://issues.apache.org/jira/browse/STORM-1936
 Project: Apache Storm
  Issue Type: New Feature
  Components: storm-core
Reporter: darion yaphet
Assignee: darion yaphet






--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Commented] (STORM-1934) Race condition between sync-supervisor and sync-processes raises several strange issues

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1934?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15355445#comment-15355445
 ] 

ASF GitHub Bot commented on STORM-1934:
---

Github user satishd commented on the issue:

https://github.com/apache/storm/pull/1528
  
+1 LGTM. 


> Race condition between sync-supervisor and sync-processes raises several 
> strange issues
> ---
>
> Key: STORM-1934
> URL: https://issues.apache.org/jira/browse/STORM-1934
> Project: Apache Storm
>  Issue Type: Bug
>  Components: storm-core
>Affects Versions: 1.0.0, 2.0.0, 1.0.1
>Reporter: Jungtaek Lim
>Assignee: Jungtaek Lim
>Priority: Critical
>
> There're some strange issues including STORM-1933 and others (which I will 
> file an issue soon) which are related to race condition in supervisor.
> As I mentioned to STORM-1933, basically sync-supervisor relies on zk 
> assignment, and sync-processes relies on local assignment and local workers 
> directory, but in fact sync-supervisor also access local state and take some 
> actions which affects sync-processes. And also Satish left the comment to 
> STORM-1933 describing other issue related to race condition and idea to fix 
> this which is same page on me.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[GitHub] storm issue #1528: STORM-1934 Fix race condition between sync-supervisor and...

2016-06-29 Thread satishd
Github user satishd commented on the issue:

https://github.com/apache/storm/pull/1528
  
@HeartSaVioR added resolution details in the description of the PR. Adding 
one more point here: `(kill-existing-workers-with-change-in-components 
supervisor existing-assignment new-assignment)` is not needed in 
`sync-supervisor` as it kills all the workers with invalid state which includes 
the old workers and rebalanced workers.


---
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.
---


[jira] [Commented] (STORM-1934) Race condition between sync-supervisor and sync-processes raises several strange issues

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1934?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15355442#comment-15355442
 ] 

ASF GitHub Bot commented on STORM-1934:
---

Github user satishd commented on the issue:

https://github.com/apache/storm/pull/1528
  
@HeartSaVioR added resolution details in the description of the PR. Adding 
one more point here: `(kill-existing-workers-with-change-in-components 
supervisor existing-assignment new-assignment)` is not needed in 
`sync-supervisor` as it kills all the workers with invalid state which includes 
the old workers and rebalanced workers.


> Race condition between sync-supervisor and sync-processes raises several 
> strange issues
> ---
>
> Key: STORM-1934
> URL: https://issues.apache.org/jira/browse/STORM-1934
> Project: Apache Storm
>  Issue Type: Bug
>  Components: storm-core
>Affects Versions: 1.0.0, 2.0.0, 1.0.1
>Reporter: Jungtaek Lim
>Assignee: Jungtaek Lim
>Priority: Critical
>
> There're some strange issues including STORM-1933 and others (which I will 
> file an issue soon) which are related to race condition in supervisor.
> As I mentioned to STORM-1933, basically sync-supervisor relies on zk 
> assignment, and sync-processes relies on local assignment and local workers 
> directory, but in fact sync-supervisor also access local state and take some 
> actions which affects sync-processes. And also Satish left the comment to 
> STORM-1933 describing other issue related to race condition and idea to fix 
> this which is same page on me.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[GitHub] storm issue #1528: STORM-1934 Fix race condition between sync-supervisor and...

2016-06-29 Thread satishd
Github user satishd commented on the issue:

https://github.com/apache/storm/pull/1528
  
+1 LGTM. 


---
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.
---


[jira] [Commented] (STORM-1934) Race condition between sync-supervisor and sync-processes raises several strange issues

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1934?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15355330#comment-15355330
 ] 

ASF GitHub Bot commented on STORM-1934:
---

Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/1528
  
I'd like to get reviewed by @kishorvpatil if (but only if) he's available 
since it removes the changeset of STORM-1561. Since it's critical issue and 
related to race condition I'd also like to get reviewed by more contributors / 
committers. 
And that would be really nice if affected users can apply this patch to see 
if this helps. I'll ping reporters for relevant issues and ask them to help 
testing.


> Race condition between sync-supervisor and sync-processes raises several 
> strange issues
> ---
>
> Key: STORM-1934
> URL: https://issues.apache.org/jira/browse/STORM-1934
> Project: Apache Storm
>  Issue Type: Bug
>  Components: storm-core
>Affects Versions: 1.0.0, 2.0.0, 1.0.1
>Reporter: Jungtaek Lim
>Assignee: Jungtaek Lim
>Priority: Critical
>
> There're some strange issues including STORM-1933 and others (which I will 
> file an issue soon) which are related to race condition in supervisor.
> As I mentioned to STORM-1933, basically sync-supervisor relies on zk 
> assignment, and sync-processes relies on local assignment and local workers 
> directory, but in fact sync-supervisor also access local state and take some 
> actions which affects sync-processes. And also Satish left the comment to 
> STORM-1933 describing other issue related to race condition and idea to fix 
> this which is same page on me.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[GitHub] storm issue #1528: STORM-1934 Fix race condition between sync-supervisor and...

2016-06-29 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/1528
  
I'd like to get reviewed by @kishorvpatil if (but only if) he's available 
since it removes the changeset of STORM-1561. Since it's critical issue and 
related to race condition I'd also like to get reviewed by more contributors / 
committers. 
And that would be really nice if affected users can apply this patch to see 
if this helps. I'll ping reporters for relevant issues and ask them to help 
testing.


---
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.
---


[jira] [Commented] (STORM-1919) Introduce FilterBolt on storm-redis

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1919?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15355317#comment-15355317
 ] 

ASF GitHub Bot commented on STORM-1919:
---

Github user abhishekagarwal87 commented on a diff in the pull request:

https://github.com/apache/storm/pull/1517#discussion_r68954287
  
--- Diff: 
external/storm-redis/src/test/java/org/apache/storm/redis/topology/WhitelistWordCount.java
 ---
@@ -0,0 +1,155 @@
+/**
+ * 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.topology;
+
+import org.apache.storm.Config;
+import org.apache.storm.LocalCluster;
+import org.apache.storm.StormSubmitter;
+import org.apache.storm.redis.bolt.RedisFilterBolt;
+import org.apache.storm.redis.common.config.JedisPoolConfig;
+import org.apache.storm.redis.common.mapper.RedisDataTypeDescription;
+import org.apache.storm.redis.common.mapper.RedisFilterMapper;
+import org.apache.storm.task.OutputCollector;
+import org.apache.storm.task.TopologyContext;
+import org.apache.storm.topology.OutputFieldsDeclarer;
+import org.apache.storm.topology.TopologyBuilder;
+import org.apache.storm.topology.base.BaseRichBolt;
+import org.apache.storm.tuple.Fields;
+import org.apache.storm.tuple.ITuple;
+import org.apache.storm.tuple.Tuple;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Map;
+import java.util.Random;
+
+public class WhitelistWordCount {
--- End diff --

They do. One reason I know of is the dependency collision. But test doesn't 
seem the right folder, because a user is not going to look in the test folder. 
We can do the following - 
1. Module level sub-directories within examples e.g. examples/storm-redis, 
examples/storm-opentsdb etc.
2. examples file within the module which are not shipped with jar. The link 
to such examples can be put up in the documentation so that users can refer to 
the example code and copy/run it accordingly. 
what do you say?


> Introduce FilterBolt on storm-redis
> ---
>
> Key: STORM-1919
> URL: https://issues.apache.org/jira/browse/STORM-1919
> Project: Apache Storm
>  Issue Type: New Feature
>  Components: storm-redis
>Reporter: Jungtaek Lim
>Assignee: Jungtaek Lim
>
> While discussing about STORM-1880, it would be better to have FilterBolt 
> explicitly instead of letting users set up their lookup mapper to act as 
> filter.
> There's other benefit here: we can use exists / hexists on STRING / HASH 
> datatype instead of retrieving actual value which reduces execution time / 
> latency from Redis side.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Commented] (STORM-1934) Race condition between sync-supervisor and sync-processes raises several strange issues

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1934?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15355318#comment-15355318
 ] 

ASF GitHub Bot commented on STORM-1934:
---

Github user arunmahadevan commented on the issue:

https://github.com/apache/storm/pull/1528
  
Thanks @HeartSaVioR. It looks good.

+1


> Race condition between sync-supervisor and sync-processes raises several 
> strange issues
> ---
>
> Key: STORM-1934
> URL: https://issues.apache.org/jira/browse/STORM-1934
> Project: Apache Storm
>  Issue Type: Bug
>  Components: storm-core
>Affects Versions: 1.0.0, 2.0.0, 1.0.1
>Reporter: Jungtaek Lim
>Assignee: Jungtaek Lim
>Priority: Critical
>
> There're some strange issues including STORM-1933 and others (which I will 
> file an issue soon) which are related to race condition in supervisor.
> As I mentioned to STORM-1933, basically sync-supervisor relies on zk 
> assignment, and sync-processes relies on local assignment and local workers 
> directory, but in fact sync-supervisor also access local state and take some 
> actions which affects sync-processes. And also Satish left the comment to 
> STORM-1933 describing other issue related to race condition and idea to fix 
> this which is same page on me.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[GitHub] storm pull request #1517: STORM-1919 Introduce FilterBolt on storm-redis

2016-06-29 Thread abhishekagarwal87
Github user abhishekagarwal87 commented on a diff in the pull request:

https://github.com/apache/storm/pull/1517#discussion_r68954287
  
--- Diff: 
external/storm-redis/src/test/java/org/apache/storm/redis/topology/WhitelistWordCount.java
 ---
@@ -0,0 +1,155 @@
+/**
+ * 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.topology;
+
+import org.apache.storm.Config;
+import org.apache.storm.LocalCluster;
+import org.apache.storm.StormSubmitter;
+import org.apache.storm.redis.bolt.RedisFilterBolt;
+import org.apache.storm.redis.common.config.JedisPoolConfig;
+import org.apache.storm.redis.common.mapper.RedisDataTypeDescription;
+import org.apache.storm.redis.common.mapper.RedisFilterMapper;
+import org.apache.storm.task.OutputCollector;
+import org.apache.storm.task.TopologyContext;
+import org.apache.storm.topology.OutputFieldsDeclarer;
+import org.apache.storm.topology.TopologyBuilder;
+import org.apache.storm.topology.base.BaseRichBolt;
+import org.apache.storm.tuple.Fields;
+import org.apache.storm.tuple.ITuple;
+import org.apache.storm.tuple.Tuple;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Map;
+import java.util.Random;
+
+public class WhitelistWordCount {
--- End diff --

They do. One reason I know of is the dependency collision. But test doesn't 
seem the right folder, because a user is not going to look in the test folder. 
We can do the following - 
1. Module level sub-directories within examples e.g. examples/storm-redis, 
examples/storm-opentsdb etc.
2. examples file within the module which are not shipped with jar. The link 
to such examples can be put up in the documentation so that users can refer to 
the example code and copy/run it accordingly. 
what do you say?


---
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 issue #1528: STORM-1934 Fix race condition between sync-supervisor and...

2016-06-29 Thread arunmahadevan
Github user arunmahadevan commented on the issue:

https://github.com/apache/storm/pull/1528
  
Thanks @HeartSaVioR. It looks good.

+1


---
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.
---


[jira] [Commented] (STORM-1934) Race condition between sync-supervisor and sync-processes raises several strange issues

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1934?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15355314#comment-15355314
 ] 

ASF GitHub Bot commented on STORM-1934:
---

Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/1528
  
@arunmahadevan 
This works perfectly.

- Writing new assignment
```
6700 {:storm-id "test-topology2-4-1467185073", :executors ([7 7] [4 4] [1 
1]), :resources [0.0 0.0 0.0]}, 
6702 {:storm-id "test-topology2-4-1467185073", :executors ([6 6] [3 3]), 
:resources [0.0 0.0 0.0]}, 
6701 {:storm-id "test-topology2-4-1467185073", :executors ([5 5] [2 2]), 
:resources [0.0 0.0 0.0]}
```

- Assigned executors:
```
{6700 {:storm-id "test-topology2-4-1467185073", :executors [[7 7] [4 4] [1 
1]], :resources #object[org.apache.storm.generated.WorkerResources 0x6dabc02d 
"WorkerResources(mem_on_heap:0.0, mem_off_heap:0.0, cpu:0.0)"]}, 
6701 {:storm-id "test-topology2-4-1467185073", :executors [[5 5] [2 2]], 
:resources #object[org.apache.storm.generated.WorkerResources 0x6de46954 
"WorkerResources(mem_on_heap:0.0, mem_off_heap:0.0, cpu:0.0)"]}, 
6702 {:storm-id "test-topology2-4-1467185073", :executors [[6 6] [3 3]], 
:resources #object[org.apache.storm.generated.WorkerResources 0x60f1fbb5 
"WorkerResources(mem_on_heap:0.0, mem_off_heap:0.0, cpu:0.0)"]}}
```

- Allocated:
```
"6120-0ddc-4820-9062-c79276d56cf2" [:disallowed {:time-secs 1467209575, 
:storm-id "test-topology2-4-1467185073", :executors [[2 2] [6 6] [-1 -1] [4 
4]], :port 6702}], 
"9751163a-f7b8-46a5-9b9c-bcd1fd1446cd" [:disallowed {:time-secs 1467209574, 
:storm-id "test-topology2-4-1467185073", :executors [[7 7] [3 3] [1 1] [-1 -1] 
[5 5]], :port 6701}]
```

Now three workers are running.


> Race condition between sync-supervisor and sync-processes raises several 
> strange issues
> ---
>
> Key: STORM-1934
> URL: https://issues.apache.org/jira/browse/STORM-1934
> Project: Apache Storm
>  Issue Type: Bug
>  Components: storm-core
>Affects Versions: 1.0.0, 2.0.0, 1.0.1
>Reporter: Jungtaek Lim
>Assignee: Jungtaek Lim
>Priority: Critical
>
> There're some strange issues including STORM-1933 and others (which I will 
> file an issue soon) which are related to race condition in supervisor.
> As I mentioned to STORM-1933, basically sync-supervisor relies on zk 
> assignment, and sync-processes relies on local assignment and local workers 
> directory, but in fact sync-supervisor also access local state and take some 
> actions which affects sync-processes. And also Satish left the comment to 
> STORM-1933 describing other issue related to race condition and idea to fix 
> this which is same page on me.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[GitHub] storm issue #1528: STORM-1934 Fix race condition between sync-supervisor and...

2016-06-29 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/1528
  
@arunmahadevan 
This works perfectly.

- Writing new assignment
```
6700 {:storm-id "test-topology2-4-1467185073", :executors ([7 7] [4 4] [1 
1]), :resources [0.0 0.0 0.0]}, 
6702 {:storm-id "test-topology2-4-1467185073", :executors ([6 6] [3 3]), 
:resources [0.0 0.0 0.0]}, 
6701 {:storm-id "test-topology2-4-1467185073", :executors ([5 5] [2 2]), 
:resources [0.0 0.0 0.0]}
```

- Assigned executors:
```
{6700 {:storm-id "test-topology2-4-1467185073", :executors [[7 7] [4 4] [1 
1]], :resources #object[org.apache.storm.generated.WorkerResources 0x6dabc02d 
"WorkerResources(mem_on_heap:0.0, mem_off_heap:0.0, cpu:0.0)"]}, 
6701 {:storm-id "test-topology2-4-1467185073", :executors [[5 5] [2 2]], 
:resources #object[org.apache.storm.generated.WorkerResources 0x6de46954 
"WorkerResources(mem_on_heap:0.0, mem_off_heap:0.0, cpu:0.0)"]}, 
6702 {:storm-id "test-topology2-4-1467185073", :executors [[6 6] [3 3]], 
:resources #object[org.apache.storm.generated.WorkerResources 0x60f1fbb5 
"WorkerResources(mem_on_heap:0.0, mem_off_heap:0.0, cpu:0.0)"]}}
```

- Allocated:
```
"6120-0ddc-4820-9062-c79276d56cf2" [:disallowed {:time-secs 1467209575, 
:storm-id "test-topology2-4-1467185073", :executors [[2 2] [6 6] [-1 -1] [4 
4]], :port 6702}], 
"9751163a-f7b8-46a5-9b9c-bcd1fd1446cd" [:disallowed {:time-secs 1467209574, 
:storm-id "test-topology2-4-1467185073", :executors [[7 7] [3 3] [1 1] [-1 -1] 
[5 5]], :port 6701}]
```

Now three workers are running.


---
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.
---


[jira] [Commented] (STORM-1934) Race condition between sync-supervisor and sync-processes raises several strange issues

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1934?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15355298#comment-15355298
 ] 

ASF GitHub Bot commented on STORM-1934:
---

Github user arunmahadevan commented on the issue:

https://github.com/apache/storm/pull/1528
  
@HeartSaVioR thanks for the clarification, it makes sense now. Can you also 
check rebalance from 2 -> 3 workers ?


> Race condition between sync-supervisor and sync-processes raises several 
> strange issues
> ---
>
> Key: STORM-1934
> URL: https://issues.apache.org/jira/browse/STORM-1934
> Project: Apache Storm
>  Issue Type: Bug
>  Components: storm-core
>Affects Versions: 1.0.0, 2.0.0, 1.0.1
>Reporter: Jungtaek Lim
>Assignee: Jungtaek Lim
>Priority: Critical
>
> There're some strange issues including STORM-1933 and others (which I will 
> file an issue soon) which are related to race condition in supervisor.
> As I mentioned to STORM-1933, basically sync-supervisor relies on zk 
> assignment, and sync-processes relies on local assignment and local workers 
> directory, but in fact sync-supervisor also access local state and take some 
> actions which affects sync-processes. And also Satish left the comment to 
> STORM-1933 describing other issue related to race condition and idea to fix 
> this which is same page on me.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[GitHub] storm issue #1528: STORM-1934 Fix race condition between sync-supervisor and...

2016-06-29 Thread arunmahadevan
Github user arunmahadevan commented on the issue:

https://github.com/apache/storm/pull/1528
  
@HeartSaVioR thanks for the clarification, it makes sense now. Can you also 
check rebalance from 2 -> 3 workers ?


---
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.
---


[jira] [Commented] (STORM-1919) Introduce FilterBolt on storm-redis

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1919?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15355248#comment-15355248
 ] 

ASF GitHub Bot commented on STORM-1919:
---

Github user HeartSaVioR commented on a diff in the pull request:

https://github.com/apache/storm/pull/1517#discussion_r68947165
  
--- Diff: 
external/storm-redis/src/test/java/org/apache/storm/redis/topology/WhitelistWordCount.java
 ---
@@ -0,0 +1,155 @@
+/**
+ * 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.topology;
+
+import org.apache.storm.Config;
+import org.apache.storm.LocalCluster;
+import org.apache.storm.StormSubmitter;
+import org.apache.storm.redis.bolt.RedisFilterBolt;
+import org.apache.storm.redis.common.config.JedisPoolConfig;
+import org.apache.storm.redis.common.mapper.RedisDataTypeDescription;
+import org.apache.storm.redis.common.mapper.RedisFilterMapper;
+import org.apache.storm.task.OutputCollector;
+import org.apache.storm.task.TopologyContext;
+import org.apache.storm.topology.OutputFieldsDeclarer;
+import org.apache.storm.topology.TopologyBuilder;
+import org.apache.storm.topology.base.BaseRichBolt;
+import org.apache.storm.tuple.Fields;
+import org.apache.storm.tuple.ITuple;
+import org.apache.storm.tuple.Tuple;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Map;
+import java.util.Random;
+
+public class WhitelistWordCount {
--- End diff --

Not sure where to move. Do you have an idea?
Maybe we would like to have a rule for this case, and apply all external 
modules since many modules already have example topologies in test package. 


> Introduce FilterBolt on storm-redis
> ---
>
> Key: STORM-1919
> URL: https://issues.apache.org/jira/browse/STORM-1919
> Project: Apache Storm
>  Issue Type: New Feature
>  Components: storm-redis
>Reporter: Jungtaek Lim
>Assignee: Jungtaek Lim
>
> While discussing about STORM-1880, it would be better to have FilterBolt 
> explicitly instead of letting users set up their lookup mapper to act as 
> filter.
> There's other benefit here: we can use exists / hexists on STRING / HASH 
> datatype instead of retrieving actual value which reduces execution time / 
> latency from Redis side.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[GitHub] storm pull request #1517: STORM-1919 Introduce FilterBolt on storm-redis

2016-06-29 Thread HeartSaVioR
Github user HeartSaVioR commented on a diff in the pull request:

https://github.com/apache/storm/pull/1517#discussion_r68947209
  
--- Diff: 
external/storm-redis/src/main/java/org/apache/storm/redis/bolt/RedisFilterBolt.java
 ---
@@ -0,0 +1,146 @@
+/**
+ * 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.bolt;
+
+import org.apache.storm.redis.common.config.JedisClusterConfig;
+import org.apache.storm.redis.common.config.JedisPoolConfig;
+import org.apache.storm.redis.common.mapper.RedisDataTypeDescription;
+import org.apache.storm.redis.common.mapper.RedisFilterMapper;
+import org.apache.storm.topology.OutputFieldsDeclarer;
+import org.apache.storm.tuple.Tuple;
+import redis.clients.jedis.GeoCoordinate;
+import redis.clients.jedis.JedisCommands;
+
+import java.util.List;
+
+/**
+ * Basic bolt for querying from Redis and filters out if key/field doesn't 
exist.
+ * If key/field exists on Redis, this bolt just forwards input tuple to 
default stream.
+ * 
+ * Supported data types: STRING, HASH, SET, SORTED_SET, HYPER_LOG_LOG, GEO.
+ * 
+ * Note: For STRING it checks such key exists on the key space.
+ * For HASH and SORTED_SET and GEO, it checks such field exists on that 
data structure.
+ * For SET and HYPER_LOG_LOG, it check such value exists on that data 
structure.
+ * (Note that it still refers key from tuple via 
RedisFilterMapper#getKeyFromTuple())
+ * In order to apply checking this to SET, you need to input additional 
key this case.
+ * 
+ * Note2: If you want to just query about existence of key regardless of 
actual data type,
+ * specify STRING to data type of RedisFilterMapper.
+ */
+public class RedisFilterBolt extends AbstractRedisBolt {
+private final RedisFilterMapper filterMapper;
+private final RedisDataTypeDescription.RedisDataType dataType;
+private final String additionalKey;
+
+/**
+ * Constructor for single Redis environment (JedisPool)
+ * @param config configuration for initializing JedisPool
+ * @param filterMapper mapper containing which datatype, query key 
that Bolt uses
+ */
+public RedisFilterBolt(JedisPoolConfig config, RedisFilterMapper 
filterMapper) {
+super(config);
+
+this.filterMapper = filterMapper;
+
+RedisDataTypeDescription dataTypeDescription = 
filterMapper.getDataTypeDescription();
+this.dataType = dataTypeDescription.getDataType();
+this.additionalKey = dataTypeDescription.getAdditionalKey();
+}
+
+/**
+ * Constructor for Redis Cluster environment (JedisCluster)
+ * @param config configuration for initializing JedisCluster
+ * @param filterMapper mapper containing which datatype, query key 
that Bolt uses
+ */
+public RedisFilterBolt(JedisClusterConfig config, RedisFilterMapper 
filterMapper) {
+super(config);
+
+this.filterMapper = filterMapper;
+
+RedisDataTypeDescription dataTypeDescription = 
filterMapper.getDataTypeDescription();
+this.dataType = dataTypeDescription.getDataType();
+this.additionalKey = dataTypeDescription.getAdditionalKey();
+}
+
+/**
+ * {@inheritDoc}
+ */
+@Override
+public void execute(Tuple input) {
+String key = filterMapper.getKeyFromTuple(input);
+
+boolean found;
+JedisCommands jedisCommand = null;
+try {
+jedisCommand = getInstance();
+
+switch (dataType) {
+case STRING:
+found = jedisCommand.exists(key);
+break;
+
+case SET:
+if (additionalKey == null) {
+throw new IllegalArgumentException("additionalKey 
should be defined");
+}
+found = jedisCommand.sismember(addi

[GitHub] storm pull request #1517: STORM-1919 Introduce FilterBolt on storm-redis

2016-06-29 Thread HeartSaVioR
Github user HeartSaVioR commented on a diff in the pull request:

https://github.com/apache/storm/pull/1517#discussion_r68947207
  
--- Diff: 
external/storm-redis/src/main/java/org/apache/storm/redis/bolt/RedisFilterBolt.java
 ---
@@ -0,0 +1,146 @@
+/**
+ * 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.bolt;
+
+import org.apache.storm.redis.common.config.JedisClusterConfig;
+import org.apache.storm.redis.common.config.JedisPoolConfig;
+import org.apache.storm.redis.common.mapper.RedisDataTypeDescription;
+import org.apache.storm.redis.common.mapper.RedisFilterMapper;
+import org.apache.storm.topology.OutputFieldsDeclarer;
+import org.apache.storm.tuple.Tuple;
+import redis.clients.jedis.GeoCoordinate;
+import redis.clients.jedis.JedisCommands;
+
+import java.util.List;
+
+/**
+ * Basic bolt for querying from Redis and filters out if key/field doesn't 
exist.
+ * If key/field exists on Redis, this bolt just forwards input tuple to 
default stream.
+ * 
+ * Supported data types: STRING, HASH, SET, SORTED_SET, HYPER_LOG_LOG, GEO.
+ * 
+ * Note: For STRING it checks such key exists on the key space.
+ * For HASH and SORTED_SET and GEO, it checks such field exists on that 
data structure.
+ * For SET and HYPER_LOG_LOG, it check such value exists on that data 
structure.
+ * (Note that it still refers key from tuple via 
RedisFilterMapper#getKeyFromTuple())
+ * In order to apply checking this to SET, you need to input additional 
key this case.
+ * 
+ * Note2: If you want to just query about existence of key regardless of 
actual data type,
+ * specify STRING to data type of RedisFilterMapper.
+ */
+public class RedisFilterBolt extends AbstractRedisBolt {
+private final RedisFilterMapper filterMapper;
+private final RedisDataTypeDescription.RedisDataType dataType;
+private final String additionalKey;
+
+/**
+ * Constructor for single Redis environment (JedisPool)
+ * @param config configuration for initializing JedisPool
+ * @param filterMapper mapper containing which datatype, query key 
that Bolt uses
+ */
+public RedisFilterBolt(JedisPoolConfig config, RedisFilterMapper 
filterMapper) {
+super(config);
+
+this.filterMapper = filterMapper;
+
+RedisDataTypeDescription dataTypeDescription = 
filterMapper.getDataTypeDescription();
+this.dataType = dataTypeDescription.getDataType();
+this.additionalKey = dataTypeDescription.getAdditionalKey();
+}
+
+/**
+ * Constructor for Redis Cluster environment (JedisCluster)
+ * @param config configuration for initializing JedisCluster
+ * @param filterMapper mapper containing which datatype, query key 
that Bolt uses
+ */
+public RedisFilterBolt(JedisClusterConfig config, RedisFilterMapper 
filterMapper) {
+super(config);
+
+this.filterMapper = filterMapper;
+
+RedisDataTypeDescription dataTypeDescription = 
filterMapper.getDataTypeDescription();
+this.dataType = dataTypeDescription.getDataType();
+this.additionalKey = dataTypeDescription.getAdditionalKey();
+}
+
+/**
+ * {@inheritDoc}
+ */
+@Override
+public void execute(Tuple input) {
+String key = filterMapper.getKeyFromTuple(input);
+
+boolean found;
+JedisCommands jedisCommand = null;
+try {
+jedisCommand = getInstance();
+
+switch (dataType) {
+case STRING:
+found = jedisCommand.exists(key);
+break;
+
+case SET:
+if (additionalKey == null) {
--- End diff --

Addressed.


---
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

[jira] [Commented] (STORM-1919) Introduce FilterBolt on storm-redis

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1919?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15355247#comment-15355247
 ] 

ASF GitHub Bot commented on STORM-1919:
---

Github user HeartSaVioR commented on a diff in the pull request:

https://github.com/apache/storm/pull/1517#discussion_r68947207
  
--- Diff: 
external/storm-redis/src/main/java/org/apache/storm/redis/bolt/RedisFilterBolt.java
 ---
@@ -0,0 +1,146 @@
+/**
+ * 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.bolt;
+
+import org.apache.storm.redis.common.config.JedisClusterConfig;
+import org.apache.storm.redis.common.config.JedisPoolConfig;
+import org.apache.storm.redis.common.mapper.RedisDataTypeDescription;
+import org.apache.storm.redis.common.mapper.RedisFilterMapper;
+import org.apache.storm.topology.OutputFieldsDeclarer;
+import org.apache.storm.tuple.Tuple;
+import redis.clients.jedis.GeoCoordinate;
+import redis.clients.jedis.JedisCommands;
+
+import java.util.List;
+
+/**
+ * Basic bolt for querying from Redis and filters out if key/field doesn't 
exist.
+ * If key/field exists on Redis, this bolt just forwards input tuple to 
default stream.
+ * 
+ * Supported data types: STRING, HASH, SET, SORTED_SET, HYPER_LOG_LOG, GEO.
+ * 
+ * Note: For STRING it checks such key exists on the key space.
+ * For HASH and SORTED_SET and GEO, it checks such field exists on that 
data structure.
+ * For SET and HYPER_LOG_LOG, it check such value exists on that data 
structure.
+ * (Note that it still refers key from tuple via 
RedisFilterMapper#getKeyFromTuple())
+ * In order to apply checking this to SET, you need to input additional 
key this case.
+ * 
+ * Note2: If you want to just query about existence of key regardless of 
actual data type,
+ * specify STRING to data type of RedisFilterMapper.
+ */
+public class RedisFilterBolt extends AbstractRedisBolt {
+private final RedisFilterMapper filterMapper;
+private final RedisDataTypeDescription.RedisDataType dataType;
+private final String additionalKey;
+
+/**
+ * Constructor for single Redis environment (JedisPool)
+ * @param config configuration for initializing JedisPool
+ * @param filterMapper mapper containing which datatype, query key 
that Bolt uses
+ */
+public RedisFilterBolt(JedisPoolConfig config, RedisFilterMapper 
filterMapper) {
+super(config);
+
+this.filterMapper = filterMapper;
+
+RedisDataTypeDescription dataTypeDescription = 
filterMapper.getDataTypeDescription();
+this.dataType = dataTypeDescription.getDataType();
+this.additionalKey = dataTypeDescription.getAdditionalKey();
+}
+
+/**
+ * Constructor for Redis Cluster environment (JedisCluster)
+ * @param config configuration for initializing JedisCluster
+ * @param filterMapper mapper containing which datatype, query key 
that Bolt uses
+ */
+public RedisFilterBolt(JedisClusterConfig config, RedisFilterMapper 
filterMapper) {
+super(config);
+
+this.filterMapper = filterMapper;
+
+RedisDataTypeDescription dataTypeDescription = 
filterMapper.getDataTypeDescription();
+this.dataType = dataTypeDescription.getDataType();
+this.additionalKey = dataTypeDescription.getAdditionalKey();
+}
+
+/**
+ * {@inheritDoc}
+ */
+@Override
+public void execute(Tuple input) {
+String key = filterMapper.getKeyFromTuple(input);
+
+boolean found;
+JedisCommands jedisCommand = null;
+try {
+jedisCommand = getInstance();
+
+switch (dataType) {
+case STRING:
+found = jedisCommand.exists(key);
+break;
+
+

[jira] [Commented] (STORM-1919) Introduce FilterBolt on storm-redis

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1919?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15355249#comment-15355249
 ] 

ASF GitHub Bot commented on STORM-1919:
---

Github user HeartSaVioR commented on a diff in the pull request:

https://github.com/apache/storm/pull/1517#discussion_r68947209
  
--- Diff: 
external/storm-redis/src/main/java/org/apache/storm/redis/bolt/RedisFilterBolt.java
 ---
@@ -0,0 +1,146 @@
+/**
+ * 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.bolt;
+
+import org.apache.storm.redis.common.config.JedisClusterConfig;
+import org.apache.storm.redis.common.config.JedisPoolConfig;
+import org.apache.storm.redis.common.mapper.RedisDataTypeDescription;
+import org.apache.storm.redis.common.mapper.RedisFilterMapper;
+import org.apache.storm.topology.OutputFieldsDeclarer;
+import org.apache.storm.tuple.Tuple;
+import redis.clients.jedis.GeoCoordinate;
+import redis.clients.jedis.JedisCommands;
+
+import java.util.List;
+
+/**
+ * Basic bolt for querying from Redis and filters out if key/field doesn't 
exist.
+ * If key/field exists on Redis, this bolt just forwards input tuple to 
default stream.
+ * 
+ * Supported data types: STRING, HASH, SET, SORTED_SET, HYPER_LOG_LOG, GEO.
+ * 
+ * Note: For STRING it checks such key exists on the key space.
+ * For HASH and SORTED_SET and GEO, it checks such field exists on that 
data structure.
+ * For SET and HYPER_LOG_LOG, it check such value exists on that data 
structure.
+ * (Note that it still refers key from tuple via 
RedisFilterMapper#getKeyFromTuple())
+ * In order to apply checking this to SET, you need to input additional 
key this case.
+ * 
+ * Note2: If you want to just query about existence of key regardless of 
actual data type,
+ * specify STRING to data type of RedisFilterMapper.
+ */
+public class RedisFilterBolt extends AbstractRedisBolt {
+private final RedisFilterMapper filterMapper;
+private final RedisDataTypeDescription.RedisDataType dataType;
+private final String additionalKey;
+
+/**
+ * Constructor for single Redis environment (JedisPool)
+ * @param config configuration for initializing JedisPool
+ * @param filterMapper mapper containing which datatype, query key 
that Bolt uses
+ */
+public RedisFilterBolt(JedisPoolConfig config, RedisFilterMapper 
filterMapper) {
+super(config);
+
+this.filterMapper = filterMapper;
+
+RedisDataTypeDescription dataTypeDescription = 
filterMapper.getDataTypeDescription();
+this.dataType = dataTypeDescription.getDataType();
+this.additionalKey = dataTypeDescription.getAdditionalKey();
+}
+
+/**
+ * Constructor for Redis Cluster environment (JedisCluster)
+ * @param config configuration for initializing JedisCluster
+ * @param filterMapper mapper containing which datatype, query key 
that Bolt uses
+ */
+public RedisFilterBolt(JedisClusterConfig config, RedisFilterMapper 
filterMapper) {
+super(config);
+
+this.filterMapper = filterMapper;
+
+RedisDataTypeDescription dataTypeDescription = 
filterMapper.getDataTypeDescription();
+this.dataType = dataTypeDescription.getDataType();
+this.additionalKey = dataTypeDescription.getAdditionalKey();
+}
+
+/**
+ * {@inheritDoc}
+ */
+@Override
+public void execute(Tuple input) {
+String key = filterMapper.getKeyFromTuple(input);
+
+boolean found;
+JedisCommands jedisCommand = null;
+try {
+jedisCommand = getInstance();
+
+switch (dataType) {
+case STRING:
+found = jedisCommand.exists(key);
+break;
+
+

[GitHub] storm pull request #1517: STORM-1919 Introduce FilterBolt on storm-redis

2016-06-29 Thread HeartSaVioR
Github user HeartSaVioR commented on a diff in the pull request:

https://github.com/apache/storm/pull/1517#discussion_r68947165
  
--- Diff: 
external/storm-redis/src/test/java/org/apache/storm/redis/topology/WhitelistWordCount.java
 ---
@@ -0,0 +1,155 @@
+/**
+ * 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.topology;
+
+import org.apache.storm.Config;
+import org.apache.storm.LocalCluster;
+import org.apache.storm.StormSubmitter;
+import org.apache.storm.redis.bolt.RedisFilterBolt;
+import org.apache.storm.redis.common.config.JedisPoolConfig;
+import org.apache.storm.redis.common.mapper.RedisDataTypeDescription;
+import org.apache.storm.redis.common.mapper.RedisFilterMapper;
+import org.apache.storm.task.OutputCollector;
+import org.apache.storm.task.TopologyContext;
+import org.apache.storm.topology.OutputFieldsDeclarer;
+import org.apache.storm.topology.TopologyBuilder;
+import org.apache.storm.topology.base.BaseRichBolt;
+import org.apache.storm.tuple.Fields;
+import org.apache.storm.tuple.ITuple;
+import org.apache.storm.tuple.Tuple;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Map;
+import java.util.Random;
+
+public class WhitelistWordCount {
--- End diff --

Not sure where to move. Do you have an idea?
Maybe we would like to have a rule for this case, and apply all external 
modules since many modules already have example topologies in test package. 


---
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.
---


[jira] [Commented] (STORM-1934) Race condition between sync-supervisor and sync-processes raises several strange issues

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1934?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15355098#comment-15355098
 ] 

ASF GitHub Bot commented on STORM-1934:
---

Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/1528
  
@arunmahadevan Sorry for confusing: rebalance is done from 3 workers to 2 
workers, and since all executors map are changed so all workers are disallowed 
and two new workers are launched.


> Race condition between sync-supervisor and sync-processes raises several 
> strange issues
> ---
>
> Key: STORM-1934
> URL: https://issues.apache.org/jira/browse/STORM-1934
> Project: Apache Storm
>  Issue Type: Bug
>  Components: storm-core
>Affects Versions: 1.0.0, 2.0.0, 1.0.1
>Reporter: Jungtaek Lim
>Assignee: Jungtaek Lim
>Priority: Critical
>
> There're some strange issues including STORM-1933 and others (which I will 
> file an issue soon) which are related to race condition in supervisor.
> As I mentioned to STORM-1933, basically sync-supervisor relies on zk 
> assignment, and sync-processes relies on local assignment and local workers 
> directory, but in fact sync-supervisor also access local state and take some 
> actions which affects sync-processes. And also Satish left the comment to 
> STORM-1933 describing other issue related to race condition and idea to fix 
> this which is same page on me.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[GitHub] storm issue #1528: STORM-1934 Fix race condition between sync-supervisor and...

2016-06-29 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/1528
  
@arunmahadevan Sorry for confusing: rebalance is done from 3 workers to 2 
workers, and since all executors map are changed so all workers are disallowed 
and two new workers are launched.


---
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.
---


[jira] [Commented] (STORM-1934) Race condition between sync-supervisor and sync-processes raises several strange issues

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1934?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15355038#comment-15355038
 ] 

ASF GitHub Bot commented on STORM-1934:
---

Github user arunmahadevan commented on the issue:

https://github.com/apache/storm/pull/1528
  
@HeartSaVioR The changes looks good. May be you can add some comments in 
sync-process and mk-synchronize-supervisor on what each is supposed to do.

In your comment where you rebalanced from 2 -> 3 workers, the allocated 
status for the new worker ":port 6703" is also disallowed, which looks odd 
since its a new worker.


> Race condition between sync-supervisor and sync-processes raises several 
> strange issues
> ---
>
> Key: STORM-1934
> URL: https://issues.apache.org/jira/browse/STORM-1934
> Project: Apache Storm
>  Issue Type: Bug
>  Components: storm-core
>Affects Versions: 1.0.0, 2.0.0, 1.0.1
>Reporter: Jungtaek Lim
>Assignee: Jungtaek Lim
>Priority: Critical
>
> There're some strange issues including STORM-1933 and others (which I will 
> file an issue soon) which are related to race condition in supervisor.
> As I mentioned to STORM-1933, basically sync-supervisor relies on zk 
> assignment, and sync-processes relies on local assignment and local workers 
> directory, but in fact sync-supervisor also access local state and take some 
> actions which affects sync-processes. And also Satish left the comment to 
> STORM-1933 describing other issue related to race condition and idea to fix 
> this which is same page on me.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[GitHub] storm issue #1528: STORM-1934 Fix race condition between sync-supervisor and...

2016-06-29 Thread arunmahadevan
Github user arunmahadevan commented on the issue:

https://github.com/apache/storm/pull/1528
  
@HeartSaVioR The changes looks good. May be you can add some comments in 
sync-process and mk-synchronize-supervisor on what each is supposed to do.

In your comment where you rebalanced from 2 -> 3 workers, the allocated 
status for the new worker ":port 6703" is also disallowed, which looks odd 
since its a new worker.


---
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 #1529: STORM-1934 Fix race condition between sync-supervi...

2016-06-29 Thread HeartSaVioR
GitHub user HeartSaVioR opened a pull request:

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

STORM-1934 Fix race condition between sync-supervisor and sync-processes

* sync-supervisor just downloads new topology code and writes new local 
assignment
  * shutting down workers and removing topology code is moved to 
sync-processes
* sync-processes does all of jobs based on local assignment and allocated 
workers

PR for 1.x-branch: #1528

Please note that supervisor is ported to Java, so PR for master and PR for 
1.x-branch are having different diff. But concept is same.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/HeartSaVioR/storm STORM-1934

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/storm/pull/1529.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 #1529


commit 816a6cc47e5be505d4857e25a4aa14d2f09cade0
Author: Jungtaek Lim 
Date:   2016-06-29T10:06:43Z

STORM-1934 Fix race condition between sync-supervisor and sync-processes

* sync-supervisor just downloads new topology code and writes new local 
assignment
  * shutting down workers and removing topology code is moved to 
sync-processes
* sync-processes does all of jobs based on local assignment and allocated 
workers




---
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.
---


[jira] [Commented] (STORM-1934) Race condition between sync-supervisor and sync-processes raises several strange issues

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1934?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15354980#comment-15354980
 ] 

ASF GitHub Bot commented on STORM-1934:
---

GitHub user HeartSaVioR opened a pull request:

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

STORM-1934 Fix race condition between sync-supervisor and sync-processes

* sync-supervisor just downloads new topology code and writes new local 
assignment
  * shutting down workers and removing topology code is moved to 
sync-processes
* sync-processes does all of jobs based on local assignment and allocated 
workers

PR for 1.x-branch: #1528

Please note that supervisor is ported to Java, so PR for master and PR for 
1.x-branch are having different diff. But concept is same.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/HeartSaVioR/storm STORM-1934

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/storm/pull/1529.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 #1529


commit 816a6cc47e5be505d4857e25a4aa14d2f09cade0
Author: Jungtaek Lim 
Date:   2016-06-29T10:06:43Z

STORM-1934 Fix race condition between sync-supervisor and sync-processes

* sync-supervisor just downloads new topology code and writes new local 
assignment
  * shutting down workers and removing topology code is moved to 
sync-processes
* sync-processes does all of jobs based on local assignment and allocated 
workers




> Race condition between sync-supervisor and sync-processes raises several 
> strange issues
> ---
>
> Key: STORM-1934
> URL: https://issues.apache.org/jira/browse/STORM-1934
> Project: Apache Storm
>  Issue Type: Bug
>  Components: storm-core
>Affects Versions: 1.0.0, 2.0.0, 1.0.1
>Reporter: Jungtaek Lim
>Assignee: Jungtaek Lim
>Priority: Critical
>
> There're some strange issues including STORM-1933 and others (which I will 
> file an issue soon) which are related to race condition in supervisor.
> As I mentioned to STORM-1933, basically sync-supervisor relies on zk 
> assignment, and sync-processes relies on local assignment and local workers 
> directory, but in fact sync-supervisor also access local state and take some 
> actions which affects sync-processes. And also Satish left the comment to 
> STORM-1933 describing other issue related to race condition and idea to fix 
> this which is same page on me.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[GitHub] storm issue #1528: STORM-1934 Fix race condition between sync-supervisor and...

2016-06-29 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue:

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

https://github.com/apache/storm/commit/aad9669643d5ba8552a2d5b62982c8f31d6471cd
I don't find clear usage from introduction of StandaloneSupervisor, so I 
guess it would be better to ask or wait someone who knows this ancient thing. :)


---
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.
---


[jira] [Commented] (STORM-1919) Introduce FilterBolt on storm-redis

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1919?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15354905#comment-15354905
 ] 

ASF GitHub Bot commented on STORM-1919:
---

Github user abhishekagarwal87 commented on a diff in the pull request:

https://github.com/apache/storm/pull/1517#discussion_r68917959
  
--- Diff: 
external/storm-redis/src/test/java/org/apache/storm/redis/topology/WhitelistWordCount.java
 ---
@@ -0,0 +1,155 @@
+/**
+ * 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.topology;
+
+import org.apache.storm.Config;
+import org.apache.storm.LocalCluster;
+import org.apache.storm.StormSubmitter;
+import org.apache.storm.redis.bolt.RedisFilterBolt;
+import org.apache.storm.redis.common.config.JedisPoolConfig;
+import org.apache.storm.redis.common.mapper.RedisDataTypeDescription;
+import org.apache.storm.redis.common.mapper.RedisFilterMapper;
+import org.apache.storm.task.OutputCollector;
+import org.apache.storm.task.TopologyContext;
+import org.apache.storm.topology.OutputFieldsDeclarer;
+import org.apache.storm.topology.TopologyBuilder;
+import org.apache.storm.topology.base.BaseRichBolt;
+import org.apache.storm.tuple.Fields;
+import org.apache.storm.tuple.ITuple;
+import org.apache.storm.tuple.Tuple;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Map;
+import java.util.Random;
+
+public class WhitelistWordCount {
--- End diff --

Let's have this in examples instead since there is no test in the class. 


> Introduce FilterBolt on storm-redis
> ---
>
> Key: STORM-1919
> URL: https://issues.apache.org/jira/browse/STORM-1919
> Project: Apache Storm
>  Issue Type: New Feature
>  Components: storm-redis
>Reporter: Jungtaek Lim
>Assignee: Jungtaek Lim
>
> While discussing about STORM-1880, it would be better to have FilterBolt 
> explicitly instead of letting users set up their lookup mapper to act as 
> filter.
> There's other benefit here: we can use exists / hexists on STRING / HASH 
> datatype instead of retrieving actual value which reduces execution time / 
> latency from Redis side.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Commented] (STORM-1919) Introduce FilterBolt on storm-redis

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1919?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15354903#comment-15354903
 ] 

ASF GitHub Bot commented on STORM-1919:
---

Github user HeartSaVioR commented on a diff in the pull request:

https://github.com/apache/storm/pull/1517#discussion_r68917610
  
--- Diff: 
external/storm-redis/src/main/java/org/apache/storm/redis/bolt/RedisFilterBolt.java
 ---
@@ -0,0 +1,146 @@
+/**
+ * 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.bolt;
+
+import org.apache.storm.redis.common.config.JedisClusterConfig;
+import org.apache.storm.redis.common.config.JedisPoolConfig;
+import org.apache.storm.redis.common.mapper.RedisDataTypeDescription;
+import org.apache.storm.redis.common.mapper.RedisFilterMapper;
+import org.apache.storm.topology.OutputFieldsDeclarer;
+import org.apache.storm.tuple.Tuple;
+import redis.clients.jedis.GeoCoordinate;
+import redis.clients.jedis.JedisCommands;
+
+import java.util.List;
+
+/**
+ * Basic bolt for querying from Redis and filters out if key/field doesn't 
exist.
+ * If key/field exists on Redis, this bolt just forwards input tuple to 
default stream.
+ * 
+ * Supported data types: STRING, HASH, SET, SORTED_SET, HYPER_LOG_LOG, GEO.
+ * 
+ * Note: For STRING it checks such key exists on the key space.
+ * For HASH and SORTED_SET and GEO, it checks such field exists on that 
data structure.
+ * For SET and HYPER_LOG_LOG, it check such value exists on that data 
structure.
+ * (Note that it still refers key from tuple via 
RedisFilterMapper#getKeyFromTuple())
+ * In order to apply checking this to SET, you need to input additional 
key this case.
+ * 
+ * Note2: If you want to just query about existence of key regardless of 
actual data type,
+ * specify STRING to data type of RedisFilterMapper.
+ */
+public class RedisFilterBolt extends AbstractRedisBolt {
+private final RedisFilterMapper filterMapper;
+private final RedisDataTypeDescription.RedisDataType dataType;
+private final String additionalKey;
+
+/**
+ * Constructor for single Redis environment (JedisPool)
+ * @param config configuration for initializing JedisPool
+ * @param filterMapper mapper containing which datatype, query key 
that Bolt uses
+ */
+public RedisFilterBolt(JedisPoolConfig config, RedisFilterMapper 
filterMapper) {
+super(config);
+
+this.filterMapper = filterMapper;
+
+RedisDataTypeDescription dataTypeDescription = 
filterMapper.getDataTypeDescription();
+this.dataType = dataTypeDescription.getDataType();
+this.additionalKey = dataTypeDescription.getAdditionalKey();
+}
+
+/**
+ * Constructor for Redis Cluster environment (JedisCluster)
+ * @param config configuration for initializing JedisCluster
+ * @param filterMapper mapper containing which datatype, query key 
that Bolt uses
+ */
+public RedisFilterBolt(JedisClusterConfig config, RedisFilterMapper 
filterMapper) {
+super(config);
+
+this.filterMapper = filterMapper;
+
+RedisDataTypeDescription dataTypeDescription = 
filterMapper.getDataTypeDescription();
+this.dataType = dataTypeDescription.getDataType();
+this.additionalKey = dataTypeDescription.getAdditionalKey();
+}
+
+/**
+ * {@inheritDoc}
+ */
+@Override
+public void execute(Tuple input) {
+String key = filterMapper.getKeyFromTuple(input);
+
+boolean found;
+JedisCommands jedisCommand = null;
+try {
+jedisCommand = getInstance();
+
+switch (dataType) {
+case STRING:
+found = jedisCommand.exists(key);
+break;
+
+

[jira] [Commented] (STORM-1919) Introduce FilterBolt on storm-redis

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1919?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15354904#comment-15354904
 ] 

ASF GitHub Bot commented on STORM-1919:
---

Github user HeartSaVioR commented on a diff in the pull request:

https://github.com/apache/storm/pull/1517#discussion_r68917641
  
--- Diff: 
external/storm-redis/src/main/java/org/apache/storm/redis/bolt/RedisFilterBolt.java
 ---
@@ -0,0 +1,146 @@
+/**
+ * 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.bolt;
+
+import org.apache.storm.redis.common.config.JedisClusterConfig;
+import org.apache.storm.redis.common.config.JedisPoolConfig;
+import org.apache.storm.redis.common.mapper.RedisDataTypeDescription;
+import org.apache.storm.redis.common.mapper.RedisFilterMapper;
+import org.apache.storm.topology.OutputFieldsDeclarer;
+import org.apache.storm.tuple.Tuple;
+import redis.clients.jedis.GeoCoordinate;
+import redis.clients.jedis.JedisCommands;
+
+import java.util.List;
+
+/**
+ * Basic bolt for querying from Redis and filters out if key/field doesn't 
exist.
+ * If key/field exists on Redis, this bolt just forwards input tuple to 
default stream.
+ * 
+ * Supported data types: STRING, HASH, SET, SORTED_SET, HYPER_LOG_LOG, GEO.
+ * 
+ * Note: For STRING it checks such key exists on the key space.
+ * For HASH and SORTED_SET and GEO, it checks such field exists on that 
data structure.
+ * For SET and HYPER_LOG_LOG, it check such value exists on that data 
structure.
+ * (Note that it still refers key from tuple via 
RedisFilterMapper#getKeyFromTuple())
+ * In order to apply checking this to SET, you need to input additional 
key this case.
+ * 
+ * Note2: If you want to just query about existence of key regardless of 
actual data type,
+ * specify STRING to data type of RedisFilterMapper.
+ */
+public class RedisFilterBolt extends AbstractRedisBolt {
+private final RedisFilterMapper filterMapper;
+private final RedisDataTypeDescription.RedisDataType dataType;
+private final String additionalKey;
+
+/**
+ * Constructor for single Redis environment (JedisPool)
+ * @param config configuration for initializing JedisPool
+ * @param filterMapper mapper containing which datatype, query key 
that Bolt uses
+ */
+public RedisFilterBolt(JedisPoolConfig config, RedisFilterMapper 
filterMapper) {
+super(config);
+
+this.filterMapper = filterMapper;
+
+RedisDataTypeDescription dataTypeDescription = 
filterMapper.getDataTypeDescription();
+this.dataType = dataTypeDescription.getDataType();
+this.additionalKey = dataTypeDescription.getAdditionalKey();
+}
+
+/**
+ * Constructor for Redis Cluster environment (JedisCluster)
+ * @param config configuration for initializing JedisCluster
+ * @param filterMapper mapper containing which datatype, query key 
that Bolt uses
+ */
+public RedisFilterBolt(JedisClusterConfig config, RedisFilterMapper 
filterMapper) {
+super(config);
+
+this.filterMapper = filterMapper;
+
+RedisDataTypeDescription dataTypeDescription = 
filterMapper.getDataTypeDescription();
+this.dataType = dataTypeDescription.getDataType();
+this.additionalKey = dataTypeDescription.getAdditionalKey();
+}
+
+/**
+ * {@inheritDoc}
+ */
+@Override
+public void execute(Tuple input) {
+String key = filterMapper.getKeyFromTuple(input);
+
+boolean found;
+JedisCommands jedisCommand = null;
+try {
+jedisCommand = getInstance();
+
+switch (dataType) {
+case STRING:
+found = jedisCommand.exists(key);
+break;
+
+

[GitHub] storm pull request #1517: STORM-1919 Introduce FilterBolt on storm-redis

2016-06-29 Thread HeartSaVioR
Github user HeartSaVioR commented on a diff in the pull request:

https://github.com/apache/storm/pull/1517#discussion_r68917610
  
--- Diff: 
external/storm-redis/src/main/java/org/apache/storm/redis/bolt/RedisFilterBolt.java
 ---
@@ -0,0 +1,146 @@
+/**
+ * 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.bolt;
+
+import org.apache.storm.redis.common.config.JedisClusterConfig;
+import org.apache.storm.redis.common.config.JedisPoolConfig;
+import org.apache.storm.redis.common.mapper.RedisDataTypeDescription;
+import org.apache.storm.redis.common.mapper.RedisFilterMapper;
+import org.apache.storm.topology.OutputFieldsDeclarer;
+import org.apache.storm.tuple.Tuple;
+import redis.clients.jedis.GeoCoordinate;
+import redis.clients.jedis.JedisCommands;
+
+import java.util.List;
+
+/**
+ * Basic bolt for querying from Redis and filters out if key/field doesn't 
exist.
+ * If key/field exists on Redis, this bolt just forwards input tuple to 
default stream.
+ * 
+ * Supported data types: STRING, HASH, SET, SORTED_SET, HYPER_LOG_LOG, GEO.
+ * 
+ * Note: For STRING it checks such key exists on the key space.
+ * For HASH and SORTED_SET and GEO, it checks such field exists on that 
data structure.
+ * For SET and HYPER_LOG_LOG, it check such value exists on that data 
structure.
+ * (Note that it still refers key from tuple via 
RedisFilterMapper#getKeyFromTuple())
+ * In order to apply checking this to SET, you need to input additional 
key this case.
+ * 
+ * Note2: If you want to just query about existence of key regardless of 
actual data type,
+ * specify STRING to data type of RedisFilterMapper.
+ */
+public class RedisFilterBolt extends AbstractRedisBolt {
+private final RedisFilterMapper filterMapper;
+private final RedisDataTypeDescription.RedisDataType dataType;
+private final String additionalKey;
+
+/**
+ * Constructor for single Redis environment (JedisPool)
+ * @param config configuration for initializing JedisPool
+ * @param filterMapper mapper containing which datatype, query key 
that Bolt uses
+ */
+public RedisFilterBolt(JedisPoolConfig config, RedisFilterMapper 
filterMapper) {
+super(config);
+
+this.filterMapper = filterMapper;
+
+RedisDataTypeDescription dataTypeDescription = 
filterMapper.getDataTypeDescription();
+this.dataType = dataTypeDescription.getDataType();
+this.additionalKey = dataTypeDescription.getAdditionalKey();
+}
+
+/**
+ * Constructor for Redis Cluster environment (JedisCluster)
+ * @param config configuration for initializing JedisCluster
+ * @param filterMapper mapper containing which datatype, query key 
that Bolt uses
+ */
+public RedisFilterBolt(JedisClusterConfig config, RedisFilterMapper 
filterMapper) {
+super(config);
+
+this.filterMapper = filterMapper;
+
+RedisDataTypeDescription dataTypeDescription = 
filterMapper.getDataTypeDescription();
+this.dataType = dataTypeDescription.getDataType();
+this.additionalKey = dataTypeDescription.getAdditionalKey();
+}
+
+/**
+ * {@inheritDoc}
+ */
+@Override
+public void execute(Tuple input) {
+String key = filterMapper.getKeyFromTuple(input);
+
+boolean found;
+JedisCommands jedisCommand = null;
+try {
+jedisCommand = getInstance();
+
+switch (dataType) {
+case STRING:
+found = jedisCommand.exists(key);
+break;
+
+case SET:
+if (additionalKey == null) {
--- End diff --

@abhishekagarwal87 Great point! Will fix.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 

[GitHub] storm pull request #1517: STORM-1919 Introduce FilterBolt on storm-redis

2016-06-29 Thread abhishekagarwal87
Github user abhishekagarwal87 commented on a diff in the pull request:

https://github.com/apache/storm/pull/1517#discussion_r68917959
  
--- Diff: 
external/storm-redis/src/test/java/org/apache/storm/redis/topology/WhitelistWordCount.java
 ---
@@ -0,0 +1,155 @@
+/**
+ * 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.topology;
+
+import org.apache.storm.Config;
+import org.apache.storm.LocalCluster;
+import org.apache.storm.StormSubmitter;
+import org.apache.storm.redis.bolt.RedisFilterBolt;
+import org.apache.storm.redis.common.config.JedisPoolConfig;
+import org.apache.storm.redis.common.mapper.RedisDataTypeDescription;
+import org.apache.storm.redis.common.mapper.RedisFilterMapper;
+import org.apache.storm.task.OutputCollector;
+import org.apache.storm.task.TopologyContext;
+import org.apache.storm.topology.OutputFieldsDeclarer;
+import org.apache.storm.topology.TopologyBuilder;
+import org.apache.storm.topology.base.BaseRichBolt;
+import org.apache.storm.tuple.Fields;
+import org.apache.storm.tuple.ITuple;
+import org.apache.storm.tuple.Tuple;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Map;
+import java.util.Random;
+
+public class WhitelistWordCount {
--- End diff --

Let's have this in examples instead since there is no test in the 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] storm pull request #1517: STORM-1919 Introduce FilterBolt on storm-redis

2016-06-29 Thread HeartSaVioR
Github user HeartSaVioR commented on a diff in the pull request:

https://github.com/apache/storm/pull/1517#discussion_r68917641
  
--- Diff: 
external/storm-redis/src/main/java/org/apache/storm/redis/bolt/RedisFilterBolt.java
 ---
@@ -0,0 +1,146 @@
+/**
+ * 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.bolt;
+
+import org.apache.storm.redis.common.config.JedisClusterConfig;
+import org.apache.storm.redis.common.config.JedisPoolConfig;
+import org.apache.storm.redis.common.mapper.RedisDataTypeDescription;
+import org.apache.storm.redis.common.mapper.RedisFilterMapper;
+import org.apache.storm.topology.OutputFieldsDeclarer;
+import org.apache.storm.tuple.Tuple;
+import redis.clients.jedis.GeoCoordinate;
+import redis.clients.jedis.JedisCommands;
+
+import java.util.List;
+
+/**
+ * Basic bolt for querying from Redis and filters out if key/field doesn't 
exist.
+ * If key/field exists on Redis, this bolt just forwards input tuple to 
default stream.
+ * 
+ * Supported data types: STRING, HASH, SET, SORTED_SET, HYPER_LOG_LOG, GEO.
+ * 
+ * Note: For STRING it checks such key exists on the key space.
+ * For HASH and SORTED_SET and GEO, it checks such field exists on that 
data structure.
+ * For SET and HYPER_LOG_LOG, it check such value exists on that data 
structure.
+ * (Note that it still refers key from tuple via 
RedisFilterMapper#getKeyFromTuple())
+ * In order to apply checking this to SET, you need to input additional 
key this case.
+ * 
+ * Note2: If you want to just query about existence of key regardless of 
actual data type,
+ * specify STRING to data type of RedisFilterMapper.
+ */
+public class RedisFilterBolt extends AbstractRedisBolt {
+private final RedisFilterMapper filterMapper;
+private final RedisDataTypeDescription.RedisDataType dataType;
+private final String additionalKey;
+
+/**
+ * Constructor for single Redis environment (JedisPool)
+ * @param config configuration for initializing JedisPool
+ * @param filterMapper mapper containing which datatype, query key 
that Bolt uses
+ */
+public RedisFilterBolt(JedisPoolConfig config, RedisFilterMapper 
filterMapper) {
+super(config);
+
+this.filterMapper = filterMapper;
+
+RedisDataTypeDescription dataTypeDescription = 
filterMapper.getDataTypeDescription();
+this.dataType = dataTypeDescription.getDataType();
+this.additionalKey = dataTypeDescription.getAdditionalKey();
+}
+
+/**
+ * Constructor for Redis Cluster environment (JedisCluster)
+ * @param config configuration for initializing JedisCluster
+ * @param filterMapper mapper containing which datatype, query key 
that Bolt uses
+ */
+public RedisFilterBolt(JedisClusterConfig config, RedisFilterMapper 
filterMapper) {
+super(config);
+
+this.filterMapper = filterMapper;
+
+RedisDataTypeDescription dataTypeDescription = 
filterMapper.getDataTypeDescription();
+this.dataType = dataTypeDescription.getDataType();
+this.additionalKey = dataTypeDescription.getAdditionalKey();
+}
+
+/**
+ * {@inheritDoc}
+ */
+@Override
+public void execute(Tuple input) {
+String key = filterMapper.getKeyFromTuple(input);
+
+boolean found;
+JedisCommands jedisCommand = null;
+try {
+jedisCommand = getInstance();
+
+switch (dataType) {
+case STRING:
+found = jedisCommand.exists(key);
+break;
+
+case SET:
+if (additionalKey == null) {
+throw new IllegalArgumentException("additionalKey 
should be defined");
+}
+found = jedisCommand.sismember(addi

[jira] [Commented] (STORM-1919) Introduce FilterBolt on storm-redis

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1919?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15354901#comment-15354901
 ] 

ASF GitHub Bot commented on STORM-1919:
---

Github user abhishekagarwal87 commented on a diff in the pull request:

https://github.com/apache/storm/pull/1517#discussion_r68916823
  
--- Diff: 
external/storm-redis/src/main/java/org/apache/storm/redis/bolt/RedisFilterBolt.java
 ---
@@ -0,0 +1,146 @@
+/**
+ * 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.bolt;
+
+import org.apache.storm.redis.common.config.JedisClusterConfig;
+import org.apache.storm.redis.common.config.JedisPoolConfig;
+import org.apache.storm.redis.common.mapper.RedisDataTypeDescription;
+import org.apache.storm.redis.common.mapper.RedisFilterMapper;
+import org.apache.storm.topology.OutputFieldsDeclarer;
+import org.apache.storm.tuple.Tuple;
+import redis.clients.jedis.GeoCoordinate;
+import redis.clients.jedis.JedisCommands;
+
+import java.util.List;
+
+/**
+ * Basic bolt for querying from Redis and filters out if key/field doesn't 
exist.
+ * If key/field exists on Redis, this bolt just forwards input tuple to 
default stream.
+ * 
+ * Supported data types: STRING, HASH, SET, SORTED_SET, HYPER_LOG_LOG, GEO.
+ * 
+ * Note: For STRING it checks such key exists on the key space.
+ * For HASH and SORTED_SET and GEO, it checks such field exists on that 
data structure.
+ * For SET and HYPER_LOG_LOG, it check such value exists on that data 
structure.
+ * (Note that it still refers key from tuple via 
RedisFilterMapper#getKeyFromTuple())
+ * In order to apply checking this to SET, you need to input additional 
key this case.
+ * 
+ * Note2: If you want to just query about existence of key regardless of 
actual data type,
+ * specify STRING to data type of RedisFilterMapper.
+ */
+public class RedisFilterBolt extends AbstractRedisBolt {
+private final RedisFilterMapper filterMapper;
+private final RedisDataTypeDescription.RedisDataType dataType;
+private final String additionalKey;
+
+/**
+ * Constructor for single Redis environment (JedisPool)
+ * @param config configuration for initializing JedisPool
+ * @param filterMapper mapper containing which datatype, query key 
that Bolt uses
+ */
+public RedisFilterBolt(JedisPoolConfig config, RedisFilterMapper 
filterMapper) {
+super(config);
+
+this.filterMapper = filterMapper;
+
+RedisDataTypeDescription dataTypeDescription = 
filterMapper.getDataTypeDescription();
+this.dataType = dataTypeDescription.getDataType();
+this.additionalKey = dataTypeDescription.getAdditionalKey();
+}
+
+/**
+ * Constructor for Redis Cluster environment (JedisCluster)
+ * @param config configuration for initializing JedisCluster
+ * @param filterMapper mapper containing which datatype, query key 
that Bolt uses
+ */
+public RedisFilterBolt(JedisClusterConfig config, RedisFilterMapper 
filterMapper) {
+super(config);
+
+this.filterMapper = filterMapper;
+
+RedisDataTypeDescription dataTypeDescription = 
filterMapper.getDataTypeDescription();
+this.dataType = dataTypeDescription.getDataType();
+this.additionalKey = dataTypeDescription.getAdditionalKey();
+}
+
+/**
+ * {@inheritDoc}
+ */
+@Override
+public void execute(Tuple input) {
+String key = filterMapper.getKeyFromTuple(input);
+
+boolean found;
+JedisCommands jedisCommand = null;
+try {
+jedisCommand = getInstance();
+
+switch (dataType) {
+case STRING:
+found = jedisCommand.exists(key);
+break;
+
+  

[GitHub] storm pull request #1517: STORM-1919 Introduce FilterBolt on storm-redis

2016-06-29 Thread abhishekagarwal87
Github user abhishekagarwal87 commented on a diff in the pull request:

https://github.com/apache/storm/pull/1517#discussion_r68916823
  
--- Diff: 
external/storm-redis/src/main/java/org/apache/storm/redis/bolt/RedisFilterBolt.java
 ---
@@ -0,0 +1,146 @@
+/**
+ * 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.bolt;
+
+import org.apache.storm.redis.common.config.JedisClusterConfig;
+import org.apache.storm.redis.common.config.JedisPoolConfig;
+import org.apache.storm.redis.common.mapper.RedisDataTypeDescription;
+import org.apache.storm.redis.common.mapper.RedisFilterMapper;
+import org.apache.storm.topology.OutputFieldsDeclarer;
+import org.apache.storm.tuple.Tuple;
+import redis.clients.jedis.GeoCoordinate;
+import redis.clients.jedis.JedisCommands;
+
+import java.util.List;
+
+/**
+ * Basic bolt for querying from Redis and filters out if key/field doesn't 
exist.
+ * If key/field exists on Redis, this bolt just forwards input tuple to 
default stream.
+ * 
+ * Supported data types: STRING, HASH, SET, SORTED_SET, HYPER_LOG_LOG, GEO.
+ * 
+ * Note: For STRING it checks such key exists on the key space.
+ * For HASH and SORTED_SET and GEO, it checks such field exists on that 
data structure.
+ * For SET and HYPER_LOG_LOG, it check such value exists on that data 
structure.
+ * (Note that it still refers key from tuple via 
RedisFilterMapper#getKeyFromTuple())
+ * In order to apply checking this to SET, you need to input additional 
key this case.
+ * 
+ * Note2: If you want to just query about existence of key regardless of 
actual data type,
+ * specify STRING to data type of RedisFilterMapper.
+ */
+public class RedisFilterBolt extends AbstractRedisBolt {
+private final RedisFilterMapper filterMapper;
+private final RedisDataTypeDescription.RedisDataType dataType;
+private final String additionalKey;
+
+/**
+ * Constructor for single Redis environment (JedisPool)
+ * @param config configuration for initializing JedisPool
+ * @param filterMapper mapper containing which datatype, query key 
that Bolt uses
+ */
+public RedisFilterBolt(JedisPoolConfig config, RedisFilterMapper 
filterMapper) {
+super(config);
+
+this.filterMapper = filterMapper;
+
+RedisDataTypeDescription dataTypeDescription = 
filterMapper.getDataTypeDescription();
+this.dataType = dataTypeDescription.getDataType();
+this.additionalKey = dataTypeDescription.getAdditionalKey();
+}
+
+/**
+ * Constructor for Redis Cluster environment (JedisCluster)
+ * @param config configuration for initializing JedisCluster
+ * @param filterMapper mapper containing which datatype, query key 
that Bolt uses
+ */
+public RedisFilterBolt(JedisClusterConfig config, RedisFilterMapper 
filterMapper) {
+super(config);
+
+this.filterMapper = filterMapper;
+
+RedisDataTypeDescription dataTypeDescription = 
filterMapper.getDataTypeDescription();
+this.dataType = dataTypeDescription.getDataType();
+this.additionalKey = dataTypeDescription.getAdditionalKey();
+}
+
+/**
+ * {@inheritDoc}
+ */
+@Override
+public void execute(Tuple input) {
+String key = filterMapper.getKeyFromTuple(input);
+
+boolean found;
+JedisCommands jedisCommand = null;
+try {
+jedisCommand = getInstance();
+
+switch (dataType) {
+case STRING:
+found = jedisCommand.exists(key);
+break;
+
+case SET:
+if (additionalKey == null) {
+throw new IllegalArgumentException("additionalKey 
should be defined");
+}
+found = jedisCommand.sismembe

[jira] [Commented] (STORM-1919) Introduce FilterBolt on storm-redis

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1919?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15354894#comment-15354894
 ] 

ASF GitHub Bot commented on STORM-1919:
---

Github user abhishekagarwal87 commented on a diff in the pull request:

https://github.com/apache/storm/pull/1517#discussion_r68916461
  
--- Diff: 
external/storm-redis/src/main/java/org/apache/storm/redis/bolt/RedisFilterBolt.java
 ---
@@ -0,0 +1,146 @@
+/**
+ * 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.bolt;
+
+import org.apache.storm.redis.common.config.JedisClusterConfig;
+import org.apache.storm.redis.common.config.JedisPoolConfig;
+import org.apache.storm.redis.common.mapper.RedisDataTypeDescription;
+import org.apache.storm.redis.common.mapper.RedisFilterMapper;
+import org.apache.storm.topology.OutputFieldsDeclarer;
+import org.apache.storm.tuple.Tuple;
+import redis.clients.jedis.GeoCoordinate;
+import redis.clients.jedis.JedisCommands;
+
+import java.util.List;
+
+/**
+ * Basic bolt for querying from Redis and filters out if key/field doesn't 
exist.
+ * If key/field exists on Redis, this bolt just forwards input tuple to 
default stream.
+ * 
+ * Supported data types: STRING, HASH, SET, SORTED_SET, HYPER_LOG_LOG, GEO.
+ * 
+ * Note: For STRING it checks such key exists on the key space.
+ * For HASH and SORTED_SET and GEO, it checks such field exists on that 
data structure.
+ * For SET and HYPER_LOG_LOG, it check such value exists on that data 
structure.
+ * (Note that it still refers key from tuple via 
RedisFilterMapper#getKeyFromTuple())
+ * In order to apply checking this to SET, you need to input additional 
key this case.
+ * 
+ * Note2: If you want to just query about existence of key regardless of 
actual data type,
+ * specify STRING to data type of RedisFilterMapper.
+ */
+public class RedisFilterBolt extends AbstractRedisBolt {
+private final RedisFilterMapper filterMapper;
+private final RedisDataTypeDescription.RedisDataType dataType;
+private final String additionalKey;
+
+/**
+ * Constructor for single Redis environment (JedisPool)
+ * @param config configuration for initializing JedisPool
+ * @param filterMapper mapper containing which datatype, query key 
that Bolt uses
+ */
+public RedisFilterBolt(JedisPoolConfig config, RedisFilterMapper 
filterMapper) {
+super(config);
+
+this.filterMapper = filterMapper;
+
+RedisDataTypeDescription dataTypeDescription = 
filterMapper.getDataTypeDescription();
+this.dataType = dataTypeDescription.getDataType();
+this.additionalKey = dataTypeDescription.getAdditionalKey();
+}
+
+/**
+ * Constructor for Redis Cluster environment (JedisCluster)
+ * @param config configuration for initializing JedisCluster
+ * @param filterMapper mapper containing which datatype, query key 
that Bolt uses
+ */
+public RedisFilterBolt(JedisClusterConfig config, RedisFilterMapper 
filterMapper) {
+super(config);
+
+this.filterMapper = filterMapper;
+
+RedisDataTypeDescription dataTypeDescription = 
filterMapper.getDataTypeDescription();
+this.dataType = dataTypeDescription.getDataType();
+this.additionalKey = dataTypeDescription.getAdditionalKey();
+}
+
+/**
+ * {@inheritDoc}
+ */
+@Override
+public void execute(Tuple input) {
+String key = filterMapper.getKeyFromTuple(input);
+
+boolean found;
+JedisCommands jedisCommand = null;
+try {
+jedisCommand = getInstance();
+
+switch (dataType) {
+case STRING:
+found = jedisCommand.exists(key);
+break;
+
+  

[GitHub] storm pull request #1517: STORM-1919 Introduce FilterBolt on storm-redis

2016-06-29 Thread abhishekagarwal87
Github user abhishekagarwal87 commented on a diff in the pull request:

https://github.com/apache/storm/pull/1517#discussion_r68916461
  
--- Diff: 
external/storm-redis/src/main/java/org/apache/storm/redis/bolt/RedisFilterBolt.java
 ---
@@ -0,0 +1,146 @@
+/**
+ * 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.bolt;
+
+import org.apache.storm.redis.common.config.JedisClusterConfig;
+import org.apache.storm.redis.common.config.JedisPoolConfig;
+import org.apache.storm.redis.common.mapper.RedisDataTypeDescription;
+import org.apache.storm.redis.common.mapper.RedisFilterMapper;
+import org.apache.storm.topology.OutputFieldsDeclarer;
+import org.apache.storm.tuple.Tuple;
+import redis.clients.jedis.GeoCoordinate;
+import redis.clients.jedis.JedisCommands;
+
+import java.util.List;
+
+/**
+ * Basic bolt for querying from Redis and filters out if key/field doesn't 
exist.
+ * If key/field exists on Redis, this bolt just forwards input tuple to 
default stream.
+ * 
+ * Supported data types: STRING, HASH, SET, SORTED_SET, HYPER_LOG_LOG, GEO.
+ * 
+ * Note: For STRING it checks such key exists on the key space.
+ * For HASH and SORTED_SET and GEO, it checks such field exists on that 
data structure.
+ * For SET and HYPER_LOG_LOG, it check such value exists on that data 
structure.
+ * (Note that it still refers key from tuple via 
RedisFilterMapper#getKeyFromTuple())
+ * In order to apply checking this to SET, you need to input additional 
key this case.
+ * 
+ * Note2: If you want to just query about existence of key regardless of 
actual data type,
+ * specify STRING to data type of RedisFilterMapper.
+ */
+public class RedisFilterBolt extends AbstractRedisBolt {
+private final RedisFilterMapper filterMapper;
+private final RedisDataTypeDescription.RedisDataType dataType;
+private final String additionalKey;
+
+/**
+ * Constructor for single Redis environment (JedisPool)
+ * @param config configuration for initializing JedisPool
+ * @param filterMapper mapper containing which datatype, query key 
that Bolt uses
+ */
+public RedisFilterBolt(JedisPoolConfig config, RedisFilterMapper 
filterMapper) {
+super(config);
+
+this.filterMapper = filterMapper;
+
+RedisDataTypeDescription dataTypeDescription = 
filterMapper.getDataTypeDescription();
+this.dataType = dataTypeDescription.getDataType();
+this.additionalKey = dataTypeDescription.getAdditionalKey();
+}
+
+/**
+ * Constructor for Redis Cluster environment (JedisCluster)
+ * @param config configuration for initializing JedisCluster
+ * @param filterMapper mapper containing which datatype, query key 
that Bolt uses
+ */
+public RedisFilterBolt(JedisClusterConfig config, RedisFilterMapper 
filterMapper) {
+super(config);
+
+this.filterMapper = filterMapper;
+
+RedisDataTypeDescription dataTypeDescription = 
filterMapper.getDataTypeDescription();
+this.dataType = dataTypeDescription.getDataType();
+this.additionalKey = dataTypeDescription.getAdditionalKey();
+}
+
+/**
+ * {@inheritDoc}
+ */
+@Override
+public void execute(Tuple input) {
+String key = filterMapper.getKeyFromTuple(input);
+
+boolean found;
+JedisCommands jedisCommand = null;
+try {
+jedisCommand = getInstance();
+
+switch (dataType) {
+case STRING:
+found = jedisCommand.exists(key);
+break;
+
+case SET:
+if (additionalKey == null) {
--- End diff --

since data type is known at the time of construction, can this check be 
moved to constructor itself? Failing fast is preferred. 


---
If your project i

[jira] [Commented] (STORM-1934) Race condition between sync-supervisor and sync-processes raises several strange issues

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1934?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15354877#comment-15354877
 ] 

ASF GitHub Bot commented on STORM-1934:
---

Github user satishd commented on a diff in the pull request:

https://github.com/apache/storm/pull/1528#discussion_r68914596
  
--- Diff: storm-core/src/clj/org/apache/storm/daemon/supervisor.clj ---
@@ -592,24 +577,13 @@
   (doseq [p (set/difference (set (keys existing-assignment))
 (set (keys new-assignment)))]
 (.killedWorker isupervisor (int p)))
--- End diff --

 `StandaloneSupervisor` implementation does nothing. What is the need of 
this API?


> Race condition between sync-supervisor and sync-processes raises several 
> strange issues
> ---
>
> Key: STORM-1934
> URL: https://issues.apache.org/jira/browse/STORM-1934
> Project: Apache Storm
>  Issue Type: Bug
>  Components: storm-core
>Affects Versions: 1.0.0, 2.0.0, 1.0.1
>Reporter: Jungtaek Lim
>Assignee: Jungtaek Lim
>Priority: Critical
>
> There're some strange issues including STORM-1933 and others (which I will 
> file an issue soon) which are related to race condition in supervisor.
> As I mentioned to STORM-1933, basically sync-supervisor relies on zk 
> assignment, and sync-processes relies on local assignment and local workers 
> directory, but in fact sync-supervisor also access local state and take some 
> actions which affects sync-processes. And also Satish left the comment to 
> STORM-1933 describing other issue related to race condition and idea to fix 
> this which is same page on me.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Commented] (STORM-1934) Race condition between sync-supervisor and sync-processes raises several strange issues

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1934?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15354878#comment-15354878
 ] 

ASF GitHub Bot commented on STORM-1934:
---

Github user satishd commented on a diff in the pull request:

https://github.com/apache/storm/pull/1528#discussion_r68914602
  
--- Diff: storm-core/src/clj/org/apache/storm/daemon/supervisor.clj ---
@@ -592,24 +577,13 @@
   (doseq [p (set/difference (set (keys existing-assignment))
 (set (keys new-assignment)))]
 (.killedWorker isupervisor (int p)))
-  (kill-existing-workers-with-change-in-components supervisor 
existing-assignment new-assignment)
   (.assigned isupervisor (keys new-assignment))
--- End diff --

 `StandaloneSupervisor` implementation does nothing. What is the need of 
this API?


> Race condition between sync-supervisor and sync-processes raises several 
> strange issues
> ---
>
> Key: STORM-1934
> URL: https://issues.apache.org/jira/browse/STORM-1934
> Project: Apache Storm
>  Issue Type: Bug
>  Components: storm-core
>Affects Versions: 1.0.0, 2.0.0, 1.0.1
>Reporter: Jungtaek Lim
>Assignee: Jungtaek Lim
>Priority: Critical
>
> There're some strange issues including STORM-1933 and others (which I will 
> file an issue soon) which are related to race condition in supervisor.
> As I mentioned to STORM-1933, basically sync-supervisor relies on zk 
> assignment, and sync-processes relies on local assignment and local workers 
> directory, but in fact sync-supervisor also access local state and take some 
> actions which affects sync-processes. And also Satish left the comment to 
> STORM-1933 describing other issue related to race condition and idea to fix 
> this which is same page on me.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[GitHub] storm pull request #1528: STORM-1934 Fix race condition between sync-supervi...

2016-06-29 Thread satishd
Github user satishd commented on a diff in the pull request:

https://github.com/apache/storm/pull/1528#discussion_r68914596
  
--- Diff: storm-core/src/clj/org/apache/storm/daemon/supervisor.clj ---
@@ -592,24 +577,13 @@
   (doseq [p (set/difference (set (keys existing-assignment))
 (set (keys new-assignment)))]
 (.killedWorker isupervisor (int p)))
--- End diff --

 `StandaloneSupervisor` implementation does nothing. What is the need of 
this API?


---
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 #1528: STORM-1934 Fix race condition between sync-supervi...

2016-06-29 Thread satishd
Github user satishd commented on a diff in the pull request:

https://github.com/apache/storm/pull/1528#discussion_r68914602
  
--- Diff: storm-core/src/clj/org/apache/storm/daemon/supervisor.clj ---
@@ -592,24 +577,13 @@
   (doseq [p (set/difference (set (keys existing-assignment))
 (set (keys new-assignment)))]
 (.killedWorker isupervisor (int p)))
-  (kill-existing-workers-with-change-in-components supervisor 
existing-assignment new-assignment)
   (.assigned isupervisor (keys new-assignment))
--- End diff --

 `StandaloneSupervisor` implementation does nothing. What is the need of 
this API?


---
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.
---


[jira] [Commented] (STORM-1934) Race condition between sync-supervisor and sync-processes raises several strange issues

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1934?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15354871#comment-15354871
 ] 

ASF GitHub Bot commented on STORM-1934:
---

Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/1528
  
Build passes on storm-core so it's unrelated.


> Race condition between sync-supervisor and sync-processes raises several 
> strange issues
> ---
>
> Key: STORM-1934
> URL: https://issues.apache.org/jira/browse/STORM-1934
> Project: Apache Storm
>  Issue Type: Bug
>  Components: storm-core
>Affects Versions: 1.0.0, 2.0.0, 1.0.1
>Reporter: Jungtaek Lim
>Assignee: Jungtaek Lim
>Priority: Critical
>
> There're some strange issues including STORM-1933 and others (which I will 
> file an issue soon) which are related to race condition in supervisor.
> As I mentioned to STORM-1933, basically sync-supervisor relies on zk 
> assignment, and sync-processes relies on local assignment and local workers 
> directory, but in fact sync-supervisor also access local state and take some 
> actions which affects sync-processes. And also Satish left the comment to 
> STORM-1933 describing other issue related to race condition and idea to fix 
> this which is same page on me.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[GitHub] storm issue #1528: STORM-1934 Fix race condition between sync-supervisor and...

2016-06-29 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/1528
  
Build passes on storm-core so it's unrelated.


---
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.
---


[jira] [Commented] (STORM-1933) Intermittent test failure on test-multiple-active-storms-multiple-supervisors for supervisor-test

2016-06-29 Thread Jungtaek Lim (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1933?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15354845#comment-15354845
 ] 

Jungtaek Lim commented on STORM-1933:
-

I think Nico describes the reason of this issue correctly. Linking comment: 
https://issues.apache.org/jira/browse/STORM-1879?focusedCommentId=15344582&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15344582

> Intermittent test failure on test-multiple-active-storms-multiple-supervisors 
> for supervisor-test 
> --
>
> Key: STORM-1933
> URL: https://issues.apache.org/jira/browse/STORM-1933
> Project: Apache Storm
>  Issue Type: Sub-task
>  Components: storm-core
>Affects Versions: 1.0.0, 2.0.0, 1.0.1
>Reporter: Jungtaek Lim
>Assignee: Jungtaek Lim
> Attachments: 
> only-thread-1362-and-1363-BUG-60850-intermittent-failure-supervisor-test.txt
>
>
> test-multiple-active-storms-multiple-supervisors is failing with fairly high 
> chance. I've run unit test of 1.x branch 3 times and met this issue, and 
> users report FileNotFound issue on supervisor which seems to be related to 
> this.
> I have log file so I'll attach once issue is created.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Commented] (STORM-1594) org.apache.storm.tuple.Fields can throw NPE if given invalid field in selector

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1594?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15354841#comment-15354841
 ] 

ASF GitHub Bot commented on STORM-1594:
---

Github user abhishekagarwal87 commented on the issue:

https://github.com/apache/storm/pull/1522
  
Thank you @lujinhong. I am +1 pending travis. Please squash the commits 
once travis is passed.


> org.apache.storm.tuple.Fields can throw NPE if given invalid field in selector
> --
>
> Key: STORM-1594
> URL: https://issues.apache.org/jira/browse/STORM-1594
> Project: Apache Storm
>  Issue Type: Bug
>  Components: storm-core
>Reporter: Alessandro Bellina
>Priority: Minor
>  Labels: npe
>
> In class org.apache.storm.tuple.Fields, the select method gets the index of 
> the field to select from the first argument (selector) by using .get. It 
> doesn't set this result to an Integer and check for null (Map.get returns 
> null if the key is not found).
> When tuple.get happens, the parameter for tuple.get is an unboxed integer. 
> The null cannot be unboxed to integer, causing an NPE. There is another 
> method in Fields called fieldIndex which will throw an 
> IllegalArgumentException in the case that the field in the selector isn't in 
> the _index. 
> {code}
> public List select(Fields selector, List tuple) {
> List ret = new ArrayList<>(selector.size());  
> for(String s: selector) {
> ret.add(tuple.get(_index.get(s)));
> }
> return ret;
> }
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Commented] (STORM-1594) org.apache.storm.tuple.Fields can throw NPE if given invalid field in selector

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1594?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15354835#comment-15354835
 ] 

ASF GitHub Bot commented on STORM-1594:
---

Github user lujinhong commented on a diff in the pull request:

https://github.com/apache/storm/pull/1522#discussion_r68908757
  
--- Diff: storm-core/src/jvm/org/apache/storm/tuple/Fields.java ---
@@ -50,8 +50,12 @@ public Fields(List fields) {
 
 public List select(Fields selector, List tuple) {
 List ret = new ArrayList<>(selector.size());
-for(String s: selector) {
-ret.add(tuple.get(_index.get(s)));
--- End diff --

yes, it is a better idea.


> org.apache.storm.tuple.Fields can throw NPE if given invalid field in selector
> --
>
> Key: STORM-1594
> URL: https://issues.apache.org/jira/browse/STORM-1594
> Project: Apache Storm
>  Issue Type: Bug
>  Components: storm-core
>Reporter: Alessandro Bellina
>Priority: Minor
>  Labels: npe
>
> In class org.apache.storm.tuple.Fields, the select method gets the index of 
> the field to select from the first argument (selector) by using .get. It 
> doesn't set this result to an Integer and check for null (Map.get returns 
> null if the key is not found).
> When tuple.get happens, the parameter for tuple.get is an unboxed integer. 
> The null cannot be unboxed to integer, causing an NPE. There is another 
> method in Fields called fieldIndex which will throw an 
> IllegalArgumentException in the case that the field in the selector isn't in 
> the _index. 
> {code}
> public List select(Fields selector, List tuple) {
> List ret = new ArrayList<>(selector.size());  
> for(String s: selector) {
> ret.add(tuple.get(_index.get(s)));
> }
> return ret;
> }
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[GitHub] storm issue #1522: [STORM-1594] org.apache.storm.tuple.Fields can throw NPE ...

2016-06-29 Thread abhishekagarwal87
Github user abhishekagarwal87 commented on the issue:

https://github.com/apache/storm/pull/1522
  
Thank you @lujinhong. I am +1 pending travis. Please squash the commits 
once travis is passed.


---
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.
---


[jira] [Commented] (STORM-1934) Race condition between sync-supervisor and sync-processes raises several strange issues

2016-06-29 Thread Jungtaek Lim (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1934?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15354836#comment-15354836
 ] 

Jungtaek Lim commented on STORM-1934:
-

Linking other suspected issues.

> Race condition between sync-supervisor and sync-processes raises several 
> strange issues
> ---
>
> Key: STORM-1934
> URL: https://issues.apache.org/jira/browse/STORM-1934
> Project: Apache Storm
>  Issue Type: Bug
>  Components: storm-core
>Affects Versions: 1.0.0, 2.0.0, 1.0.1
>Reporter: Jungtaek Lim
>Assignee: Jungtaek Lim
>Priority: Critical
>
> There're some strange issues including STORM-1933 and others (which I will 
> file an issue soon) which are related to race condition in supervisor.
> As I mentioned to STORM-1933, basically sync-supervisor relies on zk 
> assignment, and sync-processes relies on local assignment and local workers 
> directory, but in fact sync-supervisor also access local state and take some 
> actions which affects sync-processes. And also Satish left the comment to 
> STORM-1933 describing other issue related to race condition and idea to fix 
> this which is same page on me.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[GitHub] storm pull request #1522: [STORM-1594] org.apache.storm.tuple.Fields can thr...

2016-06-29 Thread lujinhong
Github user lujinhong commented on a diff in the pull request:

https://github.com/apache/storm/pull/1522#discussion_r68908757
  
--- Diff: storm-core/src/jvm/org/apache/storm/tuple/Fields.java ---
@@ -50,8 +50,12 @@ public Fields(List fields) {
 
 public List select(Fields selector, List tuple) {
 List ret = new ArrayList<>(selector.size());
-for(String s: selector) {
-ret.add(tuple.get(_index.get(s)));
--- End diff --

yes, it is a better idea.


---
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.
---


[jira] [Commented] (STORM-1879) Supervisor may not shut down workers cleanly

2016-06-29 Thread Jungtaek Lim (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1879?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15354803#comment-15354803
 ] 

Jungtaek Lim commented on STORM-1879:
-

Sorry for late participating. I have been struggling with other works.

I'm suspecting that many issues from supervisor are from race condition 
(sync-supervisor and sync-processes).

One of supervisor test is intermittently failing 
([STORM-1933|https://issues.apache.org/jira/browse/STORM-1933]), and after 
digging I found that supervisor has race condition which can create various 
issues.
(What [~nico.meyer] pointed out seems to be same to what STORM-1933 shows.)

I submitted a [patch|https://github.com/apache/storm/pull/1528] to 
[STORM-1934|https://issues.apache.org/jira/browse/STORM-1934] so I'd be really 
happy if you applies my patch and see it works. 

> Supervisor may not shut down workers cleanly
> 
>
> Key: STORM-1879
> URL: https://issues.apache.org/jira/browse/STORM-1879
> Project: Apache Storm
>  Issue Type: Bug
>  Components: storm-core
>Affects Versions: 1.0.1
>Reporter: Stig Rohde Døssing
> Attachments: fix_missing_worker_pid.patch, nimbus-supervisor.zip, 
> supervisor.log
>
>
> We've run into a strange issue with a zombie worker process. It looks like 
> the worker pid file somehow got deleted without the worker process shutting 
> down. This causes the supervisor to try repeatedly to kill the worker 
> unsuccessfully, and means multiple workers may be assigned to the same port. 
> The worker root folder sticks around because the worker is still heartbeating 
> to it.
> It may or may not be related that we've seen Nimbus occasionally enter an 
> infinite loop of printing logs similar to the below.
> {code}
> 2016-05-19 14:55:14.196 o.a.s.b.BlobStoreUtils [ERROR] Could not update the 
> blob with keyZendeskTicketTopology-5-1463647641-stormconf.ser
> 2016-05-19 14:55:14.210 o.a.s.b.BlobStoreUtils [ERROR] Could not update the 
> blob with keyZendeskTicketTopology-5-1463647641-stormcode.ser
> 2016-05-19 14:55:14.218 o.a.s.b.BlobStoreUtils [ERROR] Could not update the 
> blob with keyZendeskTicketTopology-5-1463647641-stormconf.ser
> 2016-05-19 14:55:14.256 o.a.s.b.BlobStoreUtils [ERROR] Could not update the 
> blob with keyZendeskTicketTopology-5-1463647641-stormcode.ser
> 2016-05-19 14:55:14.273 o.a.s.b.BlobStoreUtils [ERROR] Could not update the 
> blob with keyZendeskTicketTopology-5-1463647641-stormcode.ser
> 2016-05-19 14:55:14.316 o.a.s.b.BlobStoreUtils [ERROR] Could not update the 
> blob with keyZendeskTicketTopology-5-1463647641-stormconf.ser
> {code}
> Which continues until Nimbus is rebooted. We also see repeating blocks 
> similar to the logs below.
> {code}
> 2016-06-02 07:45:03.656 o.a.s.d.nimbus [INFO] Cleaning up 
> ZendeskTicketTopology-127-1464780171
> 2016-06-02 07:45:04.132 o.a.s.d.nimbus [INFO] 
> ExceptionKeyNotFoundException(msg:ZendeskTicketTopology-127-1464780171-stormjar.jar)
> 2016-06-02 07:45:04.144 o.a.s.d.nimbus [INFO] 
> ExceptionKeyNotFoundException(msg:ZendeskTicketTopology-127-1464780171-stormconf.ser)
> 2016-06-02 07:45:04.155 o.a.s.d.nimbus [INFO] 
> ExceptionKeyNotFoundException(msg:ZendeskTicketTopology-127-1464780171-stormcode.ser)
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Commented] (STORM-1934) Race condition between sync-supervisor and sync-processes raises several strange issues

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1934?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15354765#comment-15354765
 ] 

ASF GitHub Bot commented on STORM-1934:
---

Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/1528
  
Note: This patch removes the change from STORM-1561 since supervisor 
originally checks the change of executors in same port. 
Please refer `read-allocated-workers` and `matches-an-assignment?`, and let 
me know if I'm missing here.


> Race condition between sync-supervisor and sync-processes raises several 
> strange issues
> ---
>
> Key: STORM-1934
> URL: https://issues.apache.org/jira/browse/STORM-1934
> Project: Apache Storm
>  Issue Type: Bug
>  Components: storm-core
>Affects Versions: 1.0.0, 2.0.0, 1.0.1
>Reporter: Jungtaek Lim
>Assignee: Jungtaek Lim
>Priority: Critical
>
> There're some strange issues including STORM-1933 and others (which I will 
> file an issue soon) which are related to race condition in supervisor.
> As I mentioned to STORM-1933, basically sync-supervisor relies on zk 
> assignment, and sync-processes relies on local assignment and local workers 
> directory, but in fact sync-supervisor also access local state and take some 
> actions which affects sync-processes. And also Satish left the comment to 
> STORM-1933 describing other issue related to race condition and idea to fix 
> this which is same page on me.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[GitHub] storm issue #1528: STORM-1934 Fix race condition between sync-supervisor and...

2016-06-29 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/1528
  
Note: This patch removes the change from STORM-1561 since supervisor 
originally checks the change of executors in same port. 
Please refer `read-allocated-workers` and `matches-an-assignment?`, and let 
me know if I'm missing here.


---
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.
---


[jira] [Commented] (STORM-1594) org.apache.storm.tuple.Fields can throw NPE if given invalid field in selector

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1594?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15354757#comment-15354757
 ] 

ASF GitHub Bot commented on STORM-1594:
---

Github user abhishekagarwal87 commented on a diff in the pull request:

https://github.com/apache/storm/pull/1522#discussion_r68901523
  
--- Diff: storm-core/src/jvm/org/apache/storm/tuple/Fields.java ---
@@ -50,8 +50,12 @@ public Fields(List fields) {
 
 public List select(Fields selector, List tuple) {
 List ret = new ArrayList<>(selector.size());
-for(String s: selector) {
-ret.add(tuple.get(_index.get(s)));
--- End diff --

You can replace _index.get(s) with fieldIndex(s) which throws 
IllegalArgumentException.


> org.apache.storm.tuple.Fields can throw NPE if given invalid field in selector
> --
>
> Key: STORM-1594
> URL: https://issues.apache.org/jira/browse/STORM-1594
> Project: Apache Storm
>  Issue Type: Bug
>  Components: storm-core
>Reporter: Alessandro Bellina
>Priority: Minor
>  Labels: npe
>
> In class org.apache.storm.tuple.Fields, the select method gets the index of 
> the field to select from the first argument (selector) by using .get. It 
> doesn't set this result to an Integer and check for null (Map.get returns 
> null if the key is not found).
> When tuple.get happens, the parameter for tuple.get is an unboxed integer. 
> The null cannot be unboxed to integer, causing an NPE. There is another 
> method in Fields called fieldIndex which will throw an 
> IllegalArgumentException in the case that the field in the selector isn't in 
> the _index. 
> {code}
> public List select(Fields selector, List tuple) {
> List ret = new ArrayList<>(selector.size());  
> for(String s: selector) {
> ret.add(tuple.get(_index.get(s)));
> }
> return ret;
> }
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[GitHub] storm pull request #1522: [STORM-1594] org.apache.storm.tuple.Fields can thr...

2016-06-29 Thread abhishekagarwal87
Github user abhishekagarwal87 commented on a diff in the pull request:

https://github.com/apache/storm/pull/1522#discussion_r68901523
  
--- Diff: storm-core/src/jvm/org/apache/storm/tuple/Fields.java ---
@@ -50,8 +50,12 @@ public Fields(List fields) {
 
 public List select(Fields selector, List tuple) {
 List ret = new ArrayList<>(selector.size());
-for(String s: selector) {
-ret.add(tuple.get(_index.get(s)));
--- End diff --

You can replace _index.get(s) with fieldIndex(s) which throws 
IllegalArgumentException.


---
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.
---


[jira] [Commented] (STORM-1934) Race condition between sync-supervisor and sync-processes raises several strange issues

2016-06-29 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/STORM-1934?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15354750#comment-15354750
 ] 

ASF GitHub Bot commented on STORM-1934:
---

GitHub user HeartSaVioR opened a pull request:

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

STORM-1934 Fix race condition between sync-supervisor and sync-processes

* sync-supervisor just downloads new topology code and writes new local 
assignment
  * shutting down workers and removing topology code is moved to 
sync-processes
* sync-processes does all of jobs based on local assignment and allocated 
workers
* remove unused / unneeded codes

Here's my test result for this patch:

* `mvn clean install` 5 times: not met supervisor intermittent failure 
(STORM-1933)
  * will try more times
* kill worker via `kill`, `kill -9`, `restart worker` from UI: no issue on 
restarting worker
* rebalance topology to change workers (2 -> 3): to test that new 
assignment has same worker port but different executors compared to assigned 
workers
  * worker is recognized as :disallowed, and killed & relaunched

Rebalance test in details:

- Writing new assignment
```
6701 {:storm-id "test-topology2-4-1467185073", :executors ([7 7] [5 5] [3 
3] [1 1]), :resources [0.0 0.0 0.0]}, 
6702 {:storm-id "test-topology2-4-1467185073", :executors ([6 6] [4 4] [2 
2]), :resources [0.0 0.0 0.0]}
```

- Assigned executors:
```
6701 {:storm-id "test-topology2-4-1467185073", :executors [[7 7] [5 5] [3 
3] [1 1]], :resources #object[org.apache.storm.generated.WorkerResources 
0x40c4d31c "WorkerResources(mem_on_heap:0.0, mem_off_heap:0.0, cpu:0.0)"]}, 
6702 {:storm-id "test-topology2-4-1467185073", :executors [[6 6] [4 4] [2 
2]], :resources #object[org.apache.storm.generated.WorkerResources 0x4ba861f4 
"WorkerResources(mem_on_heap:0.0, mem_off_heap:0.0, cpu:0.0)"]}}
```

- Allocated:
```
"2e9bea10-02b7-4e55-88e7-b194b9917a63" [:disallowed {:time-secs 1467185407, 
:storm-id "test-topology2-4-1467185073", :executors [[3 3] [6 6] [-1 -1]], 
:port 6703}], 
"4630c4bf-9786-47ff-9f3b-6b42d9781b9d" [:disallowed {:time-secs 1467185407, 
:storm-id "test-topology2-4-1467185073", :executors [[7 7] [1 1] [-1 -1] [4 
4]], :port 6701}], 
"b9a622d2-5e5b-4311-999c-8c8dd92da6b6" [:disallowed {:time-secs 1467185406, 
:storm-id "test-topology2-4-1467185073", :executors [[2 2] [-1 -1] [5 5]], 
:port 6702}]}
```

NOTE: Due to forward reference, I have to move `sync-processes` to just 
before `mk-synchronize-supervisor`. Major changes are done in sync-processes so 
reviewers need to compare before & after manually. Sorry about that.

Since supervisor.clj is already ported to Java in master branch, I should 
have time to read ported code, and modify to be in sync.

Please review and comment while I'm working against master branch. Thanks!

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/HeartSaVioR/storm STORM-1934-1.x

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/storm/pull/1528.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 #1528


commit e5857e07838af888988691af39efbe415b9a2345
Author: Jungtaek Lim 
Date:   2016-06-29T07:06:20Z

STORM-1934 Fix race condition between sync-supervisor and sync-processes

* sync-supervisor just downloads new topology code and writes new local 
assignment
  * shutting down workers and removing topology code is moved to 
sync-processes
* sync-processes does all of jobs based on local assignment and allocated 
workers
* remove unused / unneeded codes




> Race condition between sync-supervisor and sync-processes raises several 
> strange issues
> ---
>
> Key: STORM-1934
> URL: https://issues.apache.org/jira/browse/STORM-1934
> Project: Apache Storm
>  Issue Type: Bug
>  Components: storm-core
>Affects Versions: 1.0.0, 2.0.0, 1.0.1
>Reporter: Jungtaek Lim
>Assignee: Jungtaek Lim
>Priority: Critical
>
> There're some strange issues including STORM-1933 and others (which I will 
> file an issue soon) which are related to race condition in supervisor.
> As I mentioned to STORM-1933, basically sync-supervisor relies on zk 
> assignment, and sync-processes relies on local assignment and local workers 
> directory, but in fact sync-supervisor also access local state and take some 
> actions which affects sync-processes. And also Satish left the comment to 
> STORM-1933 describing other issue related to race

[GitHub] storm pull request #1528: STORM-1934 Fix race condition between sync-supervi...

2016-06-29 Thread HeartSaVioR
GitHub user HeartSaVioR opened a pull request:

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

STORM-1934 Fix race condition between sync-supervisor and sync-processes

* sync-supervisor just downloads new topology code and writes new local 
assignment
  * shutting down workers and removing topology code is moved to 
sync-processes
* sync-processes does all of jobs based on local assignment and allocated 
workers
* remove unused / unneeded codes

Here's my test result for this patch:

* `mvn clean install` 5 times: not met supervisor intermittent failure 
(STORM-1933)
  * will try more times
* kill worker via `kill`, `kill -9`, `restart worker` from UI: no issue on 
restarting worker
* rebalance topology to change workers (2 -> 3): to test that new 
assignment has same worker port but different executors compared to assigned 
workers
  * worker is recognized as :disallowed, and killed & relaunched

Rebalance test in details:

- Writing new assignment
```
6701 {:storm-id "test-topology2-4-1467185073", :executors ([7 7] [5 5] [3 
3] [1 1]), :resources [0.0 0.0 0.0]}, 
6702 {:storm-id "test-topology2-4-1467185073", :executors ([6 6] [4 4] [2 
2]), :resources [0.0 0.0 0.0]}
```

- Assigned executors:
```
6701 {:storm-id "test-topology2-4-1467185073", :executors [[7 7] [5 5] [3 
3] [1 1]], :resources #object[org.apache.storm.generated.WorkerResources 
0x40c4d31c "WorkerResources(mem_on_heap:0.0, mem_off_heap:0.0, cpu:0.0)"]}, 
6702 {:storm-id "test-topology2-4-1467185073", :executors [[6 6] [4 4] [2 
2]], :resources #object[org.apache.storm.generated.WorkerResources 0x4ba861f4 
"WorkerResources(mem_on_heap:0.0, mem_off_heap:0.0, cpu:0.0)"]}}
```

- Allocated:
```
"2e9bea10-02b7-4e55-88e7-b194b9917a63" [:disallowed {:time-secs 1467185407, 
:storm-id "test-topology2-4-1467185073", :executors [[3 3] [6 6] [-1 -1]], 
:port 6703}], 
"4630c4bf-9786-47ff-9f3b-6b42d9781b9d" [:disallowed {:time-secs 1467185407, 
:storm-id "test-topology2-4-1467185073", :executors [[7 7] [1 1] [-1 -1] [4 
4]], :port 6701}], 
"b9a622d2-5e5b-4311-999c-8c8dd92da6b6" [:disallowed {:time-secs 1467185406, 
:storm-id "test-topology2-4-1467185073", :executors [[2 2] [-1 -1] [5 5]], 
:port 6702}]}
```

NOTE: Due to forward reference, I have to move `sync-processes` to just 
before `mk-synchronize-supervisor`. Major changes are done in sync-processes so 
reviewers need to compare before & after manually. Sorry about that.

Since supervisor.clj is already ported to Java in master branch, I should 
have time to read ported code, and modify to be in sync.

Please review and comment while I'm working against master branch. Thanks!

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/HeartSaVioR/storm STORM-1934-1.x

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/storm/pull/1528.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 #1528


commit e5857e07838af888988691af39efbe415b9a2345
Author: Jungtaek Lim 
Date:   2016-06-29T07:06:20Z

STORM-1934 Fix race condition between sync-supervisor and sync-processes

* sync-supervisor just downloads new topology code and writes new local 
assignment
  * shutting down workers and removing topology code is moved to 
sync-processes
* sync-processes does all of jobs based on local assignment and allocated 
workers
* remove unused / unneeded codes




---
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.
---


[jira] [Updated] (STORM-1934) Race condition between sync-supervisor and sync-processes raises several strange issues

2016-06-29 Thread Jungtaek Lim (JIRA)

 [ 
https://issues.apache.org/jira/browse/STORM-1934?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Jungtaek Lim updated STORM-1934:

Description: 
There're some strange issues including STORM-1933 and others (which I will file 
an issue soon) which are related to race condition in supervisor.

As I mentioned to STORM-1933, basically sync-supervisor relies on zk 
assignment, and sync-processes relies on local assignment and local workers 
directory, but in fact sync-supervisor also access local state and take some 
actions which affects sync-processes. And also Satish left the comment to 
STORM-1933 describing other issue related to race condition and idea to fix 
this which is same page on me.



  was:
There're some strange issues including STORM-1933 and others (which I will file 
an issue soo) which are related to race condition in supervisor.

As I mentioned to STORM-1933, basically sync-supervisor relies on zk 
assignment, and sync-processes relies on local assignment and local workers 
directory, but in fact sync-supervisor also access local state and take some 
actions which affects sync-processes. And also Satish left the comment to 
STORM-1933 describing other issue related to race condition and idea to fix 
this which is same page on me.




> Race condition between sync-supervisor and sync-processes raises several 
> strange issues
> ---
>
> Key: STORM-1934
> URL: https://issues.apache.org/jira/browse/STORM-1934
> Project: Apache Storm
>  Issue Type: Bug
>  Components: storm-core
>Affects Versions: 1.0.0, 2.0.0, 1.0.1
>Reporter: Jungtaek Lim
>Assignee: Jungtaek Lim
>Priority: Critical
>
> There're some strange issues including STORM-1933 and others (which I will 
> file an issue soon) which are related to race condition in supervisor.
> As I mentioned to STORM-1933, basically sync-supervisor relies on zk 
> assignment, and sync-processes relies on local assignment and local workers 
> directory, but in fact sync-supervisor also access local state and take some 
> actions which affects sync-processes. And also Satish left the comment to 
> STORM-1933 describing other issue related to race condition and idea to fix 
> this which is same page on me.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)