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 <ptgo...@gmail.com> 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 <ev...@oath.com.INVALID> 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 <ev...@oath.com> 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 <ptgo...@gmail.com>
> 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 <kabh...@gmail.com> 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 <eweath...@groupon.com.invalid
> >님이
> >>> 작성:
> >>>>
> >>>>> 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 <ptgo...@gmail.com>
> >>>>> wrote:
> >>>>>
> >>>>>> *serialized* ;)
> >>>>>>
> >>>>>>> On Feb 9, 2018, at 12:48 AM, P. Taylor Goetz <ptgo...@gmail.com>
> >>>>> 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.
> >>>>>>>
> >>>>>>> I think the safest option would be to remove the serialversionuid.
> >>>>>>>
> >>>>>>> -Taylor
> >>>>>>>
> >>>>>>>> On Feb 8, 2018, at 5:36 PM, Erik Weathers
> >>>>> <eweath...@groupon.com.INVALID>
> >>>>>> wrote:
> >>>>>>>>
> >>>>>>>> Something I just realized -- in the storm-kafka-client stomping
> into
> >>>>>>>> 1.0.x-branch PR, I backported a change to Fields.java which added
> a
> >>>>>>>> serialVersionUID.
> >>>>>>>> Could that potentially break topologies when you upgrade
> storm-core
> >>> on
> >>>>>> the
> >>>>>>>> servers (nimbus, workers) from 1.0.{1..5} to 1.0.6?   I'm not
> super
> >>>>>>>> familiar with the serialization that occurs in Storm and whether
> >>> that
> >>>>>> could
> >>>>>>>> break people.
> >>>>>>>>
> >>>>>>>> https://github.com/apache/storm/pull/2550/files#diff-71a428d
> >>>>>> 508c4f5af0bfe3cc186e8edcf
> >>>>>>>>
> >>>>>>>> - Erik
> >>>>>>>>
> >>>>>>>>> On Thu, Feb 8, 2018 at 1:25 PM, Bobby Evans
> <ev...@oath.com.invalid
> >>>>
> >>>>>> wrote:
> >>>>>>>>>
> >>>>>>>>> +1 I built the code from the git tag, ran all the unit tests
> (which
> >>>>>> passed
> >>>>>>>>> the first time), and ran some tests on a single node cluster.
> >>>>>>>>>
> >>>>>>>>> It all looked good.
> >>>>>>>>>
> >>>>>>>>> - Bobby
> >>>>>>>>>
> >>>>>>>>>> On Thu, Feb 8, 2018 at 1:22 PM P. Taylor Goetz <
> ptgo...@gmail.com
> >>>>
> >>>>>> 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=7993db01580ce62d
> >>>>>> 44866dc00e0a72
> >>>>>>>>> 66984638d0
> >>>>>>>>>>
> >>>>>>>>>> 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/orgapache
> >>>>>> storm-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
> >>>>>>>>>
> >>>>>>
> >>>>>
> >>>
> >>>
>
>

Reply via email to