[GitHub] storm pull request #2367: STORM-2607: Storm-kafka-client never commits the l...

2017-10-10 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/2367#discussion_r143914514 --- Diff: external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/internal/OffsetManager.java --- @@ -77,57 +78,58 @@ public void

[GitHub] storm pull request #2366: [STORM-2686] Add locality awareness to LoadAwareSh...

2017-10-10 Thread Ethanlm
Github user Ethanlm commented on a diff in the pull request: https://github.com/apache/storm/pull/2366#discussion_r143865842 --- Diff: storm-client/src/jvm/org/apache/storm/grouping/LoadAwareShuffleGrouping.java --- @@ -50,10 +69,28 @@ volatile int[] choices;

[GitHub] storm pull request #2368: STORM-2771: By default don't run any tests as inte...

2017-10-10 Thread revans2
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2368 STORM-2771: By default don't run any tests as integration tests You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm

[GitHub] storm pull request #2366: [STORM-2686] Add locality awareness to LoadAwareSh...

2017-10-10 Thread HeartSaVioR
Github user HeartSaVioR commented on a diff in the pull request: https://github.com/apache/storm/pull/2366#discussion_r143862027 --- Diff: storm-client/src/jvm/org/apache/storm/grouping/LoadAwareShuffleGrouping.java --- @@ -50,10 +69,28 @@ volatile int[] choices;

[GitHub] storm issue #2366: [STORM-2686] Add locality awareness to LoadAwareShuffleGr...

2017-10-10 Thread Ethanlm
Github user Ethanlm commented on the issue: https://github.com/apache/storm/pull/2366 Thanks! Will do ---

[GitHub] storm issue #2366: [STORM-2686] Add locality awareness to LoadAwareShuffleGr...

2017-10-10 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2366 @Ethanlm Everything looks good I am +1 on this change. I would love to see more numbers to back this up, but as of right now it looks really good. ---

[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 issue #2366: [STORM-2686] Add locality awareness to LoadAwareShuffleGr...

2017-10-10 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2366 I ran some small performance tests and things are looking really good. Have you run any tests? How did you come up with the 80% config for growing in size, and 20% for shrinking? I would

[GitHub] storm pull request #2366: [STORM-2686] Add locality awareness to LoadAwareSh...

2017-10-10 Thread Ethanlm
Github user Ethanlm commented on a diff in the pull request: https://github.com/apache/storm/pull/2366#discussion_r143837500 --- Diff: storm-client/src/jvm/org/apache/storm/grouping/LoadAwareShuffleGrouping.java --- @@ -50,10 +61,28 @@ volatile int[] choices;

Re: [DISCUSS] Release Storm 1.0.5 / 1.1.2

2017-10-10 Thread Stig Rohde Døssing
Thanks Jungtaek, that sounds like a good plan. Here's the new PR for 2607 https://github.com/apache/storm/pull/2367. Beginning release next week sounds good to me. 2017-10-10 17:42 GMT+02:00 Arun Mahadevan : > +1 for addressing the pending reviews and getting 1.2.0 out soon. >

[GitHub] storm issue #2181: [STORM-2607] Offset consumer + 1

2017-10-10 Thread srdo
Github user srdo commented on the issue: https://github.com/apache/storm/pull/2181 @tiodollar Since there is still a conflict here, and we'd like this to go in 1.1.2 which we hope to release before too long, I've resolved the conflicts and addressed the review comments. There's a PR

[GitHub] storm pull request #2367: STORM-2607: Storm-kafka-client never commits the l...

2017-10-10 Thread srdo
GitHub user srdo opened a pull request: https://github.com/apache/storm/pull/2367 STORM-2607: Storm-kafka-client never commits the last message on a partition. This finishes up https://github.com/apache/storm/pull/2181, which has been blocked on a rebase for a while. It was

[GitHub] storm pull request #2366: [STORM-2686] Add locality awareness to LoadAwareSh...

2017-10-10 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2366#discussion_r143829089 --- Diff: storm-client/src/jvm/org/apache/storm/grouping/LoadAwareShuffleGrouping.java --- @@ -50,10 +61,28 @@ volatile int[] choices;

[GitHub] storm pull request #2366: [STORM-2686] Add locality awareness to LoadAwareSh...

2017-10-10 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2366#discussion_r143828158 --- Diff: storm-client/src/jvm/org/apache/storm/task/WorkerTopologyContext.java --- @@ -34,6 +36,8 @@ private String _pidDir; Map

[GitHub] storm pull request #2366: [STORM-2686] Add locality awareness to LoadAwareSh...

2017-10-10 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2366#discussion_r143826667 --- Diff: storm-client/src/jvm/org/apache/storm/grouping/LoadAwareShuffleGrouping.java --- @@ -20,14 +20,20 @@ import

[GitHub] storm issue #2366: [STORM-2686] Add locality awareness to LoadAwareShuffleGr...

2017-10-10 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2366 @Ethanlm You have some NPEs in the LoadAwareShuffleGroupingTest ---

[GitHub] storm issue #2363: STORM-2759: Let users indicate if a blob should restart a...

2017-10-10 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2363 #2345 was merged so I rebased to make it more clear the new changes. I actually delete code now :). ---

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

2017-10-10 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/2345 ---

[GitHub] storm pull request #2365: [STORM-2773]If a drpcserver node in cluster is dow...

2017-10-10 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2365#discussion_r143807488 --- Diff: storm-client/src/jvm/org/apache/storm/drpc/DRPCSpout.java --- @@ -105,13 +105,19 @@ public Adder(String server, int port, Map

[GitHub] storm issue #2365: [STORM-2773]If a drpcserver node in cluster is down,drpc ...

2017-10-10 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2365 You are also going to need to fix the checkstyle errors. ---

[GitHub] storm pull request #2365: [STORM-2773]If a drpcserver node in cluster is dow...

2017-10-10 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2365#discussion_r143807656 --- Diff: storm-client/src/jvm/org/apache/storm/drpc/DRPCSpout.java --- @@ -105,13 +105,19 @@ public Adder(String server, int port, Map

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

Re: [DISCUSS] Release Storm 1.0.5 / 1.1.2

2017-10-10 Thread Arun Mahadevan
+1 for addressing the pending reviews and getting 1.2.0 out soon. On 10/10/17, 6:14 AM, "Jungtaek Lim" wrote: >Stig, > >Let's just handle all the issues pending Storm 1.1.2. For pending issues on >Storm 1.2.0, I already handled all the things. > >For STORM-2607, could you

[GitHub] storm issue #2203: STORM-2153: New Metrics Reporting API

2017-10-10 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/2203 @revans2 I need to recheck which thing (meter or counter) they are using, but JStorm also doesn’t sample metrics other than histogram. Unless they did custom optimization, IMHO it should be

[GitHub] storm issue #2203: STORM-2153: New Metrics Reporting API

2017-10-10 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2203 @HeartSaVioR I agree that if we cannot find a better way we may not want to support a smooth transition for the metrics, but that also means that we cannot put it into 1.x. But I think the

[GitHub] storm pull request #2270: [STORM-2686] Add Locality Aware Shuffle Grouping

2017-10-10 Thread Ethanlm
Github user Ethanlm closed the pull request at: https://github.com/apache/storm/pull/2270 ---

[GitHub] storm issue #2270: [STORM-2686] Add Locality Aware Shuffle Grouping

2017-10-10 Thread Ethanlm
Github user Ethanlm commented on the issue: https://github.com/apache/storm/pull/2270 This https://github.com/apache/storm/pull/2366 addes locality awareness to LoadAwareShuffleGrouping. Closing this one. ---

[GitHub] storm pull request #2366: [STORM-2686] Add locality awareness to LoadAwareSh...

2017-10-10 Thread Ethanlm
GitHub user Ethanlm opened a pull request: https://github.com/apache/storm/pull/2366 [STORM-2686] Add locality awareness to LoadAwareShuffleGrouping A redesign of https://github.com/apache/storm/pull/2270 . This adds the locality awareness to LoadAwareShuffleGrouping. It applies