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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 user kishorvpatil commented on the issue:
https://github.com/apache/storm/pull/2345
@revans2 LGTM.
---
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 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 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 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 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 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 user kishorvpatil commented on the issue:
https://github.com/apache/storm/pull/2487
@srdo , Please review changes addressing the comments.
---
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 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 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 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 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 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 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 user kishorvpatil commented on the issue:
https://github.com/apache/storm/pull/2487
closing the PR as no longer needed for master branch.
---
Github user kishorvpatil closed the pull request at:
https://github.com/apache/storm/pull/2487
---
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 user kishorvpatil commented on the issue:
https://github.com/apache/storm/pull/2563
@HeartSaVioR Thank you, I fixed the indentation.
---
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 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 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 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 user kishorvpatil closed the pull request at:
https://github.com/apache/storm/pull/2603
---
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 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 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 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 user kishorvpatil commented on the issue:
https://github.com/apache/storm/pull/2631
@agresch I have addressed the code review comments.
---
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 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 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 user kishorvpatil commented on the issue:
https://github.com/apache/storm/pull/2638
@srdo Moved the `LOG.error` before changing exception Cause.
---
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 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 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 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 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 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 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 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 user kishorvpatil closed the pull request at:
https://github.com/apache/storm/pull/2638
---
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 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 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 user kishorvpatil commented on the issue:
https://github.com/apache/storm/pull/2651
@HeartSaVioR @revans2 sorry for delay in addressing the review comments.
---
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 user kishorvpatil commented on the issue:
https://github.com/apache/storm/pull/2718
Good catch @agresch
---
Github user kishorvpatil commented on the issue:
https://github.com/apache/storm/pull/2660
@raghavgautam Can you please address the merge conflicts?
---
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 - 100 of 372 matches
Mail list logo