KAFKA-10477: Enabling the same behavior of NULL JsonNodeType to MISSI…

[shaikzakiriitm]

…NG JsonNodeType in JsonConverter.

From v2.10.0 onwards, in jackson lib, ObjectMapper.readTree(input) started to return JsonNode of type MISSING for empty input, as mentioned in the issue: FasterXML/jackson-databind#2211.

This caused to throw a DataException["Unknown schema type: null"] when an empty message key was parsed to be converted to Connect format via JsonConverter. The schemaType for MISSING type of JsonNode was returned as null, which resulted in this exception.

As part of this PR, made changes in JsonParser to incorporate the Jackson's ObjectMapper treatment of empty input from v2.10.0 onwards. Treating MISSING JsonNodeType in a similar fashion as that of NULL JsonNodeType.
Added a unit test for this.

Related Jira ticket captures further details: https://issues.apache.org/jira/browse/KAFKA-10477
All unit tests passed locally.

Committer Checklist (excluded from commit message)

• Verify design and implementation
• Verify test coverage and CI build status
• Verify documentation (including upgrade notes)

[shaikzakiriitm]

@kkonstantine @rhauch Kindly take a look at the changes. Thanks!

[shaikzakiriitm]

Failures are in ConnectionQuotasTest, and ConsumerBounceTest test classes, and appear unrelated to the change in this PR.

[rhauch]

@shaikzakiriitm, you mentioned above:

From v2.10.0 onwards, in jackson lib, ObjectMapper.readTree(input) started to return JsonNode of type MISSING for empty input, as mentioned in the issue: FasterXML/jackson-databind#2211.

However, the FasterXML/jackson-databind#2211 you mention above talks about this behavior changing in 2.9 relative to 2.8. This seems to not align with the your assertion in the KAFKA-10477 that:

Things were working fine when the dependency on jackson lib was of version v2.9.10.3

Can you clarify?

[shaikzakiriitm]

Following are the version dependencies of Apache Kafka (ak) on com.fasterxml.jackson.core:jackson-databind (j)

• ak v2.1.0 -> j v2.9.7
• ak v2.1.1 -> j v2.9.8
• ak v2.2.0 -> j v2.9.8
• ak v2.2.1 -> j v2.9.8
• ak v2.2.2 -> j v2.10.0
• ak v2.3.0 -> j v2.9.9 [Looks like an outlier, where ak version increased with a decrease in jackson-databind version]
• ak v2.3.1 -> j v2.10.0
• ak v2.4.0 -> j v2.10.0
• ak v2.4.1 -> j v2.10.0
• ak v2.5.1 -> j v2.10.2
• ak v2.6 -> j v2.10.2

The behavior of the Jackson ObjectMapper#readTree() method when called with a blank string/empty-input changed as follows (based on the information from FasterXML/jackson-databind#2211) :

• For Jackson v2.x-2.8.x, return NullNode
• For Jackson v2.9.x return null
• For Jackson v2.10.0 and beyond return MissingNode

However, in all AK versions JsonConverter throws an error when it gets a MissingNode. This is only a problem for AK versions (v2.2.2, v2.3.1, v2.4.0, v2.4.1, v2.5.1, v2.6) that were released with Jackson 2.10.0 or later:

• All versions of AK 2.1.x-2.2.1, ObjectMapper.readTree() returns null on empty input, which AK populates in an envelope, and the payload in the envelope which is null is treated as json node of JsonNodeType NULL. And this NULL case is handled in JsonConverter#convertToConnect().
• AK version 2.2.2, ObjectMapper.readTree() returns MissingNode on empty input, which AK populates as payload in envelope; this payload is a jsonNode of JsonNodeType MISSING. And JsonConverter#convertToConnect() considers schemaType to be null for MISSING case, which results in DataException.
• AK 2.3.0, (uses jackson-databind version 2.9.9), ObjectMapper.readTree() returns null on empty input, which AK populates as payload in envelope; this payload when retrieved in to be used in JsonConverter.convertToConnect(), is treated as NullNode, of JsonNodeType NULL. (as in the case of AK 2.1.x-2.2.1).
• AK 2.3.1-2.6, ObjectMapper.readTree() returns MissingNode on empty input, which AK populates as payload in envelope; this payload is a jsonNode of type MISSING which causes the DataException.

So, this PR aims to resolve the DataException that gets thrown when jackson-databind's ObjectMapper.readTree() returns MissingNode upon encountering an empty input, so that JsonConverter treats MissingNode in a similar behavior as that of null.

[shaikzakiriitm]

@shaikzakiriitm, you mentioned above:

From v2.10.0 onwards, in jackson lib, ObjectMapper.readTree(input) started to return JsonNode of type MISSING for empty input, as mentioned in the issue: FasterXML/jackson-databind#2211.

However, the FasterXML/jackson-databind#2211 you mention above talks about this behavior changing in 2.9 relative to 2.8. This seems to not align with the your assertion in the KAFKA-10477 that:

Things were working fine when the dependency on jackson lib was of version v2.9.10.3

Can you clarify?

Hi, I have shared more details in the comment #9306 (comment) . Kindly let me know if any further details are needed. Thanks!

[rhauch]

Thanks for putting together this comprehensive list of AK versions, Jackson versions, and behavior.

• ak v2.3.0 -> j v2.9.9 [Looks like an outlier, where ak version increased with a decrease in jackson-databind version]
  I don't think this was an outlier. If you sort them by release dates, things look a bit more logical and you can see how the Jackson versions only continually increase over time:

ak v2.1.0 (2018-11-20)-> j v2.9.7
ak v2.1.1 (2019-02-15) -> j v2.9.8
ak v2.2.0 (2019-03-22) -> j v2.9.8
ak v2.2.1 (2019-06-01) -> j v2.9.8
ak v2.3.0 (2019-06-24) -> j v2.9.9
ak v2.3.1 (2019-10-24) -> j v2.10.0
ak v2.2.2 (2019-12-01) -> j v2.10.0
ak v2.4.0 (2019-12-13) -> j v2.10.0
ak v2.4.1 (2020-03-10)-> j v2.10.0
ak v2.5.0 (2020-04-14) -> j v2.10.2
ak v2.6.0 (2020-08-03) -> j v2.10.2
ak v2.5.1 (2020-08-10) -> j v2.10.2

IOW, the following versions are all affected since they use Jackson 2.10.x:

• 2.3.1
• 2.2.2
• 2.4.0
• 2.4.1
• 2.5.0
• 2.5.1
• 2.6.0

and thus the following branches are affected since they now use Jackson 2.10.x:

• 2.2
• 2.3
• 2.4
• 2.5
• 2.6

I've also gone through a number of versions of the https://github.com/FasterXML/jackson-databind code, looking for how MissingNode is used. It's obvious that MissingNode is used a lot more prevalently in 2.10.0. But the documentation for MissingNode is as follows:

In most respects this placeholder node will act as {@link NullNode};
for example, for purposes of value conversions, value is considered
to be null and represented as value zero when used for numeric
conversions.

So I think it's pretty clear that we should treat JsonNodeType.MISSING as null. This is what this PR tries to do, and I'm now less concerned about unexpected behavioral changes after having read and verified this analysis.

Once this PR is merged to trunk, this fix will need to be backported to the following branches that are all affected because they use Jackson 2.10.x:

• 2.2
• 2.3
• 2.4
• 2.5
• 2.6

[rhauch]

@shaikzakiriitm I added another test to verify that having a null within a JSON field is not a problem. It builds for me locally, and the new test passes when I've tested it against 2.3.1, 2.2.2, 2.4.0, 2.4.1, 2.5.0, 2.5.1, 2.6.0. However, even though it doesn't fail with these versions like your other test, it is still useful to at least verify that such behavior is unchanged.

If we get a green build, I'll merge as discussed above.

[rhauch]

LGTM. Thanks, @shaikzakiriitm!

[rhauch]

No green jobs, but all of the Connect and MirrorMaker2 tests in all of the jobs passed. All of the failures in the tests were (flaky) client, broker, or streams tests.