[GitHub] storm pull request: This closes #STORM-1730 #1730 LocalCluster#shu...

2016-05-09 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/1400#issuecomment-217898782 @fbyrne, closing and reopening the PR or adding commit triggers the build. --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] storm pull request: STORM-1837: Fix complete-topology and prevent ...

2016-05-16 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/1417#issuecomment-219545865 The code changes look good. I ran unit test and had `netty_unit_test` timeout once. I think we need to change the timeout for https://github.com/apache/storm

[GitHub] storm pull request: STORM-1837: Fix complete-topology and prevent ...

2016-05-17 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/1417#issuecomment-219741531 @HeartSaVioR, you are right, I did not notice the `DisruptorQueueTest` failure. @srdo, Thank you for putting up #1421. It's ironic to see `LocalNimbu

[GitHub] storm pull request: STORM-1844: Make netty unit test use STORM_TES...

2016-05-17 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/1421#issuecomment-219741847 This should help us avoid more of those unwanted build failures. --- If your project is set up for it, you can reply to this email and have your reply appear on

[GitHub] storm pull request: STORM-1844: Make netty unit test use STORM_TES...

2016-05-17 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/1421#issuecomment-219741678 +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

[GitHub] storm pull request: Bump version to 1.1.0-SNAPSHOT

2016-05-19 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/1432#issuecomment-220371291 +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

[GitHub] storm issue #1486: STORM-1890: ensure static UI resources are re-requested w...

2016-06-14 Thread kishorvpatil
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/1486 LGTM. +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

[GitHub] storm pull request #1536: Storm 1890 ensure we refetch static resources afte...

2016-07-13 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/1536#discussion_r70703286 --- Diff: storm-core/src/clj/org/apache/storm/ui/helpers.clj --- @@ -36,22 +37,35 @@ [org.eclipse.jetty.server DispatcherType

[GitHub] storm issue #1536: Storm 1890 ensure we refetch static resources after packa...

2016-07-13 Thread kishorvpatil
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/1536 One minor comment. The rest looks good. --- 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

[GitHub] storm issue #1536: Storm 1890 ensure we refetch static resources after packa...

2016-07-15 Thread kishorvpatil
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/1536 Thank you @abellina. 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

[GitHub] storm issue #1628: STORM-2041. Make Java 8 as minimum requirement for 2.0 re...

2016-08-16 Thread kishorvpatil
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/1628 +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

[GitHub] storm pull request #1642: STORM-2018: Supervisor V2.

2016-08-29 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/1642#discussion_r76637435 --- Diff: storm-core/src/jvm/org/apache/storm/daemon/supervisor/AdvancedFSOps.java --- @@ -0,0 +1,313 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request #1642: STORM-2018: Supervisor V2.

2016-08-29 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/1642#discussion_r76637608 --- Diff: storm-core/src/jvm/org/apache/storm/daemon/supervisor/AdvancedFSOps.java --- @@ -0,0 +1,300 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request: STORM-430: Allow netty SASL to support encrypt...

2014-10-28 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/250#issuecomment-60834167 @RaghavendraNandagopal, would you like to respond to @revans2's comments? --- If your project is set up for it, you can reply to this email and have your

[GitHub] storm pull request: [STORM-569] Add Configuration to enable/disabl...

2014-11-21 Thread kishorvpatil
GitHub user kishorvpatil opened a pull request: https://github.com/apache/storm/pull/324 [STORM-569] Add Configuration to enable/disable Bolt's outgoing overflow-buffer - Added topology level flag `topology.bolts.outgoing.overflow.buffer.enable` - which defaults to

[GitHub] storm pull request: [STORM-570] replace table sorter with data tab...

2014-11-26 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/328#issuecomment-64673038 For dataTables, I would recommend the use of `compact` class to reduce the amount of white-space, instead of `stripe` --- If your project is set up for it, you can

[GitHub] storm pull request: [STORM-570] replace table sorter with data tab...

2014-11-26 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/328#issuecomment-64682472 `Tooltip` placing/formatting etc seems to have changed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as

[GitHub] storm pull request: STORM-632: New grouping for better load balanc...

2015-01-23 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/395#issuecomment-71250215 LGTM. +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

[GitHub] storm pull request: [STORM-570] replace table sorter with data tab...

2015-01-27 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/328#issuecomment-71722015 +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

[GitHub] storm pull request: [STORM-570] replace table sorter with data tab...

2015-01-27 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/328#issuecomment-71722168 @revans2 could you please upmerge the change again? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well

[GitHub] storm pull request: update supervisor.clj add a comment to supervi...

2015-01-30 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/258#issuecomment-72265757 @BuDongDong, can you please create JIRA to elaborate on the change, and need to delete "missing-tasks"? Also it would be good to get PR subject descri

[GitHub] storm pull request: Allow parsing of String into int for command l...

2015-01-30 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/119#issuecomment-72262509 @jreichhold could you please address the concerns raised here? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub

[GitHub] storm pull request: [STORM-400] Thrift upgrade to thrift-0.9.2

2015-02-02 Thread kishorvpatil
GitHub user kishorvpatil opened a pull request: https://github.com/apache/storm/pull/410 [STORM-400] Thrift upgrade to thrift-0.9.2 You can merge this pull request into a Git repository by running: $ git pull https://github.com/kishorvpatil/incubator-storm thrift092

[GitHub] storm pull request: [STORM-400] Thrift upgrade to thrift-0.9.2

2015-02-05 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/410#issuecomment-73174063 @ptgoetz, Thanks for reviewing this. I have up merged the changes. --- If your project is set up for it, you can reply to this email and have your reply appear on

[GitHub] storm pull request: [STORM-653] missing DRPC HTTP port in SECURITY...

2015-02-09 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/413#issuecomment-73544830 +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

[GitHub] storm pull request: [STORM-400] Thrift upgrade to thrift-0.9.2

2015-02-17 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/410#issuecomment-74700449 @darionyaphet, Thrift 0.9.2 includes python hash function. [https://issues.apache.org/jira/browse/THRIFT-2621]. Without this change, we were using @nathanmarz &#

[GitHub] storm pull request: STORM-563. Kafka Spout doesn't pick up from th...

2015-02-23 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/321#issuecomment-75573879 @harshach would you like to provide update on this? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well

[GitHub] storm pull request: STORM-532:Supervisor should restart worker imm...

2015-03-02 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/296#issuecomment-76745378 @caofangkun, could you please upmerge this change? --- 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: [STORM-557] Created docs directory and added i...

2015-03-02 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/314#issuecomment-76749927 +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

[GitHub] storm pull request: [STORM-557] Created docs directory and added i...

2015-03-02 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/314#issuecomment-76750061 Thank you @revans2. --- 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

[GitHub] storm pull request: STORM-682: supervisor should handle worker sta...

2015-03-02 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/437#issuecomment-76751433 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

[GitHub] storm pull request: [STORM-625] Don't leak netty clients when work...

2015-03-10 Thread kishorvpatil
GitHub user kishorvpatil opened a pull request: https://github.com/apache/storm/pull/461 [STORM-625] Don't leak netty clients when worker moves or reuse netty client. - Adding tracking of netty connection by key and reuse whenever client is available. - Fix included ori

[GitHub] storm pull request: [STORM-712] Storm daemons shutdown if OutOfMem...

2015-03-18 Thread kishorvpatil
GitHub user kishorvpatil opened a pull request: https://github.com/apache/storm/pull/468 [STORM-712] Storm daemons shutdown if OutOfMemoryError occurs in any thread - Creating `ExtendedThreadPoolExecutor` to pass uncaughtExceptions as part of `afterExecute` - All daemon main

[GitHub] storm pull request: [STORM-570] replace table sorter with data tab...

2015-03-19 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/328#issuecomment-83749375 @revans2 Thank you. I have merged in the changes. Sorry for delay. --- If your project is set up for it, you can reply to this email and have your reply appear on

[GitHub] storm pull request: [STORM-712] Storm daemons shutdown if OutOfMem...

2015-03-25 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/468#issuecomment-86350235 @lazyval, sorry for delay in responding. We have seen this happen in case some of our clusters. When the `ExecutorService`, executes the `FutureTask`, it captures

[GitHub] storm pull request: [STORM-712] Storm daemons shutdown if OutOfMem...

2015-03-27 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/468#discussion_r27317843 --- Diff: storm-core/src/jvm/backtype/storm/drpc/DRPCSpout.java --- @@ -129,7 +133,9 @@ private void checkFutures() { public void open(Map conf

[GitHub] storm pull request: [STORM-712] Storm daemons shutdown if OutOfMem...

2015-03-27 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/468#discussion_r27318086 --- Diff: storm-core/src/jvm/backtype/storm/utils/Utils.java --- @@ -533,4 +533,16 @@ private static SerializationDelegate getSerializationDelegate(Map

[GitHub] storm pull request: STORM-750: Set Config serialVersionUID

2015-04-06 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/507#issuecomment-90160883 +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

[GitHub] storm pull request: [STORM-712] Storm daemons shutdown if OutOfMem...

2015-04-08 Thread kishorvpatil
Github user kishorvpatil commented on the pull request: https://github.com/apache/storm/pull/468#issuecomment-91057918 @lazyval Thanks for pointing out. I have updated the PR to rethrow non OOM Errors. Also, main server thread should exit on Error. --- If your project is set up for

[GitHub] storm pull request #2312: YSTORM-2725: Generic Resource Scheduling - initial...

2017-09-14 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/2312#discussion_r138924774 --- Diff: storm-client/src/jvm/org/apache/storm/topology/BaseConfigurationDeclarer.java --- @@ -83,4 +83,29 @@ public T setCPULoad(Number amount

[GitHub] storm pull request #2312: YSTORM-2725: Generic Resource Scheduling - initial...

2017-09-14 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/2312#discussion_r138926679 --- Diff: storm-server/src/main/java/org/apache/storm/daemon/supervisor/timer/SupervisorHeartbeat.java --- @@ -73,10 +73,24 @@ private SupervisorInfo

[GitHub] storm pull request #2337: [STORM2751] Removing AsyncLoggerContext from Super...

2017-09-21 Thread kishorvpatil
GitHub user kishorvpatil opened a pull request: https://github.com/apache/storm/pull/2337 [STORM2751] Removing AsyncLoggerContext from Supervisor Configuration If disk can not keep it up with all logging, the {{AsyncLoggingContext}} causes large heap memory be utilized causing the

[GitHub] storm issue #2337: [STORM2751] Removing AsyncLoggerContext from Supervisor C...

2017-09-21 Thread kishorvpatil
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/2337 @HeartSaVioR I am noticed it on supervisors, but I am suspecting topologies being too verbose making many workers adding load on disk. I am not sure if supervisor is too verbose. But with

[GitHub] storm issue #2337: [STORM2751] Removing AsyncLoggerContext from Supervisor C...

2017-09-21 Thread kishorvpatil
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/2337 @HeartSaVioR I do not notice this on logviewer but clearly it do not have so much activity or logging for that matter. ---

[GitHub] storm pull request #2337: [STORM2751] Removing AsyncLoggerContext from Super...

2017-09-21 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/2337#discussion_r140328729 --- Diff: bin/storm.py --- @@ -739,7 +739,6 @@ def supervisor(klass="org.apache.storm.daemon.supervisor.Supervisor"):

[GitHub] storm pull request #2358: [STORM-2770] Add fragmentation metrics for CPU and...

2017-10-04 Thread kishorvpatil
GitHub user kishorvpatil opened a pull request: https://github.com/apache/storm/pull/2358 [STORM-2770] Add fragmentation metrics for CPU and Memory The patch addresses following: - Calculate fragmentation of CPU/memory - Display it on UI under Cluster summary - Add

[GitHub] storm pull request #2345: STORM-2438: added in rebalance changes to support ...

2017-10-09 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/2345#discussion_r143499778 --- Diff: storm-server/src/main/java/org/apache/storm/daemon/supervisor/Slot.java --- @@ -104,9 +111,12 @@ this.port = port

[GitHub] storm pull request #2345: STORM-2438: added in rebalance changes to support ...

2017-10-09 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/2345#discussion_r143501314 --- Diff: storm-server/src/main/java/org/apache/storm/daemon/supervisor/Slot.java --- @@ -263,7 +314,41 @@ public String toString

[GitHub] storm pull request #2345: STORM-2438: added in rebalance changes to support ...

2017-10-09 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/2345#discussion_r143510721 --- Diff: storm-server/src/main/java/org/apache/storm/localizer/AsyncLocalizer.java --- @@ -156,17 +146,141 @@ public AsyncLocalizer(Map conf

[GitHub] storm pull request #2345: STORM-2438: added in rebalance changes to support ...

2017-10-09 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/2345#discussion_r143559023 --- Diff: storm-server/src/main/java/org/apache/storm/localizer/LocallyCachedBlob.java --- @@ -0,0 +1,220 @@ +/** + * Licensed to the Apache

[GitHub] storm issue #2345: STORM-2438: added in rebalance changes to support RAS

2017-10-10 Thread kishorvpatil
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/2345 @revans2 LGTM. ---

[GitHub] storm pull request #2363: STORM-2759: Let users indicate if a blob should re...

2017-10-10 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/2363#discussion_r143844543 --- Diff: storm-server/src/test/java/org/apache/storm/localizer/AsyncLocalizerTest.java --- @@ -286,29 +297,80 @@ public void

[GitHub] storm pull request #2390: [STORM-2790] Add nimbus admins groups

2017-10-27 Thread kishorvpatil
GitHub user kishorvpatil opened a pull request: https://github.com/apache/storm/pull/2390 [STORM-2790] Add nimbus admins groups You can merge this pull request into a Git repository by running: $ git pull https://github.com/kishorvpatil/incubator-storm add-admin-groups

[GitHub] storm pull request #2390: [STORM-2790] Add nimbus admins groups

2017-10-30 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/2390#discussion_r147774596 --- Diff: storm-server/src/main/java/org/apache/storm/DaemonConfig.java --- @@ -159,14 +159,6 @@ public static final String

[GitHub] storm pull request #2400: STORM-2792: Remove RAS EvictionPolicy and cleanup

2017-11-03 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/2400#discussion_r148882086 --- Diff: storm-server/src/main/java/org/apache/storm/scheduler/resource/ResourceAwareScheduler.java --- @@ -62,123 +70,144 @@ public void prepare(Map

[GitHub] storm pull request #2456: YSTORM-4457: Fix for wouldFit

2017-12-14 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/2456#discussion_r157072777 --- Diff: storm-server/src/test/java/org/apache/storm/scheduler/resource/TestResourceAwareScheduler.java --- @@ -640,6 +641,150 @@ public void

[GitHub] storm pull request #2487: [STORM-2873] Do not delete backpressure ephemeral ...

2017-12-29 Thread kishorvpatil
GitHub user kishorvpatil opened a pull request: https://github.com/apache/storm/pull/2487 [STORM-2873] Do not delete backpressure ephemeral node frequently If ephemeral znode is created once, then we can leave it as is - as other workers would look at timestamp to ensure it is not

[GitHub] storm issue #2487: [STORM-2873] Do not delete backpressure ephemeral node fr...

2017-12-29 Thread kishorvpatil
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/2487 @srdo , Please review changes addressing the comments. ---

[GitHub] storm issue #2487: [STORM-2873] Do not delete backpressure ephemeral node fr...

2018-01-02 Thread kishorvpatil
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/2487 Ok. I will create the PR for 1.x-branch. ---

[GitHub] storm pull request #2529: [STORM-2873] Do not delete backpressure ephemeral ...

2018-01-23 Thread kishorvpatil
GitHub user kishorvpatil opened a pull request: https://github.com/apache/storm/pull/2529 [STORM-2873] Do not delete backpressure ephemeral node frequently - Do not delete backpressure ephemeral node frequently - Add backpressure timeout. - Cleanup backpressure znodes

[GitHub] storm issue #2487: [STORM-2873] Do not delete backpressure ephemeral node fr...

2018-01-23 Thread kishorvpatil
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/2487 @HeartSaVioR, Please find patch for 1.x-branch - https://github.com/apache/storm/pull/2529 ---

[GitHub] storm pull request #2531: STORM-2898: Support for WorkerToken authentication

2018-01-24 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/2531#discussion_r163728603 --- Diff: storm-client/src/jvm/org/apache/storm/security/auth/kerberos/KerberosSaslTransportPlugin.java --- @@ -114,22 +120,26 @@ public

[GitHub] storm pull request #2531: STORM-2898: Support for WorkerToken authentication

2018-01-24 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/2531#discussion_r163729225 --- Diff: storm-client/src/jvm/org/apache/storm/security/auth/kerberos/ServerCallbackHandler.java --- @@ -18,79 +18,86 @@ package

[GitHub] storm pull request #2531: STORM-2898: Support for WorkerToken authentication

2018-01-26 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/2531#discussion_r164134542 --- Diff: storm-core/test/clj/org/apache/storm/nimbus_test.clj --- @@ -46,7 +46,7 @@ (:import [org.apache.commons.io FileUtils]) (:import

[GitHub] storm pull request #2531: STORM-2898: Support for WorkerToken authentication

2018-01-26 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/2531#discussion_r164154647 --- Diff: storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java --- @@ -2638,6 +2647,34 @@ private int getNumOfAckerExecs(Map

[GitHub] storm issue #2487: [STORM-2873] Do not delete backpressure ephemeral node fr...

2018-02-12 Thread kishorvpatil
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/2487 closing the PR as no longer needed for master branch. ---

[GitHub] storm pull request #2487: [STORM-2873] Do not delete backpressure ephemeral ...

2018-02-12 Thread kishorvpatil
Github user kishorvpatil closed the pull request at: https://github.com/apache/storm/pull/2487 ---

[GitHub] storm pull request #2563: [STORM-2961] Refactor addResource and addResources...

2018-02-19 Thread kishorvpatil
GitHub user kishorvpatil opened a pull request: https://github.com/apache/storm/pull/2563 [STORM-2961] Refactor addResource and addResources Methods in BaseConfigurationDeclarer You can merge this pull request into a Git repository by running: $ git pull https://github.com

[GitHub] storm issue #2563: [STORM-2961] Refactor addResource and addResources Method...

2018-02-19 Thread kishorvpatil
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/2563 @HeartSaVioR Thank you, I fixed the indentation. ---

[GitHub] storm pull request #2576: [STORM-2976] Fix Supervisor HealthCheck validation...

2018-02-26 Thread kishorvpatil
GitHub user kishorvpatil opened a pull request: https://github.com/apache/storm/pull/2576 [STORM-2976] Fix Supervisor HealthCheck validations The couple of issues with Supervisor HealthCheck functionality. 1. `ClassCastException` while reading configuration. 2. The

[GitHub] storm pull request #2603: [STORM-3003] Adding Assignment caching to Nimbus

2018-03-22 Thread kishorvpatil
GitHub user kishorvpatil opened a pull request: https://github.com/apache/storm/pull/2603 [STORM-3003] Adding Assignment caching to Nimbus Since nimbus ( scheduler generates assignments) it can cache it instead of polling for it from ZK or other state manager. This would improve

[GitHub] storm issue #2603: [STORM-3003] Adding Assignment caching to Nimbus

2018-03-26 Thread kishorvpatil
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/2603 @d2r Thank you for the review. I have addressed the comments. The failure seems unrelated to my changes - the test passes on local environment. ---

[GitHub] storm issue #2603: [STORM-3003] Adding Assignment caching to Nimbus

2018-03-28 Thread kishorvpatil
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/2603 @HeartSaVioR Sure. Let me review #2433. In the meantime, I will create patch for 1.x-branch. ---

[GitHub] storm pull request #2603: [STORM-3003] Adding Assignment caching to Nimbus

2018-04-03 Thread kishorvpatil
Github user kishorvpatil closed the pull request at: https://github.com/apache/storm/pull/2603 ---

[GitHub] storm issue #2603: [STORM-3003] Adding Assignment caching to Nimbus

2018-04-03 Thread kishorvpatil
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/2603 @HeartSaVioR , I think changes in #2433 do take care of caching assignments within `InMemoryAssignmentBackend`, so the changes proposed in #2603 are no longer needed. I am closing this PR ---

[GitHub] storm pull request #2631: [STORM-3025] Optimize Cluster methods with Caching...

2018-04-10 Thread kishorvpatil
GitHub user kishorvpatil opened a pull request: https://github.com/apache/storm/pull/2631 [STORM-3025] Optimize Cluster methods with Caching to avoid loopy loops Optimizing the `Cluster` methods that are called from with high frequency calls to speed-up scheduling time on new

[GitHub] storm pull request #2631: [STORM-3025] Optimize Cluster methods with Caching...

2018-04-11 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/2631#discussion_r180800090 --- Diff: storm-server/src/main/java/org/apache/storm/scheduler/resource/normalization/NormalizedResourceRequest.java --- @@ -187,4 +187,29 @@ public

[GitHub] storm pull request #2631: [STORM-3025] Optimize Cluster methods with Caching...

2018-04-11 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/2631#discussion_r180811674 --- Diff: storm-server/src/main/java/org/apache/storm/scheduler/resource/normalization/NormalizedResourceRequest.java --- @@ -187,4 +187,29 @@ public

[GitHub] storm issue #2631: [STORM-3025] Optimize Cluster methods with Caching to avo...

2018-04-11 Thread kishorvpatil
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/2631 @agresch I have addressed the code review comments. ---

[GitHub] storm pull request #2638: [STORM-3034] Adding exception stacktrace for execu...

2018-04-19 Thread kishorvpatil
GitHub user kishorvpatil opened a pull request: https://github.com/apache/storm/pull/2638 [STORM-3034] Adding exception stacktrace for executor failures in worker You can merge this pull request into a Git repository by running: $ git pull https://github.com/kishorvpatil

[GitHub] storm pull request #2638: [STORM-3034] Adding exception stacktrace for execu...

2018-04-19 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/2638#discussion_r182843808 --- Diff: storm-client/src/jvm/org/apache/storm/utils/Utils.java --- @@ -361,6 +363,7 @@ public void run() { Time.sleep

[GitHub] storm pull request #2638: [STORM-3034] Adding exception stacktrace for execu...

2018-04-19 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/2638#discussion_r182847223 --- Diff: storm-client/src/jvm/org/apache/storm/utils/Utils.java --- @@ -109,6 +109,8 @@ import javax.security.auth.Subject

[GitHub] storm issue #2638: [STORM-3034] Adding exception stacktrace for executor fai...

2018-04-19 Thread kishorvpatil
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/2638 @srdo Moved the `LOG.error` before changing exception Cause. ---

[GitHub] storm pull request #2641: [STORM-3037] Lowering CheckStyle Violations across...

2018-04-22 Thread kishorvpatil
GitHub user kishorvpatil opened a pull request: https://github.com/apache/storm/pull/2641 [STORM-3037] Lowering CheckStyle Violations across all modules Below is the list of modules affected by this patch. The max allowed violations are going down by 17000+. | Module Name

[GitHub] storm issue #2638: [STORM-3034] Adding exception stacktrace for executor fai...

2018-04-23 Thread kishorvpatil
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/2638 @HeartSaVioR @srdo ., The issue in assuming that `InterruptedException` is only coming from external interruptions. Below is the actual example we noticed when Kafka Spout`TimeoutException` was

[GitHub] storm pull request #2641: [STORM-3037] Lowering CheckStyle Violations across...

2018-04-24 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/2641#discussion_r183771851 --- Diff: examples/storm-hdfs-examples/src/main/java/org/apache/storm/hdfs/bolt/SequenceFileTopology.java --- @@ -151,15 +145,15 @@ public void

[GitHub] storm pull request #2641: [STORM-3037] Lowering CheckStyle Violations across...

2018-04-24 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/2641#discussion_r183772336 --- Diff: examples/storm-hbase-examples/src/main/java/org/apache/storm/hbase/topology/LookupWordCount.java --- @@ -1,25 +1,19

[GitHub] storm pull request #2641: [STORM-3037] Lowering CheckStyle Violations across...

2018-04-26 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/2641#discussion_r184525138 --- Diff: examples/storm-perf/src/main/java/org/apache/storm/perf/utils/MetricsSample.java --- @@ -160,7 +159,7 @@ private static MetricsSample

[GitHub] storm pull request #2641: [STORM-3037] Lowering CheckStyle Violations across...

2018-04-26 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/2641#discussion_r184525490 --- Diff: examples/storm-starter/src/jvm/org/apache/storm/starter/BlobStoreAPIWordCountTopology.java --- @@ -38,176 +47,66 @@ import

[GitHub] storm issue #2638: [STORM-3034] Adding exception stacktrace for executor fai...

2018-04-30 Thread kishorvpatil
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/2638 @srdo , What I mean is there are many instances where kafka is wrapping actual exceptions into `InterruptedException`. I am not sure why/what is the objective, but only way to understand the

[GitHub] storm pull request #2651: [STORM-3054] Add Topology level configuration sock...

2018-04-30 Thread kishorvpatil
GitHub user kishorvpatil opened a pull request: https://github.com/apache/storm/pull/2651 [STORM-3054] Add Topology level configuration socket timeout for DRPC Invocation Client This patch fixes following this: - Add Topology level configuration socket timeout for DRPC

[GitHub] storm pull request #2638: [STORM-3034] Adding exception stacktrace for execu...

2018-05-07 Thread kishorvpatil
Github user kishorvpatil closed the pull request at: https://github.com/apache/storm/pull/2638 ---

[GitHub] storm issue #2638: [STORM-3034] Adding exception stacktrace for executor fai...

2018-05-07 Thread kishorvpatil
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/2638 @srdo, You are right. This was exception I noticed on old internal version of the code package. It is clearly not necessary in 2.x latest code. Let me close this patch. ---

[GitHub] storm pull request #2651: [STORM-3054] Add Topology level configuration sock...

2018-05-14 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/2651#discussion_r188107411 --- Diff: storm-client/src/jvm/org/apache/storm/drpc/ReturnResults.java --- @@ -103,21 +85,32 @@ public void execute(Tuple input

[GitHub] storm pull request #2651: [STORM-3054] Add Topology level configuration sock...

2018-05-14 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/2651#discussion_r188107350 --- Diff: storm-client/src/jvm/org/apache/storm/drpc/ReturnResults.java --- @@ -103,21 +85,32 @@ public void execute(Tuple input

[GitHub] storm issue #2651: [STORM-3054] Add Topology level configuration socket time...

2018-05-14 Thread kishorvpatil
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/2651 @HeartSaVioR @revans2 sorry for delay in addressing the review comments. ---

[GitHub] storm pull request #2700: [STORM-3093] Cache the storm id to executors mappi...

2018-06-04 Thread kishorvpatil
Github user kishorvpatil commented on a diff in the pull request: https://github.com/apache/storm/pull/2700#discussion_r192811084 --- Diff: storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java --- @@ -1333,6 +1336,21 @@ private AssignmentDistributionService

[GitHub] storm issue #2718: STORM-3103 allow nimbus to shutdown properly

2018-06-14 Thread kishorvpatil
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/2718 Good catch @agresch ---

[GitHub] storm issue #2660: STORM-3056: Add a test for quickly rebinding to a port

2018-06-14 Thread kishorvpatil
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/2660 @raghavgautam Can you please address the merge conflicts? ---

[GitHub] storm pull request #2744: [STORM-3132] Avoid NPE in the Values Constructor

2018-06-28 Thread kishorvpatil
GitHub user kishorvpatil opened a pull request: https://github.com/apache/storm/pull/2744 [STORM-3132] Avoid NPE in the Values Constructor `Values` construction could end up throwing NPE. You can merge this pull request into a Git repository by running: $ git pull https

  1   2   3   4   >