Re: [DISCUSS] [Storm-Kafka] - Maintenance, Branch Support, and Deprecation Plans for storm-kafka and storm-kaka-client

2018-02-12 Thread Jungtaek Lim
I forgot the one, but this is great time to revisit this, since we got
resolved many storm-kafka-client issues hence feeling it as fairly stable.

I'm +1 on deprecate storm-kafka at 1.x version lines and remove it at
2.0.0. Do we want to have explicit VOTE to receive more opinions here?

Thanks,
Jungtaek Lim (HeartSaVioR)

2017년 7월 20일 (목) 오전 5:10, P. Taylor Goetz 님이 작성:

> +1 I’m fine with taking this approach.
>
> -Taylor
>
> > On Jul 19, 2017, at 2:04 PM, Stig Rohde Døssing 
> wrote:
> >
> > +1 for removing storm-kafka from master, since we shouldn't encourage
> > people to use a component that won't work on new Kafka versions. As you
> > both mentioned, the 1.x version of storm-kafka should still be usable on
> a
> > 2.0 cluster, so it will still be available in case people need it. A wiki
> > page for tracking current missing pieces for storm-kafka-client sounds
> good.
> >
> > 2017-07-19 19:09 GMT+02:00 Harsha :
> >
> >> +1 on moving away from storm-kafka for Storm 2.0. For existing users we
> >> can provide any critical bug fixes and provide it as part of 1.x
> >> releases. They can still use the existing 1.x storm-kafka against 2.0.
> >> Since kafka itself is moving away from older APIs continuing two
> >> versions of kafka connector doesnt’ make sense and honestly splits the
> >> usage which doesn’t give us any feedback on new storm-kafka-client.
> >> Thanks,
> >> Harsha
> >>
> >> On Wed, Jul 19, 2017, at 09:20 AM, Hugo Da Cruz Louro wrote:
> >>> Hi,
> >>>
> >>> The goal of this email is to summarize and unify the discussion started
> >>> across several email threads (Storm 2.0
> >>> Roadmap >> 5BDISCUSS%5D+Storm+2.0+Roadmap%22>,
> >>> 1.1.1 Release
> >>> Planning >> subj=Release+Planning+for+1+1+1+and+others+>,
> >>> Lag
> >>> Issues >> subj=Lag+issues+using+Storm+1+1+1+latest+build+with+
> >> StormKafkaClient+1+1+1+vs+old+StormKafka+spouts>)
> >>> concerning the maintenance, branch support, and eventual deprecation of
> >>> storm-kafka and storm-kafka-client.
> >>>
> >>> It was proposed in an earlier
> >>> discussion >> 22%5BDISCUSS%5D+Storm+2.0+Roadmap%22>
> >>> the plan to deprecate storm-kafka in prol of storm-kafka-client. To
> >>> clarify, the idea is not to completely eliminate storm-kafka, but
> rather
> >>> keep supporting it in the 1.x-branch, while removing it from master
> (i.e.
> >>> Storm 2.0 onwards). That is, storm-kafka-client will then become the
> only
> >>> Storm Kafka option available for Storm 2.0 onwards, given that we have
> >>> enough confidence in its stability by the time of the Storm 2.0
> release.
> >>>
> >>> The main reason for this proposal is the fact that the Kafka community
> >>> agreed >> KIP-109:+Old+Consumer+Deprecation>
> >>> to deprecate the old consumer APIs starting in version 0.10.2, and will
> >>> remove them in the next major version (0.12). This implies that
> >>> storm-kafka will not work for Kafka 0.12 onwards. Important features
> >>> missing in the old Kafka consumer are: security, new message format,
> and
> >>> fetching offsets based on time stamp (KIP-79).
> >>>
> >>> In earlier discussions the Storm community has shown concerns about the
> >>> performance and stability of the storm-kafka-client. Those concerns are
> >>> valid and were mirrored by the Kafka community in their early
> deprecation
> >>> discussions. I align with what was said in the Kafka
> >>> discussion: the
> >>> storm-kafka-client has bugs, but so does storm-kafka, and all the
> >>> development is currently going into storm-kafka-client, which will be
> >>> even more prevalent in face of Kafka discontinuing the old consumer
> >>> API’s. The only way to stabilize a complex component such as
> >>> storm-kafka-client is to test it extensively in all its variants, which
> >>> inevitably comes from users using it. Furthermore, removing storm-kafka
> >>> from Storm 2.0 does not prevent users from still referring to
> storm-kafka
> >>> version 1.x in their topologies.
> >>>
> >>> I did a quick analysis of the JIRA issues for storm-kafka and
> >>> storm-kafka-client [1].  As of July 11 there are 22 open or in-progress
> >>> bugs for storm-kafka (1 blocker) and 15 for storm-kafka-client.
> >>>
> >>> The recent refactoring around manual partition assignment should solve
> a
> >>> lot of edge case bugs that occurred during rebalance. There are also a
> >>> few open pull requests for Trident  and fixing some internal state
> >>> details such as maxUncommittedOffsets, topic compaction, etc.
> >>> Nevertheless, there are several areas that need to be addressed to
> >>> stabilize and improve storm-kafka-client. Similarly to what was 

[DISCUSS] consider EOL for version lines

2018-02-12 Thread Jungtaek Lim
Hi devs,

I've noticed that we are providing 4 different version lines (1.1.x, 1.0.x,
0.10.x, 0.9.x) in download page, and I expect we will add one more for
1.2.0. Moreover, we have one more develop version line (2.0.0 - master)
which most of development happens there.

Recently we're releasing 3 version lines (1.0.6 / 1.1.2 / 1.2.0)
simultaneously and it took heavy effort to track all the RCs and verify all
of them. I guess release manager would take more overhead of releasing, and
it doesn't make sense for me if we continue maintaining all of them.

Ideally I'd like to propose maintaining three version lines: 2.0.0 (next
major) / 1.3.0 (next minor - may not happen) / 1.2.1 (next bugfix) and
making others EOL (that respects semantic versioning and even other
projects tend to maintain only two version lines), but if someone feels too
aggressive, I propose at least we explicitly announce EOL to 0.x version
lines and get rid of any supports (downloads) for them.

Would like to hear your opinion.

Thanks,
Jungtaek Lim (HeartSaVioR)


[GitHub] storm issue #2554: STORM-2939 add WorkerMetricsProcessor interface

2018-02-12 Thread agresch
Github user agresch commented on the issue:

https://github.com/apache/storm/pull/2554
  
That is correct.  With the current default RocksDB implementation, we would 
have workers forward to the Supervisor, and the Supervisor metric processor 
would be NimbusMetricProcessor - sending them to Nimbus for insertion into 
RocksDB.


---


[GitHub] storm issue #2554: STORM-2939 add WorkerMetricsProcessor interface

2018-02-12 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue:

https://github.com/apache/storm/pull/2554
  
@agresch 

> We're planning on getting the metrics forwarded from the workers to the 
Supervisor and inserting from HBase there (or forwarding to Nimbus for 
RocksDB). In the non-HBase implementation, we need to be able to have a path 
from the workers to Nimbus.

Once you intend WorkerMetricsProcessor to be run in Supervisor, non-HBase 
implementation should also forward metrics to Supervisor to delegate 
forwarding, hence the path would be worker -> Supervisor -> Nimbus, right? Just 
be clear about this.


---


[GitHub] storm-site pull request #2: Update dependencies

2018-02-12 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/storm-site/pull/2


---


Re: [VOTE] Release Apache Storm 1.0.6 (rc3)

2018-02-12 Thread P. Taylor Goetz
This vote is now closed and passes with 3 binding +1 votes, and no 0 or -1 
votes.

Vote tally:

+1:
- Bobby Evans
- Jungtaek Lim
- P. Taylor Goetz

I will release the staged artifacts and announce the release after 24 hours.

-Taylor

> On Feb 8, 2018, at 2:22 PM, P. Taylor Goetz  wrote:
> 
> This is a call to vote on releasing Apache Storm 1.0.6 (rc3)
> 
> Full list of changes in this release:
> 
> https://dist.apache.org/repos/dist/dev/storm/apache-storm-1.0.6-rc3/RELEASE_NOTES.html
> 
> The tag/commit to be voted upon is v1.0.6:
> 
> https://git-wip-us.apache.org/repos/asf?p=storm.git;a=tree;h=e68365f9f947ddd1794b2edef2149fdfaa1590a2;hb=7993db01580ce62d44866dc00e0a7266984638d0
> 
> The source archive being voted upon can be found here:
> 
> https://dist.apache.org/repos/dist/dev/storm/apache-storm-1.0.6-rc3/apache-storm-1.0.6-src.tar.gz
> 
> Other release files, signatures and digests can be found here:
> 
> https://dist.apache.org/repos/dist/dev/storm/apache-storm-1.0.6-rc3/
> 
> The release artifacts are signed with the following key:
> 
> https://git-wip-us.apache.org/repos/asf?p=storm.git;a=blob_plain;f=KEYS;hb=22b832708295fa2c15c4f3c70ac0d2bc6fded4bd
> 
> The Nexus staging repository for this release is:
> 
> https://repository.apache.org/content/repositories/orgapachestorm-1060
> 
> Please vote on releasing this package as Apache Storm 1.0.6.
> 
> When voting, please list the actions taken to verify the release.
> 
> This vote will be open for at least 72 hours.
> 
> [ ] +1 Release this package as Apache Storm 1.0.6
> [ ]  0 No opinion
> [ ] -1 Do not release this package because...
> 
> Thanks to everyone who contributed to this release.
> 
> -Taylor



signature.asc
Description: Message signed with OpenPGP


[RESULT] [VOTE] Release Apache Storm 1.1.2 (rc3)

2018-02-12 Thread P. Taylor Goetz
This vote is now closed and passes with 3 binding +1 votes and no -1 or 0 votes.

Vote tally:

+1:
- Bobby Evans
- Jungtaek Lim
- P. Taylor Goetz

I will release the staged artifacts and announce the release after 24 hours.

-Taylor

> On Feb 8, 2018, at 12:48 PM, P. Taylor Goetz  wrote:
> 
> This is a call to vote on releasing Apache Storm 1.1.2 (rc3)
> 
> Full list of changes in this release:
> 
> https://dist.apache.org/repos/dist/dev/storm/apache-storm-1.1.2-rc3/RELEASE_NOTES.html
> 
> The tag/commit to be voted upon is v1.1.2:
> 
> https://git-wip-us.apache.org/repos/asf?p=storm.git;a=tree;h=3aab00d053d3576d3f8a37bcc3c7e51072ead22e;hb=0bb7a66a9b26ad96afc81b27dd45f93ae8969c44
> 
> The source archive being voted upon can be found here:
> 
> https://dist.apache.org/repos/dist/dev/storm/apache-storm-1.1.2-rc3/apache-storm-1.1.2-src.tar.gz
> 
> Other release files, signatures and digests can be found here:
> 
> https://dist.apache.org/repos/dist/dev/storm/apache-storm-1.1.2-rc3/
> 
> The release artifacts are signed with the following key:
> 
> https://git-wip-us.apache.org/repos/asf?p=storm.git;a=blob_plain;f=KEYS;hb=22b832708295fa2c15c4f3c70ac0d2bc6fded4bd
> 
> The Nexus staging repository for this release is:
> 
> https://repository.apache.org/content/repositories/orgapachestorm-1059
> 
> Please vote on releasing this package as Apache Storm 1.1.2.
> 
> When voting, please list the actions taken to verify the release.
> 
> This vote will be open for at least 72 hours.
> 
> [ ] +1 Release this package as Apache Storm 1.1.2
> [ ]  0 No opinion
> [ ] -1 Do not release this package because...
> 
> Thanks to everyone who contributed to this release.
> 
> -Taylor



signature.asc
Description: Message signed with OpenPGP


Re: [VOTE] Release Apache Storm 1.1.2 (rc3)

2018-02-12 Thread P. Taylor Goetz
+1 (binding)

-Taylor

> On Feb 8, 2018, at 12:48 PM, P. Taylor Goetz  wrote:
> 
> This is a call to vote on releasing Apache Storm 1.1.2 (rc3)
> 
> Full list of changes in this release:
> 
> https://dist.apache.org/repos/dist/dev/storm/apache-storm-1.1.2-rc3/RELEASE_NOTES.html
> 
> The tag/commit to be voted upon is v1.1.2:
> 
> https://git-wip-us.apache.org/repos/asf?p=storm.git;a=tree;h=3aab00d053d3576d3f8a37bcc3c7e51072ead22e;hb=0bb7a66a9b26ad96afc81b27dd45f93ae8969c44
> 
> The source archive being voted upon can be found here:
> 
> https://dist.apache.org/repos/dist/dev/storm/apache-storm-1.1.2-rc3/apache-storm-1.1.2-src.tar.gz
> 
> Other release files, signatures and digests can be found here:
> 
> https://dist.apache.org/repos/dist/dev/storm/apache-storm-1.1.2-rc3/
> 
> The release artifacts are signed with the following key:
> 
> https://git-wip-us.apache.org/repos/asf?p=storm.git;a=blob_plain;f=KEYS;hb=22b832708295fa2c15c4f3c70ac0d2bc6fded4bd
> 
> The Nexus staging repository for this release is:
> 
> https://repository.apache.org/content/repositories/orgapachestorm-1059
> 
> Please vote on releasing this package as Apache Storm 1.1.2.
> 
> When voting, please list the actions taken to verify the release.
> 
> This vote will be open for at least 72 hours.
> 
> [ ] +1 Release this package as Apache Storm 1.1.2
> [ ]  0 No opinion
> [ ] -1 Do not release this package because...
> 
> Thanks to everyone who contributed to this release.
> 
> -Taylor



signature.asc
Description: Message signed with OpenPGP


Re: [VOTE] Release Apache Storm 1.0.6 (rc3)

2018-02-12 Thread P. Taylor Goetz
+1 (binding)

-Taylor

> On Feb 8, 2018, at 2:22 PM, P. Taylor Goetz  wrote:
> 
> This is a call to vote on releasing Apache Storm 1.0.6 (rc3)
> 
> Full list of changes in this release:
> 
> https://dist.apache.org/repos/dist/dev/storm/apache-storm-1.0.6-rc3/RELEASE_NOTES.html
> 
> The tag/commit to be voted upon is v1.0.6:
> 
> https://git-wip-us.apache.org/repos/asf?p=storm.git;a=tree;h=e68365f9f947ddd1794b2edef2149fdfaa1590a2;hb=7993db01580ce62d44866dc00e0a7266984638d0
> 
> The source archive being voted upon can be found here:
> 
> https://dist.apache.org/repos/dist/dev/storm/apache-storm-1.0.6-rc3/apache-storm-1.0.6-src.tar.gz
> 
> Other release files, signatures and digests can be found here:
> 
> https://dist.apache.org/repos/dist/dev/storm/apache-storm-1.0.6-rc3/
> 
> The release artifacts are signed with the following key:
> 
> https://git-wip-us.apache.org/repos/asf?p=storm.git;a=blob_plain;f=KEYS;hb=22b832708295fa2c15c4f3c70ac0d2bc6fded4bd
> 
> The Nexus staging repository for this release is:
> 
> https://repository.apache.org/content/repositories/orgapachestorm-1060
> 
> Please vote on releasing this package as Apache Storm 1.0.6.
> 
> When voting, please list the actions taken to verify the release.
> 
> This vote will be open for at least 72 hours.
> 
> [ ] +1 Release this package as Apache Storm 1.0.6
> [ ]  0 No opinion
> [ ] -1 Do not release this package because...
> 
> Thanks to everyone who contributed to this release.
> 
> -Taylor



signature.asc
Description: Message signed with OpenPGP


[RESULT] [VOTE] Release Apache Storm 1.2.0 (rc4)

2018-02-12 Thread P. Taylor Goetz
This vote is now closed and passes with 4 binding +1 votes, 1 non-binding +1, 
and no 0 or -1 votes.

Vote tally (* indicates a binding vote):

+1:
- Jungtaek Lim*
- Satish Duggana*
- Arun Iyer*
- Alexandre Vermeerbergen
- Bobby Evans*

I will release the staged artifacts and announce the release after the 24 hour 
waiting period.

-Taylor

> On Feb 7, 2018, at 5:24 PM, P. Taylor Goetz  wrote:
> 
> This is a call to vote on releasing Apache Storm 1.2.0 (rc4)
> 
> Note that the only difference between rc4 and rc3 is the fix for 
> https://issues.apache.org/jira/browse/STORM-2942 
> .
> 
> Full list of changes in this release:
> 
> https://dist.apache.org/repos/dist/dev/storm/apache-storm-1.2.0-rc4/RELEASE_NOTES.html
>  
> 
> 
> The tag/commit to be voted upon is v1.2.0:
> 
> https://git-wip-us.apache.org/repos/asf?p=storm.git;a=tree;h=cef4d49e222e53656f38c40d754d4f41799cd9a9;hb=2a0097f9a20b9df494caadb87c87d4e4db01a7ed
>  
> 
> 
> The source archive being voted upon can be found here:
> 
> https://dist.apache.org/repos/dist/dev/storm/apache-storm-1.2.0-rc4/apache-storm-1.2.0-src.tar.gz
>  
> 
> 
> Other release files, signatures and digests can be found here:
> 
> https://dist.apache.org/repos/dist/dev/storm/apache-storm-1.2.0-rc4/ 
> 
> 
> The release artifacts are signed with the following key:
> 
> https://git-wip-us.apache.org/repos/asf?p=storm.git;a=blob_plain;f=KEYS;hb=22b832708295fa2c15c4f3c70ac0d2bc6fded4bd
>  
> 
> 
> The Nexus staging repository for this release is:
> 
> https://repository.apache.org/content/repositories/orgapachestorm-1058 
> 
> 
> Please vote on releasing this package as Apache Storm 1.2.0.
> 
> When voting, please list the actions taken to verify the release.
> 
> This vote will be open for at least 72 hours.
> 
> [ ] +1 Release this package as Apache Storm 1.2.0
> [ ]  0 No opinion
> [ ] -1 Do not release this package because...
> 
> Thanks to everyone who contributed to this release.
> 
> -Taylor



signature.asc
Description: Message signed with OpenPGP


[GitHub] storm-site issue #2: Update dependencies

2018-02-12 Thread ptgoetz
Github user ptgoetz commented on the issue:

https://github.com/apache/storm-site/pull/2
  
+1


---


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


---


Re: [VOTE] Release Apache Storm 1.2.0 (rc4)

2018-02-12 Thread Bobby Evans
+1

I built from source all of the unit tests passed and some basic integration
tests passed too.

On Sat, Feb 10, 2018 at 12:00 PM Alexandre Vermeerbergen <
avermeerber...@gmail.com> wrote:

> Hello,
>
> +1 (non binding)
>
> Details: I almost got nuts when applying storm-kafka-client-1.2.0 RC4
> to our preproduction Storm cluster whose performances dropped quite a
> lot (at least x2 less throughput) ... but then I realized that I made
> the classic mistake of mixing two changes  : I had also upgraded our
> Kafka dependencies from Kafka Client 0.10.20.0 to Kafka  Client
> 1.0.0.0.
>
> Then I rollbacked our Kafka dependencies to Kafka 0.10.2.0 but I kept
> Storm 1.2.0 RC4 : since then (~10 hours ago) it's working smoothly.
>
> In particular, performances of Storm Kafka Client 1.2.0 RC4 having
> autocommit=true setting are equivalent to the ones of Storm Kafka
> Client 1.2.0 with
> .setProcessingGuarantee(ProcessingGuarantee.NO_GUARANTEE).
>
> Note : our preproduction Storm cluster consumes & produces data in a
> Kafka Cluster based on Kafka 1.0.0. I guess there's some details we
> missed when using Kafka Client 1.0.0 libraries instead of Kafka Client
> 0.10.2.0, but I think this is something we can study outside this
> 1.2.0 voting process, because consuming & producing Kafka messages
> against a Kafka cluster based on Kafka 1.0.0 Brokers is considered as
> compatible by Kakfa documentation.
>
> Since we want to move to Kafka Client 1.0.0 at some time (in
> particular, to take advantage of official JRE 9 support of this Kafka
> version), we will keep on testing our system with this newer Kafka
> version and we will feeback to Storm dev list whatever the outcome is
> (I'm guessing some property changes from 0.10.2 to 1.0.0)
>
> Thank you very much to all Storm developers for bringing this great
> Storm 1.2.0 to us, and for your taking into account my feedbacks: I
> have no more bad news for 1.2.0 :)
>
>
> Best regards,
> Alexandre Vermeerbergen
>
> PS: just one question: is there a way to access 1.2.0 RC4 Storm
> documentation before it's switched on public Storm site?
> PPS: I saw Stig's feedback about our deprecated usage of
> auto.commit.internal.ms, but I haven't yet tried the "new way",
> meaning that it has no impact on our big scale test. But I will try it
> later.
>
> 2018-02-09 9:18 GMT+01:00 Alexandre Vermeerbergen <
> avermeerber...@gmail.com>:
> > Hello,
> >
> > For your information, yesterday I upgraded our preproduction cluster
> > from Storm 1.2.0 SNAPSHOT of December 2017 to 1.2.0 Release Candidate
> > 4.
> > Our alerting system based on Storm behaved quite bad with 1.2.0 RC4,
> > and this morning, one of our team member noticed this message in our
> > topologie's startup logs:
> >
> > 1279 [main] WARN  o.a.s.k.s.KafkaSpoutConfig - The KafkaConsumer
> > enable.auto.commit setting is not supported. You can configure similar
> > behavior through KafkaSpoutConfig.Builder.setProcessingGuarantee.This
> > will be treated as an error in the next major release.
> >
> > 1279 [main] INFO  o.a.s.k.s.KafkaSpoutConfig - Setting Kafka consumer
> > property 'enable.auto.commit' to 'false', because the spout does not
> > support auto-commit
> >
> >
> > So I will resume our tests with this new setting and I will feedback
> > once I have enought uptime on our "large preproduction cluster" with
> > 1.2.0 RC4.
> >
> > Note: may I suggest to make this breaking change visible in Storm
> > 1.2.0 releases notes? this is quite impacting. Or event better: make
> > the topologies unable to start when they use such a removed property,
> > so that at least people aren't fooled until they wonder why their
> > Kafka spouts aren't anymore behaving like before and check logs?
> >
> > More to come when my preproduction tests will have been completed (1or
> > 2 days needed).
> >
> > Best regards,
> > Alexandre
> >
> > 2018-02-07 23:24 GMT+01:00 P. Taylor Goetz :
> >> This is a call to vote on releasing Apache Storm 1.2.0 (rc4)
> >>
> >> Note that the only difference between rc4 and rc3 is the fix for
> >> https://issues.apache.org/jira/browse/STORM-2942.
> >>
> >> Full list of changes in this release:
> >>
> >>
> https://dist.apache.org/repos/dist/dev/storm/apache-storm-1.2.0-rc4/RELEASE_NOTES.html
> >>
> >> The tag/commit to be voted upon is v1.2.0:
> >>
> >>
> https://git-wip-us.apache.org/repos/asf?p=storm.git;a=tree;h=cef4d49e222e53656f38c40d754d4f41799cd9a9;hb=2a0097f9a20b9df494caadb87c87d4e4db01a7ed
> >>
> >> The source archive being voted upon can be found here:
> >>
> >>
> https://dist.apache.org/repos/dist/dev/storm/apache-storm-1.2.0-rc4/apache-storm-1.2.0-src.tar.gz
> >>
> >> Other release files, signatures and digests can be found here:
> >>
> >> https://dist.apache.org/repos/dist/dev/storm/apache-storm-1.2.0-rc4/
> >>
> >> The release artifacts are signed with the following key:
> >>
> >>
> 

Re: [VOTE] Release Apache Storm 1.0.6 (rc3)

2018-02-12 Thread Bobby Evans
Happy to do the check.  This is something we don't want to get wrong.

On Fri, Feb 9, 2018 at 5:38 PM P. Taylor Goetz  wrote:

> Doh! I didn’t even think consider that it was likely autogenerated by an
> IDE and likely identical to what the JDK would generate.
>
> I agree with Bobby, let’s leave it there and continue with the RC.
>
> Thanks for pointing that out Bobby.
>
> -Taylor
>
> > On Feb 9, 2018, at 4:07 PM, Bobby Evans  wrote:
> >
> > Actually it is set properly.  Do NOT change it.  It works just fine the
> way
> > it is.
> >
> > $ git checkout f6f35dd98d2492a38aa4d61da7f6caee4ec2f31a # The git version
> > right before this change on branch-1.x
> > $ mvn clean install -DskipTests
> > $ serialver -classpath ./storm-core/target/storm-core-1.1.0-SNAPSHOT.jar
> > org.apache.storm.tuple.Fields
> > org.apache.storm.tuple.Fields:private static final long
> > serialVersionUID = -3377931843059975424L;
> >
> > This is the same version that we set it to as a part of the change.
> >
> > - Bobby
> >
> > On Fri, Feb 9, 2018 at 3:00 PM Bobby Evans  wrote:
> >
> >> Fields changing will cause problems if it is serialized as part of a
> >> bolt/spout or as part of a custom grouping.  I have not checked
> explicitly,
> >> but removing that line is the wrong thing to do.  By default the
> serialVersionUID
> >> is generated from the class itself, so removing it, but leaving in the
> >> modified code would still break backwards compatibility.
> >>
> >> I'll take a look and see what you need to set it to so you don't break
> >> backwards compatibility on 1.x
> >>
> >> On Fri, Feb 9, 2018 at 10:19 AM P. Taylor Goetz 
> wrote:
> >>
> >>> What are others’ opinions on removing the serialversionUid an moving
> >>> ahead with an RC4?
> >>>
> >>> -Taylor
> >>>
>  On Feb 9, 2018, at 7:21 AM, Jungtaek Lim  wrote:
> 
>  I just went ahead verifying current RC except serialization UID issue
> in
>  Fields. I could also vote for RC4 immediately if necessary.
> 
>  +1 (binding)
> 
> > source
> 
>  - verify file (signature, MD5, SHA)
>  -- source, tar.gz : OK
>  -- source, zip : OK
> 
>  - extract file
>  -- source, tar.gz : OK
>  -- source, zip : OK
> 
>  - diff-ing extracted files between tar.gz and zip : OK
> 
>  - build source with JDK 7
>  -- source, tar.gz : OK
> 
>  - build source dist
>  -- source, tar.gz : OK
> 
>  - build binary dist
>  -- source, tar.gz : OK
> 
> > binary
> 
>  - verify file (signature, MD5, SHA)
>  -- binary, tar.gz : OK
>  -- binary, zip : OK
> 
>  - extract file
>  -- binary, tar.gz : OK
>  -- binary, zip : OK
> 
>  - diff-ing extracted files between tar.gz and zip : OK
> 
>  - launch daemons : OK
> 
>  - run RollingTopWords (local) : OK
> 
>  - run RollingTopWords (remote) : OK
>  - activate / deactivate / rebalance / kill : OK
>  - logviewer (worker dir, daemon dir) : OK
>  - change log level : OK
>  - thread dump, heap dump, restart worker : OK
>  - log search : OK
> 
>  Thanks,
>  Jungtaek Lim (HeartSaVioR)
> 
>  2018년 2월 9일 (금) 오후 6:18, Erik Weathers  >님이
> >>> 작성:
> 
> > I'm fine submitting a PR to back that line out (or any of you
> committer
> > folks could just rip it out).
> >
> > But I'd like to understand Storm a bit better as part of making this
> > decision. :-)  Am I correct in assuming it would only be a problem if
> >>> the
> > serialized Fields were stored somewhere (e.g., ZooKeeper, local
> >>> filesystem)
> > and then read back in after the Nimbus/Workers are brought back up
> >>> after
> > the upgrade?  Seems Fields is used in a *lot* of places, and I don't
> >>> know
> > precisely what is serialized for reused upon Storm Nimbus/Worker
> daemon
> > restarts.  I believe there are examples of Fields being used to
> create
> > Spout or Bolt objects that are used to create the StormTopology
> object,
> > which I believe is serialized into ZooKeeper.  But I'm not clear if
> >>> it's
> > directly the Fields object itself or some kind of translation from
> that
> > into the thrift objects that make up StormTopology.
> >
> > I also don't know exactly when kryo is applicable in Storm.  I've
> never
> > done anything with kryo directly.
> >
> > - Erik
> >
> > On Thu, Feb 8, 2018 at 10:00 PM, P. Taylor Goetz 
> > wrote:
> >
> >> *serialized* ;)
> >>
> >>> On Feb 9, 2018, at 12:48 AM, P. Taylor Goetz 
> > wrote:
> >>>
> >>> I’d have to check (can’t right now), but I think that class gets
> >> sterilized via kryo. If that’s not the case, yes, it could cause
> > problems.
> >>>
> 

[GitHub] storm pull request #2502: new PR for STORM-2306 : Messaging subsystem redesi...

2018-02-12 Thread revans2
Github user revans2 commented on a diff in the pull request:

https://github.com/apache/storm/pull/2502#discussion_r167625247
  
--- Diff: 
storm-client/src/jvm/org/apache/storm/cluster/IStormClusterState.java ---
@@ -40,109 +40,117 @@
 import org.apache.storm.nimbus.NimbusInfo;
 
 public interface IStormClusterState {
-public List assignments(Runnable callback);
+List assignments(Runnable callback);
 
-public Assignment assignmentInfo(String stormId, Runnable callback);
+Assignment assignmentInfo(String stormId, Runnable callback);
 
-public VersionedData assignmentInfoWithVersion(String 
stormId, Runnable callback);
+VersionedData assignmentInfoWithVersion(String stormId, 
Runnable callback);
 
-public Integer assignmentVersion(String stormId, Runnable callback) 
throws Exception;
+Integer assignmentVersion(String stormId, Runnable callback) throws 
Exception;
 
-public List blobstoreInfo(String blobKey);
+List blobstoreInfo(String blobKey);
 
-public List nimbuses();
+List nimbuses();
 
-public void addNimbusHost(String nimbusId, NimbusSummary 
nimbusSummary);
+void addNimbusHost(String nimbusId, NimbusSummary nimbusSummary);
 
-public List activeStorms();
+List activeStorms();
 
 /**
  * Get a storm base for a topology
  * @param stormId the id of the topology
  * @param callback something to call if the data changes (best effort)
  * @return the StormBase or null if it is not alive.
  */
-public StormBase stormBase(String stormId, Runnable callback);
+StormBase stormBase(String stormId, Runnable callback);
 
-public ClusterWorkerHeartbeat getWorkerHeartbeat(String stormId, 
String node, Long port);
+ClusterWorkerHeartbeat getWorkerHeartbeat(String stormId, String node, 
Long port);
 
-public List getWorkerProfileRequests(String stormId, 
NodeInfo nodeInfo);
+List getWorkerProfileRequests(String stormId, NodeInfo 
nodeInfo);
 
-public List getTopologyProfileRequests(String stormId);
+List getTopologyProfileRequests(String stormId);
 
-public void setWorkerProfileRequest(String stormId, ProfileRequest 
profileRequest);
+void setWorkerProfileRequest(String stormId, ProfileRequest 
profileRequest);
 
-public void deleteTopologyProfileRequests(String stormId, 
ProfileRequest profileRequest);
+void deleteTopologyProfileRequests(String stormId, ProfileRequest 
profileRequest);
 
-public Map executorBeats(String stormId, 
Map executorNodePort);
+Map executorBeats(String stormId, 
Map executorNodePort);
 
-public List supervisors(Runnable callback);
+List supervisors(Runnable callback);
 
-public SupervisorInfo supervisorInfo(String supervisorId); // returns 
nil if doesn't exist
+SupervisorInfo supervisorInfo(String supervisorId); // returns nil if 
doesn't exist
 
-public void setupHeatbeats(String stormId);
+void setupHeatbeats(String stormId);
 
-public void teardownHeartbeats(String stormId);
+void teardownHeartbeats(String stormId);
 
-public void teardownTopologyErrors(String stormId);
+void teardownTopologyErrors(String stormId);
 
-public List heartbeatStorms();
+List heartbeatStorms();
 
-public List errorTopologies();
+List errorTopologies();
 
-public List backpressureTopologies();
+/** @deprecated: In Storm 2.0. Retained for enabling transition from 
1.x. Will be removed soon. */
--- End diff --

done. 3.0.0


---


[GitHub] storm issue #2555: STORM-2841 Use extended class instead of partial mock for...

2018-02-12 Thread arunmahadevan
Github user arunmahadevan commented on the issue:

https://github.com/apache/storm/pull/2555
  
+1


---


[GitHub] storm pull request #2553: STORM-2935 update autocredential metric documentat...

2018-02-12 Thread asfgit
Github user asfgit closed the pull request at:

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


---


[GitHub] storm issue #2553: STORM-2935 update autocredential metric documentation

2018-02-12 Thread revans2
Github user revans2 commented on the issue:

https://github.com/apache/storm/pull/2553
  
Thanks @agresch I just merged this into master.


---


[GitHub] storm pull request #2554: STORM-2939 add WorkerMetricsProcessor interface

2018-02-12 Thread revans2
Github user revans2 commented on a diff in the pull request:

https://github.com/apache/storm/pull/2554#discussion_r167608762
  
--- Diff: 
storm-server/src/main/java/org/apache/storm/metricstore/WorkerMetricsProcessor.java
 ---
@@ -0,0 +1,40 @@
+/**
+ * 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.metricstore;
+
+import java.util.Map;
+import org.apache.storm.generated.WorkerMetrics;
+
+public interface WorkerMetricsProcessor {
+
+/**
+ * Process insertion of worker metrics.
+ * @param conf Storm config map
--- End diff --

Can we make sure that we distinguish between the topology config here, and 
the daemon config that is passed into prepare?


---


[GitHub] storm pull request #2554: STORM-2939 add WorkerMetricsProcessor interface

2018-02-12 Thread revans2
Github user revans2 commented on a diff in the pull request:

https://github.com/apache/storm/pull/2554#discussion_r167608546
  
--- Diff: 
storm-server/src/main/java/org/apache/storm/metricstore/WorkerMetricsProcessor.java
 ---
@@ -0,0 +1,40 @@
+/**
+ * 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.metricstore;
+
+import java.util.Map;
+import org.apache.storm.generated.WorkerMetrics;
+
+public interface WorkerMetricsProcessor {
+
+/**
+ * Process insertion of worker metrics.
--- End diff --

Can we mention something about thread safety in here?


---


[GitHub] storm pull request #2554: STORM-2939 add WorkerMetricsProcessor interface

2018-02-12 Thread revans2
Github user revans2 commented on a diff in the pull request:

https://github.com/apache/storm/pull/2554#discussion_r167607557
  
--- Diff: storm-server/src/main/java/org/apache/storm/DaemonConfig.java ---
@@ -1040,6 +1039,13 @@
 // and log an error message, so just validating this as a String for 
now.
 public static final String STORM_METRIC_STORE_CLASS = 
"storm.metricstore.class";
 
+/**
+ * Class implementing WorkerMetricsProcessor.
--- End diff --

It might be good to mention here in the javadocs that this is intended to 
be run on the supervisor.


---


[GitHub] storm pull request #2554: STORM-2939 add WorkerMetricsProcessor interface

2018-02-12 Thread revans2
Github user revans2 commented on a diff in the pull request:

https://github.com/apache/storm/pull/2554#discussion_r167608078
  
--- Diff: 
storm-server/src/main/java/org/apache/storm/metricstore/MetricStoreConfig.java 
---
@@ -41,5 +41,23 @@ public static MetricStore configure(Map conf) throws 
MetricException {
 throw new MetricException("Failed to create metric store", t);
 }
 }
+
+/**
+ * Configures metric processor to use the class specified in the conf.
+ * @param conf Storm config map
+ * @return WorkerMetricsProcessor prepared processor
+ * @throws MetricException  on misconfiguration
+ */
+public static WorkerMetricsProcessor configureMetricProcessor(Map 
conf) throws MetricException {
+
+try {
+String processorClass = 
(String)conf.get(DaemonConfig.STORM_METRIC_PROCESSOR_CLASS);
+WorkerMetricsProcessor processor = (WorkerMetricsProcessor) 
(Class.forName(processorClass)).newInstance();
+processor.prepare(conf);
+return processor;
+} catch (Throwable t) {
--- End diff --

Can we just make this an Exception instead of a Throwable?  Most Errors are 
not things that we want to try and recover from.


---


[GitHub] storm issue #2553: STORM-2935 update autocredential metric documentation

2018-02-12 Thread agresch
Github user agresch commented on the issue:

https://github.com/apache/storm/pull/2553
  
Can this be merged?  


---


[GitHub] storm pull request #2555: STORM-2841 Use extended class instead of partial m...

2018-02-12 Thread HeartSaVioR
GitHub user HeartSaVioR opened a pull request:

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

STORM-2841 Use extended class instead of partial mock for TestHiveBolt

* while mocking partially, oddly it calls real method intermittently which 
breaks test

Please refer https://issues.apache.org/jira/browse/STORM-2841 to see the 
detail of failure.

I guess this is clean cherry-picking for 1.x version lines. I'll submit 
another one if not.

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

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

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

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


commit 2977d98b5cc43dd46fce33c537ea7ac4db36a3b2
Author: Jungtaek Lim 
Date:   2018-02-12T08:55:48Z

STORM-2841 Use extended class instead of partial mock for TestHiveBolt

* while mocking partially, oddly it calls real method intermittently which 
breaks test




---