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

[HeartSaVioR]

Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/2203
  
This patch adds new types of metrics on top of current metrics, so 
performance hit is expected. (20% drop looks like not small indeed.) We may 
want to consider not supporting smooth transition because of this.

Even we don't support smooth transition, we may want to comment all the old 
metrics and re-run benchmark to measure real performance hit.


---

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

[liu-zhaokun]

GitHub user liu-zhaokun opened a pull request:

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

[STORM-2773]If a drpcserver node in cluster is down,drpc cluster won't work 
if we don't modify the drpc.server configuration and restart the cluster


[https://issues.apache.org/jira/browse/STORM-2773](https://issues.apache.org/jira/browse/STORM-2773)

There is a cluster which includes three nodes named 
storm1,storm2,storm3.And there is a drpcserver in every node,a worker which has 
been started on strom1.When strom1 was down with hardware failure,my drpc 
topology won't work,when I send request from drpcclient.
As storm1 was down,so the worker will be restarted on another node,but it 
can't Initialize successfully because the call method of Adder will throw a 
RuntimeException,when drpcspout try to connect to storm1,so the worker will 
restart again.
In conclusion,If a drpcserver node in cluster is down,drpc cluster won't 
work until we modify the drpc.server configuration and restart the cluster,but 
in production,it's difficult to restart whole cluster.
So I think we should catch the RuntimeException and log it,and the drpc 
topology will work normally.

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

$ git pull https://github.com/liu-zhaokun/storm master1010

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

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


commit d3dec0d3ca80db518511bf26810d20035ce0ae81
Author: liuzhaokun 
Date:   2017-10-10T03:05:23Z

[STORM-2773] If a drpcserver node in cluster is down,drpc cluster won't 
work if we don't modify the drpc.server configuration and restart the cluster




---

[GitHub] storm pull request #2364: [STORM-2772] In the DRPCSpout class, when the fetc...

[httfighter]

GitHub user httfighter opened a pull request:

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

 [STORM-2772] In the DRPCSpout class, when the fetch from the DRPC server 
fails,the log should return to get the DRPC request failed instead of getting 
the DRPC result failed


[https://issues.apache.org/jira/browse/STORM-2772](https://issues.apache.org/jira/browse/STORM-2772)

In the DRPCSpout class, when the fetch from the DRPC server fails, the log 
error should return to get the DRPC request failed instead of getting the DRPC 
result failed.
for example, in line 216 of DRPCSpout class,
LOG.error("Not authorized to fetch DRPC result from DRPC server", aze);
this should be modified to 
LOG.error("Not authorized to fetch DRPC request from DRPC server", aze);

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

$ git pull https://github.com/httfighter/storm branch_4

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

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


commit cf91236b385065209cf50d86d2d3071a402e5cd8
Author: httfighter 
Date:   2017-10-10T02:51:06Z

In the DRPCSpout class, when the fetch from the DRPC server fails, the log 
should return to get the DRPC request failed instead of getting the DRPC result 
failed




---

Re: [DISCUSS] Release Storm 1.0.5 / 1.1.2

[Jungtaek Lim]

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 just take over and craft a new pull request? We
are waiting more than 2 months after requesting simple rebase (sadly it is
not done yet), which I don't think it's acceptable. That issue relates a
bug which we should handle it in time.
(The patch includes your work indeed.)

For STORM-2549, let's see someone could review in this week. I'll try to
get it too.

Then I think we can start release phase for Storm 1.1.2 and 1.2.0 at next
week. Opinions anyone?

Thanks,
Jungtaek Lim (HeartSaVioR)

2017년 10월 10일 (화) 오전 4:02, Stig Rohde Døssing 님이 작성:

> Maybe we would be better off releasing 1.1.2 as is, and postponing the
> other issues to 1.2.0? I don't think we should delay the fix for
> https://issues.apache.org/jira/browse/STORM-2682 for much longer.
>
> 2017-09-22 14:50 GMT+02:00 Alexandre Vermeerbergen <
> avermeerber...@gmail.com
> >:
>
> > Hello,
> >
> > I don't know if that help, but we're still waiting with lots of
> > expectations https://issues.apache.org/jira/browse/STORM-2648 with Storm
> > 1.2.0 !
> >
> > Best regards,
> > Alexandre Vermeerbergen
> >
> >
> > 2017-09-22 12:24 GMT+02:00 Jungtaek Lim :
> >
> > > Looks like three weeks went by from initiating the thread.
> > >
> > > I'm seeing some issues pending for review and all of them are regarding
> > > storm-kafka-client.
> > >
> > > Remaining issues are below:
> > >
> > > > Storm 1.1.2
> > >
> > > https://issues.apache.org/jira/browse/STORM-2549
> > > https://issues.apache.org/jira/browse/STORM-2607
> > > https://issues.apache.org/jira/browse/STORM-2666
> > >
> > > > Storm 1.2.0
> > >
> > > https://issues.apache.org/jira/browse/STORM-2648
> > >
> > > Please note that above issues are 'effectively' blocker for releases.
> > Like
> > > I said Storm 1.1.1 has critical issue which is fixed and will be
> > available
> > > at Storm 1.1.2, so at least I'd like to see the progress on Storm
> 1.1.2,
> > > and ideally with Storm 1.2.0 since there's only one issue left on epic.
> > >
> > > Please finish reviewing if you are in reviewing one or more of them.
> I'll
> > > try to start reviewing them but take some times since I'm not familiar
> > with
> > > that module.
> > >
> > > Thanks,
> > > Jungtaek Lim (HeartSaVioR)
> > >
> > > 2017년 8월 30일 (수) 오전 2:45, P. Taylor Goetz 님이 작성:
> > >
> > > > It looks to me like 1.0.5 is ready for a release candidate (still
> some
> > > > ongoing work for 1.1.2, but likely soon).
> > > >
> > > > Is there anything else we would want to include in 1.0.5 or should we
> > go
> > > > ahead with a release?
> > > >
> > > > -Taylor
> > > >
> > > > > On Aug 25, 2017, at 3:26 AM, Jungtaek Lim 
> wrote:
> > > > >
> > > > > Hi devs,
> > > > >
> > > > > We received a bug report (STORM-2682
> > > > > ) on Storm 1.0.4
> > and
> > > > > 1.1.1 which prevents Storm cluster from update. Personally it looks
> > > like
> > > > > pretty critical, and hopefully it is fixed now.
> > > > > So maybe we would like to have another bug fix releases quickly for
> > > > > affected 1.x version lines. What do you think?
> > > > >
> > > > > Also please enumerate the issues if you would want to include any
> bug
> > > fix
> > > > > issues to the new bug fix releases, so that we can create epic
> issues
> > > and
> > > > > track them to make releases happening sooner.
> > > > >
> > > > > Thanks,
> > > > > Jungtaek Lim (HeartSaVioR)
> > > >
> > > >
> > >
> >
>

[GitHub] storm pull request #2357: STORM-2666 (1.1.x)

[asfgit]

Github user asfgit closed the pull request at:

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


---

[GitHub] storm issue #2357: STORM-2666 (1.1.x)

[HeartSaVioR]

Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/2357
  
+1
The build ran fine locally with JDK 7. The Travis build failure came from 
stuck on AvroGenericRecordBoltTest in storm-hdfs with JDK 7, not in local but 
often in Travis CI build.


---

[GitHub] storm pull request #2356: STORM-2666 (1.x)

[asfgit]

Github user asfgit closed the pull request at:

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


---

[GitHub] storm issue #2356: STORM-2666 (1.x)

[HeartSaVioR]

Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/2356
  
+1
The build ran fine locally with JDK 7. The Travis build failure came from 
stuck on AvroGenericRecordBoltTest in storm-hdfs with JDK 7, not in local but 
often in Travis CI build.


---

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

[revans2]

Github user revans2 commented on the issue:

https://github.com/apache/storm/pull/2345
  
@kishorvpatil I think I addressed your review comments.  If you really want 
the log message to be info I can do it.


---

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

[revans2]

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

https://github.com/apache/storm/pull/2345#discussion_r143575205
  
--- Diff: 
storm-server/src/main/java/org/apache/storm/daemon/supervisor/Slot.java ---
@@ -104,9 +111,12 @@
 this.port = port;
 this.iSupervisor = iSupervisor;
 this.localState = localState;
+this.changingCallback = changingCallback;
 }
 }
-
+
+//TODO go through all of the state transitions and make sure we handle 
changingBlobs
+//TODO make sure to add in transition helpers that clean changingBlobs 
&& pendingChangeingBlobs for not the current topology
--- End diff --

Yes I left in a lot of TODOs #2363 cleans them up, but I will try to pull 
back what I can here.

In this particular case I did go through the transitions and I think I have 
it all covered.


---

Storm pubsub spout? does one exist?

[Tom Raney]

I asked this on the user list, but maybe it's a better question for the dev
list.

Has anyone built one to work with the Google cloud pubsub service?

Not seeing one here: https://github.com/apache/storm/tree/master/external

There is a raw beta java pubsub client here:
https://github.com/GoogleCloudPlatform/google-cloud-java/tree/master/google-cloud-pubsub

It won't be hard to build one, but I'm curious to hear if people have
avoided it for a reason, or maybe it just hasn't come up yet.

Thanks!

Regards,

Tom

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

[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 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.localizer;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.LinkOption;
+import java.nio.file.Path;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import org.apache.storm.blobstore.BlobStore;
+import org.apache.storm.blobstore.ClientBlobStore;
+import org.apache.storm.generated.AuthorizationException;
+import org.apache.storm.generated.KeyNotFoundException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Represents a blob that is cached locally on disk by the supervisor.
+ */
+public abstract class LocallyCachedBlob {
+private static final Logger LOG = 
LoggerFactory.getLogger(LocallyCachedBlob.class);
+public static final long NOT_DOWNLOADED_VERSION = -1;
+// A callback that does nothing.
+private static final BlobChangingCallback NOOP_CB = (assignment, port, 
resource, go) -> {};
+
+private long lastUsed = System.currentTimeMillis();
+private final Map references 
= new HashMap<>();
+private final String blobDescription;
+private final String blobKey;
+private CompletableFuture doneUpdating = null;
+
+/**
+ * Create a new LocallyCachedBlob.
+ * @param blobDescription a description of the blob this represents.  
Typically it should at least be the blob key, but ideally also
+ * include if it is an archive or not, what user or topology it is 
for, or if it is a storm.jar etc.
+ */
+protected LocallyCachedBlob(String blobDescription, String blobKey) {
+this.blobDescription = blobDescription;
+this.blobKey = blobKey;
+}
+
+/**
+ * Get the version of the blob cached locally.  If the version is 
unknown or it has not been downloaded NOT_DOWNLOADED_VERSION
+ * should be returned.
+ * PRECONDITION: this can only be called with a lock on this instance 
held.
+ */
+public abstract long getLocalVersion();
+
+/**
+ * Get the version of the blob in the blob store.
+ * PRECONDITION: this can only be called with a lock on this instance 
held.
+ */
+public abstract long getRemoteVersion(ClientBlobStore store) throws 
KeyNotFoundException, AuthorizationException;
+
+/**
+ * Download the latest version to a temp location. This may also 
include unzipping some or all of the data to a temp location.
+ * PRECONDITION: this can only be called with a lock on this instance 
held.
+ * @param store the store to us to download the data.
+ * @return the version that was downloaded.
+ */
+public abstract long downloadToTempLocation(ClientBlobStore store) 
throws IOException, KeyNotFoundException, AuthorizationException;
+
+/**
+ * Commit the new version and make it available for the end user.
+ * PRECONDITION: uncompressToTempLocationIfNeeded will have been 
called.
+ * PRECONDITION: this can only be called with a lock on this instance 
held.
+ * @param version the version of the blob to commit.
+ */
+public abstract void commitNewVersion(long version) throws IOException;
+
+/**
+ * Clean up any temporary files.  This will be called after updating a 
blob, either successfully or if an error has occured.
+ * The goal is to find any files that may be left over and remove them 
so space is not leaked.
+ * PRECONDITION: this can only be called with a lock on this 

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

[revans2]

GitHub user revans2 opened a pull request:

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

 STORM-2759: Let users indicate if a blob should restart a worker 

This is based off of the changes in #2345.  They are all in the first 
commit.  The second commit has all of the newer changes for this JIRA in it.

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

$ git pull https://github.com/revans2/incubator-storm STORM-2759

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

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






---

Re: [DISCUSS] Release Storm 1.0.5 / 1.1.2

[Stig Rohde Døssing]

Maybe we would be better off releasing 1.1.2 as is, and postponing the
other issues to 1.2.0? I don't think we should delay the fix for
https://issues.apache.org/jira/browse/STORM-2682 for much longer.

2017-09-22 14:50 GMT+02:00 Alexandre Vermeerbergen :

> Hello,
>
> I don't know if that help, but we're still waiting with lots of
> expectations https://issues.apache.org/jira/browse/STORM-2648 with Storm
> 1.2.0 !
>
> Best regards,
> Alexandre Vermeerbergen
>
>
> 2017-09-22 12:24 GMT+02:00 Jungtaek Lim :
>
> > Looks like three weeks went by from initiating the thread.
> >
> > I'm seeing some issues pending for review and all of them are regarding
> > storm-kafka-client.
> >
> > Remaining issues are below:
> >
> > > Storm 1.1.2
> >
> > https://issues.apache.org/jira/browse/STORM-2549
> > https://issues.apache.org/jira/browse/STORM-2607
> > https://issues.apache.org/jira/browse/STORM-2666
> >
> > > Storm 1.2.0
> >
> > https://issues.apache.org/jira/browse/STORM-2648
> >
> > Please note that above issues are 'effectively' blocker for releases.
> Like
> > I said Storm 1.1.1 has critical issue which is fixed and will be
> available
> > at Storm 1.1.2, so at least I'd like to see the progress on Storm 1.1.2,
> > and ideally with Storm 1.2.0 since there's only one issue left on epic.
> >
> > Please finish reviewing if you are in reviewing one or more of them. I'll
> > try to start reviewing them but take some times since I'm not familiar
> with
> > that module.
> >
> > Thanks,
> > Jungtaek Lim (HeartSaVioR)
> >
> > 2017년 8월 30일 (수) 오전 2:45, P. Taylor Goetz 님이 작성:
> >
> > > It looks to me like 1.0.5 is ready for a release candidate (still some
> > > ongoing work for 1.1.2, but likely soon).
> > >
> > > Is there anything else we would want to include in 1.0.5 or should we
> go
> > > ahead with a release?
> > >
> > > -Taylor
> > >
> > > > On Aug 25, 2017, at 3:26 AM, Jungtaek Lim  wrote:
> > > >
> > > > Hi devs,
> > > >
> > > > We received a bug report (STORM-2682
> > > > ) on Storm 1.0.4
> and
> > > > 1.1.1 which prevents Storm cluster from update. Personally it looks
> > like
> > > > pretty critical, and hopefully it is fixed now.
> > > > So maybe we would like to have another bug fix releases quickly for
> > > > affected 1.x version lines. What do you think?
> > > >
> > > > Also please enumerate the issues if you would want to include any bug
> > fix
> > > > issues to the new bug fix releases, so that we can create epic issues
> > and
> > > > track them to make releases happening sooner.
> > > >
> > > > Thanks,
> > > > Jungtaek Lim (HeartSaVioR)
> > >
> > >
> >
>

[GitHub] storm pull request #2361: STORM-2698: Upgrade to latest Mockito and Hamcrest

[asfgit]

Github user asfgit closed the pull request at:

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


---

[GitHub] storm issue #2361: STORM-2698: Upgrade to latest Mockito and Hamcrest

[srdo]

Github user srdo commented on the issue:

https://github.com/apache/storm/pull/2361
  
Thanks for review @HeartSaVioR. Merged to master.


---

[GitHub] storm issue #2362: Improve mention of aggregate() in the partitionAggregate(...

[srdo]

Github user srdo commented on the issue:

https://github.com/apache/storm/pull/2362
  
Merged to master, 1.x and 1.1.x.


---

[GitHub] storm issue #2362: Improve mention of aggregate() in the partitionAggregate(...

[srdo]

Github user srdo commented on the issue:

https://github.com/apache/storm/pull/2362
  
Thanks, +1


---

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

[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() {
 return "{ " + topoId + ": " + request + " }";
 }
 }
-
+
+/**
+ * Holds the information about a blob that is changing.
+ */
+static class BlobChangeing {
--- End diff --

Does this need to be `BlobChanging`?


---

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

[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;
 this.iSupervisor = iSupervisor;
 this.localState = localState;
+this.changingCallback = changingCallback;
 }
 }
-
+
+//TODO go through all of the state transitions and make sure we handle 
changingBlobs
+//TODO make sure to add in transition helpers that clean changingBlobs 
&& pendingChangeingBlobs for not the current topology
--- End diff --

??


---

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

[revans2]

Github user revans2 commented on the issue:

https://github.com/apache/storm/pull/2345
  
For anyone interested I just rebased to fix a very minor merge conflict.


---

[GitHub] storm pull request #2362: Improve mention of aggregate() in the partitionAgg...

[F30]

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

https://github.com/apache/storm/pull/2362#discussion_r143400447
  
--- Diff: docs/Trident-API-Overview.md ---
@@ -484,7 +484,7 @@ public class Count implements CombinerAggregator {
 }
 ```
 
-The benefits of CombinerAggregators are seen when you use them with the 
aggregate method instead of partitionAggregate. In that case, Trident 
automatically optimizes the computation by doing partial aggregations before 
transferring tuples over the network.
+CombinerAggregators offer high efficiency when used with the aggregate 
method instead of partitionAggregate (see below).
--- End diff --

I have added a link now.


---