Hi Jay, Comments inline.
On Tue, Jul 14, 2015 at 11:04 PM, Jay Kreps <j...@confluent.io> wrote: > Is this going to become a dependency for core and then transitively for the > old clients? That's right. > The current json library is definitely not great, but it does > parse json and it's not used in any context where performance is a concern. > The issue seemed to indicate that the blocking and slow performance were causing issues: https://issues.apache.org/jira/browse/KAFKA-1595 > > Because the older clients aren't well modularized, adding core dependencies > sucks these up into every user of the clients. This particularly becomes a > problem with common libraries since it will turn out we require version X > but other code in the same app requires version Y. > Yes, I understand. Still, if we use Jackson and only use methods that existed in 2.0, then it's unlikely to cause issues. I checked it with Jackson's author: https://twitter.com/ijuma/status/621106341503991808. The reasoning is as follows: - Jackson 1 and Jackson 2 use different packages, so they co-exist peacefully. - Jackson 2.x releases are backwards compatible so clients are free to choose whichever version they want. If they don't pick a version, then the highest version among the dependencies will be chosen. - It is possible that bugs in an untested release would cause issues, but that could affect the existing JSON parser too (clients may use different Scala versions). Limiting ourselves to methods from Jackson 2.0 is not as hard as it sounds because the code only interacts with our thin wrapper of Jackson, all the code that deals directly with Jackson is isolated. The new clients fix this issue but not everyone is using them yet. > > If there is a pressing need maybe we should just do it and people who have > problems can just hack their build to exclude the dependency (since the > client code won't need it). If not it might be better just to leave it for > a bit until we have at least get a couple releases with both the new > producer and the new consumer. > Hacking the builds to exclude the transitive dependency would be a last resort, but not needed in most (hopefully all) cases. Does this make it less problematic in your view? If people are concerned about this, we can delay it, of course. A bit of a shame though, as this change improves performance, makes the code more readable and makes it safer. Ismael
