srdo commented on issue #3193: [STORM-3566] add serialVersionUID field to class which implement Serializable URL: https://github.com/apache/storm/pull/3193#issuecomment-578095138 @Ethanlm I'm not sure. I think we should probably add the field for all bolts and classes serialized as part of bolt state, since these classes are likely to be serialized by the submitter JVM, and then deserialized by the supervisor/worker. For other classes, we should probably look at whether they get serialized by one JVM, and then deserialized by another. If they are, we should add the field, and will then have to remember to change the value if we add/change fields in PRs. Regarding whether we remember to update the field, I think there might be a compatibility issue in the current code. As it is now, I think we can't change state in any of the serializable classes that are not packaged in topology jars without breaking user code due to changed UID. This would for example include the bolts in storm-client. I think if we change a class there, we would run into issues with rolling upgrades, since e.g. Nimbus might be running 2.1.0, and the worker might be running 2.0.1, and the version of the class that is loaded depends on the Storm version instead of depending on the contents of the topology jar. For classes that get packaged as part of the topology jar (e.g. any bolts in the externals or examples), we should be fine. I'm not sure how to solve the compatibility issue (assuming it exists), without moving the serializable classes out of storm-client, and into another module that we then require topology jars to package.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
