Hello! I'm motivated to see this happen :D +Zoltan, the original author. I created a PR against apache/avro master here: https://github.com/apache/avro/pull/589
I cherry-picked the commit from your fork, and reapplied spotless/checkstyle. I hope this is the correct way to preserve authorship and that I'm not stepping on any toes! Can someone take a look at the above PR? Best regards, Ryan On Tue, Jul 16, 2019 at 11:58 AM Ismaël Mejía <ieme...@gmail.com> wrote: > Yes probably it is overkill to warn given the examples you mention. > Also your argument towards reusing the mature (and battle tested) > combination of Schema.Parser + String serialization makes sense. > > Adding this to 1.9.1 will be an extra selling point for projects > wanting to migrate to the latest version of Avro so it sounds good to > me but you should add it to master and then we can cherry pick it from > there. > > > On Tue, Jul 16, 2019 at 11:16 AM Ryan Skraba <r...@skraba.com> wrote: > > > > Hello! Thanks to the reference to AVRO-1852. It's exactly what I was > looking for. > > > > I agree that Java serialization shouldn't be used for anything > cross-platform, or (in my opinion) used for any data persistence at all. > Especially not for an Avro container file or sending binary data through a > messaging system... > > > > But Java serialization is definitely useful and used for sending > instances of "distributed work" implemented in Java from node to node in a > cluster. I'm not too worried about existing connectors -- we can see that > each framework has "solved" the problem one at a time. In addition to > Flink, there's > https://github.com/apache/beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/io/AvroUtils.java#L29 > and > https://github.com/apache/spark/blob/3663dbe541826949cecf5e1ea205fe35c163d147/external/avro/src/main/scala/org/apache/spark/sql/avro/AvroOutputWriterFactory.scala#L35 > . > > > > Specifically, I see the advantage for user-defined distributed functions > that happen to carry along an Avro Schema -- and I can personally say that > I've encountered this a lot in our code! > > > > That being said, I think it's probably overkill to warn the user about > the perils of Java serialization (not being cross-language and requiring > consistent JDKs and libraries across JVMs). If an error occurs for one of > those reasons, there's a larger problem for the dev to address, and it's > just as likely to occur for any Java library in the job if the environment > is bad. Related, we've encountered similar issues with logical types > existing in Avro 1.8 in the driver but not in Avro 1.7 on the cluster... > the solution is "make sure you don't do that". (Looking at you, guava and > jackson!) > > > > The patch in question delegates serialization to the string form of the > schema, so it's basically doing what all of the above Avro "holders" are > doing -- I wouldn't object to having a sample schema available that fully > exercises what a schema can hold, but I also think that Schema.Parser (used > underneath) is currently pretty well tested and mature! > > > > Do you think this could be a candidate for 1.9.1 as a minor > improvement? I can't think of any reason that this wouldn't be backwards > compatible. > > > > Ryan > > > > side note: I wrote java.lang.Serializable earlier, which probably didn't > help my search for prior discussion... :/ > > > > On Tue, Jul 16, 2019 at 9:59 AM Ismaël Mejía <ieme...@gmail.com> wrote: > >> > >> This is a good idea even if it may have some issues that we should > >> probably document and warn users about: > >> > >> 1. Java based serialization is really practical for JVM based systems, > >> but we should probably add a warning or documentation because Java > >> serialization is not deterministic between JVMs so this could be a > >> source for issues (usually companies use the same version of the JVM > >> so this is less critical, but this still can happen specially now with > >> all the different versions of Java and OpenJDK based flavors). > >> > >> 2. This is not cross language compatible, the String based > >> representation (or even an Avro based representation of Schema) can be > >> used in every language. > >> > >> Even with these I think just for ease of use it is worth to make > >> Schema Serializable. Is the plan to fully serialize it, or just to > >> make it a String and serialize the String as done in the issue Doug > >> mentioned? > >> If we take the first approach we need to properly test with a Schema > >> that has elements of the full specification that (de)-serialization > >> works correctly. Does anyone know if we have already a test schema > >> that covers the full ‘schema’ specification to reuse it if so? > >> > >> On Mon, Jul 15, 2019 at 11:46 PM Driesprong, Fokko <fo...@driesprong.frl> > wrote: > >> > > >> > Correct me if I'm wrong here. But as far as I understood the way of > >> > serializing the schema is using Avro, as it is part of the file. To > avoid > >> > confusion there should be one way of serializing. > >> > > >> > However, I'm not sure if this is worth the hassle of not simply > >> > implementing serializable. Also Flink there is a rather far from > optimal > >> > implementation: > >> > > https://github.com/apache/flink/blob/master/flink-formats/flink-parquet/src/main/java/org/apache/flink/formats/parquet/avro/ParquetAvroWriters.java#L72 > >> > This converts it to JSON and back while distributing the schema to the > >> > executors. > >> > > >> > Cheers, Fokko > >> > > >> > Op ma 15 jul. 2019 om 23:03 schreef Doug Cutting <cutt...@gmail.com>: > >> > > >> > > I can't think of a reason Schema should not implement Serializable. > >> > > > >> > > There's actually already an issue & patch for this: > >> > > > >> > > https://issues.apache.org/jira/browse/AVRO-1852 > >> > > > >> > > Doug > >> > > > >> > > On Mon, Jul 15, 2019 at 6:49 AM Ismaël Mejía <ieme...@gmail.com> > wrote: > >> > > > >> > > > +d...@avro.apache.org > >> > > > > >> > > > On Mon, Jul 15, 2019 at 3:30 PM Ryan Skraba <r...@skraba.com> > wrote: > >> > > > > > >> > > > > Hello! > >> > > > > > >> > > > > I'm looking for any discussion or reference why the Schema > object isn't > >> > > > serializable -- I'm pretty sure this must have already been > discussed > >> > > (but > >> > > > the keywords +avro +serializable +schema have MANY results in all > the > >> > > > searches I did: JIRA, stack overflow, mailing list, web) > >> > > > > > >> > > > > In particular, I was at a demo today where we were asked why > Schemas > >> > > > needed to be passed as strings to run in distributed tasks. I > remember > >> > > > running into this problem years ago with MapReduce, and again in > Spark, > >> > > and > >> > > > again in Beam... > >> > > > > > >> > > > > Is there any downside to making a Schema implement > >> > > > java.lang.Serializable? The only thing I can think of is that > the schema > >> > > > _should not_ be serialized with the data, and making it > non-serializable > >> > > > loosely enforces this (at the cost of continually writing > different > >> > > > flavours of "Avro holders" for when you really do want to > serialize it). > >> > > > > > >> > > > > Willing to create a JIRA and work on the implementation, of > course! > >> > > > > > >> > > > > All my best, Ryan > >> > > > > >> > > >
