[SPARK-24601] Update Jackson to 2.9.6

[Fokko]

Hi all,

Jackson is incompatible with upstream versions, therefore bump the Jackson version to a more recent one. I bumped into some issues with Azure CosmosDB that is using a more recent version of Jackson. This can be fixed by adding exclusions and then it works without any issues. So no breaking changes in the API's.

I would also consider bumping the version of Jackson in Spark. I would suggest to keep up to date with the dependencies, since in the future this issue will pop up more frequently.

Jira: https://issues.apache.org/jira/projects/SPARK/issues/SPARK-24601

What changes were proposed in this pull request?

Bump Jackson to 2.9.6

How was this patch tested?

Compiled and tested it locally to see if anything broke.

Please review http://spark.apache.org/contributing.html before opening a pull request.

[HyukjinKwon]

The tests will be already failed given my tries before and there are many concerns about this. Also see and address the concerns in #9759.

[Fokko]

This was more than a year ago, we should eventually upgrade..

[vanzin]

I agree we should upgrade, but just changing the pom version will not work. I have an internal patch for this and I needed small changes in a few other places...

[vanzin]

Which is, btw, a way of saying you should run dev/run-tests locally, at least, when changing a dependency.

[HyukjinKwon]

I tried this in my local too. I didn't mean that we shouldn't but let's fix the test cases (or main codes) too and address other concerns.

[Fokko]

I could not get the tests working locally. :-) Let me give it another try.

[HyukjinKwon]

ok to test

[SparkQA]

Test build #92166 has finished for PR 21596 at commit ba454b0.

• This patch fails build dependency tests.
• This patch merges cleanly.
• This patch adds no public classes.

[MaxGekk]

Please, make sure that performance doesn't degrade after upgrading Jackson. You can check that by JsonBenchmarks.

[cxzl25]

#20738

Bump jackson from 2.6.7&2.6.7.1 to 2.7.7
Jackson(>=2.7.7) fixes the possibility of missing tail data when the length of the value is in a range
https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.7.7
https://github.com/FasterXML/jackson-core/issues/30

spark-shell:

val value = "x" * 3000
val json = s"""{"big": "$value"}"""
spark.sql("select length(get_json_object(\'"+json+"\','$.big'))" ).collect
res0: Array[org.apache.spark.sql.Row] = Array([2991])

expect result : 3000
actual result : 2991

[HyukjinKwon]

@cxzl25, 2.7.x has a regression so we had to revert it back. Please see #9759.

[HyukjinKwon]

@Fokko, mind adding the test described in #20738 here too?

[Fokko]

Yes, let me add it and run the tests locally

[Fokko]

I've pushed the changes, the tests are still running locally.

[SparkQA]

Test build #92209 has finished for PR 21596 at commit dfcc039.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[Fokko]

Hmm, I see a relevant failing test:

org.scalatest.exceptions.TestFailedException: "...430","name":"scope1"[,"parent":null]}" did not equal "...430","name":"scope1"[]}"

From the test core/src/test/scala/org/apache/spark/rdd/RDDOperationScopeSuite.scala:

  test("json de/serialization") {
    val scope1Json = scope1.toJson
    val scope2Json = scope2.toJson
    val scope3Json = scope3.toJson
    assert(scope1Json === s"""{"id":"${scope1.id}","name":"scope1"}""")
    assert(scope2Json === s"""{"id":"${scope2.id}","name":"scope2","parent":$scope1Json}""")
    assert(scope3Json === s"""{"id":"${scope3.id}","name":"scope3","parent":$scope2Json}""")
    assert(RDDOperationScope.fromJson(scope1Json) === scope1)
    assert(RDDOperationScope.fromJson(scope2Json) === scope2)
    assert(RDDOperationScope.fromJson(scope3Json) === scope3)
  }

It looks like it is inserting the null value ,"parent":null, which was omitted before. I would say that this is rather a design choice, than a bug.

Edit: I agree this field should not be rendered: https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/rdd/RDDOperationScope.scala#L44

[Fokko]

Ok, I've found the issue:

FasterXML/jackson-module-scala#325

Changing this to NON_ABSENT should fix this.

[SparkQA]

Test build #92229 has finished for PR 21596 at commit 9107328.

• This patch fails RAT tests.
• This patch merges cleanly.
• This patch adds no public classes.

[Fokko]

Oops, a dangling file. Sorry for triggering so many builds guys.

[SparkQA]

Test build #92228 has finished for PR 21596 at commit 0d2e186.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #92231 has finished for PR 21596 at commit 1fd1d10.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #92230 has finished for PR 21596 at commit 0cb0ea5.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[HyukjinKwon]

nit: ->

checkEvaluation(
  GetJsonObject(NonFoldableLiteral((s"""{"big": "$value"}""")),
  NonFoldableLiteral("$.big")), value)

[Fokko]

Fixed, much nicer indeed

[Fokko]

When looking at the history server, we have a similar issue. From at the list command https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/92231/testReport/org.apache.spark.deploy.history/HistoryServerSuite/stage_list_json/ :

I've formatted the json, expected:

[{
	"status": "COMPLETE",
	"stageId": 3,
	"attemptId": 0,
	"numTasks": 8,
	"numActiveTasks": 0,
	"numCompleteTasks": 8,
	"numFailedTasks": 0,
	"numKilledTasks": 0,
	"numCompletedIndices": 8,
	"executorRunTime": 162,
	"executorCpuTime": 0,
	"submissionTime": "2015-02-03T16:43:07.191GMT",
	"firstTaskLaunchedTime": "2015-02-03T16:43:07.191GMT",
	"completionTime": "2015-02-03T16:43:07.226GMT",
	"inputBytes": 160,
	"inputRecords": 0,
	"outputBytes": 0,
	"outputRecords": 0,
	"shuffleReadBytes": 0,
	"shuffleReadRecords": 0,
	"shuffleWriteBytes": 0,
	"shuffleWriteRecords": 0,
	"memoryBytesSpilled": 0,
	"diskBytesSpilled": 0,
	"name": "count at <console>:17",
	"details": "org.apache.spark.rdd.RDD.count(RDD.scala:910)\n$line19.$read$$iwC$$iwC$$iwC$$iwC.<init>(<console>:17)\n$line19.$read$$iwC$$iwC$$iwC.<init>(<console>:22)\n$line19.$read$$iwC$$iwC.<init>(<console>:24)\n$line19.$read$$iwC.<init>(<console>:26)\n$line19.$read.<init>(<console>:28)\n$line19.$read$.<init>(<console>:32)\n$line19.$read$.<clinit>(<console>)\n$line19.$eval$.<init>(<console>:7)\n$line19.$eval$.<clinit>(<console>)\n$line19.$eval.$print(<console>)\nsun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)\nsun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)\nsun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)\njava.lang.reflect.Method.invoke(Method.java:606)\norg.apache.spark.repl.SparkIMain$ReadEvalPrint.call(SparkIMain.scala:852)\norg.apache.spark.repl.SparkIMain$Request.loadAndRun(SparkIMain.scala:1125)\norg.apache.spark.repl.SparkIMain.loadAndRunReq$1(SparkIMain.scala:674)\norg.apache.spark.repl.SparkIMain.interpret(SparkIMain.scala:705)\norg.apache.spark.repl.SparkIMain.interpret(SparkIMain.scala:669)",
	"schedulingPool": "default",
	"rddIds": [6, 5],
	"accumulatorUpdates": [],
	"killedTasksSummary": {}
}, ... ]

found:

[{
	"status": "COMPLETE",
	"stageId": 3,
	"attemptId": 0,
	"numTasks": 8,
	"numActiveTasks": 0,
	"numCompleteTasks": 8,
	"numFailedTasks": 0,
	"numKilledTasks": 0,
	"numCompletedIndices": 8,
	"executorRunTime": 162,
	"executorCpuTime": 0,
	"submissionTime": "2015-02-03T16:43:07.191GMT",
	"firstTaskLaunchedTime": "2015-02-03T16:43:07.191GMT",
	"completionTime": "2015-02-03T16:43:07.226GMT",
	"failureReason": null,
	"inputBytes": 160,
	"inputRecords": 0,
	"outputBytes": 0,
	"outputRecords": 0,
	"shuffleReadBytes": 0,
	"shuffleReadRecords": 0,
	"shuffleWriteBytes": 0,
	"shuffleWriteRecords": 0,
	"memoryBytesSpilled": 0,
	"diskBytesSpilled": 0,
	"name": "count at <console>:17",
	"description": null,
	"details": "org.apache.spark.rdd.RDD.count(RDD.scala:910)\n$line19.$read$$iwC$$iwC$$iwC$$iwC.<init>(<console>:17)\n$line19.$read$$iwC$$iwC$$iwC.<init>(<console>:22)\n$line19.$read$$iwC$$iwC.<init>(<console>:24)\n$line19.$read$$iwC.<init>(<console>:26)\n$line19.$read.<init>(<console>:28)\n$line19.$read$.<init>(<console>:32)\n$line19.$read$.<clinit>(<console>)\n$line19.$eval$.<init>(<console>:7)\n$line19.$eval$.<clinit>(<console>)\n$line19.$eval.$print(<console>)\nsun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)\nsun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)\nsun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)\njava.lang.reflect.Method.invoke(Method.java:606)\norg.apache.spark.repl.SparkIMain$ReadEvalPrint.call(SparkIMain.scala:852)\norg.apache.spark.repl.SparkIMain$Request.loadAndRun(SparkIMain.scala:1125)\norg.apache.spark.repl.SparkIMain.loadAndRunReq$1(SparkIMain.scala:674)\norg.apache.spark.repl.SparkIMain.interpret(SparkIMain.scala:705)\norg.apache.spark.repl.SparkIMain.interpret(SparkIMain.scala:669)",
	"schedulingPool": "default",
	"rddIds": [6, 5],
	"accumulatorUpdates": [],
	"tasks": null,
	"executorSummary": null,
	"killedTasksSummary": {}
}, ... ]

We observe that the "failureReason": null, is listed, while it should have been omitted.

[HyukjinKwon]

@Fokko, btw, I believe #21596 (comment) is a valid comment. I think it would be nicer if we execute this benchmark and update the results in the file, and push it into here.

[HyukjinKwon]

BTW, I believe it's okay to use Jenkins resource if that's a faster way to resolve this issue in any event.

[Fokko]

@HyukjinKwon Any idea if this has any change of getting merged? The 2.4 branch has been cut a while ago.

The number of merge conflicts are minimal in all the times I've rebased onto master. I think this should be done some time to bump the Jackson version, since old version of Jackson aren't compatible with the newer ones. Please let me know.

[SparkQA]

Test build #96330 has finished for PR 21596 at commit 44b8d1b.

• This patch fails due to an unknown error code, -9.
• This patch merges cleanly.
• This patch adds no public classes.

[HyukjinKwon]

I think we're (at least i am) not clear on 2.5 vs 3.0 now (correct me if im mistaken). The change itself looks okay but I think it's targeted to 3.0.0 as discussed. I can take an action here when I'm clear on this.

[SparkQA]

Test build #96335 has finished for PR 21596 at commit 0b0b8fc.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #96377 has finished for PR 21596 at commit f4d294c.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #96413 has finished for PR 21596 at commit 2bab06f.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[Fokko]

Rebased onto master. @HyukjinKwon can we target this for 2.5?

[SparkQA]

Test build #96926 has finished for PR 21596 at commit b53c8f2.

• This patch fails due to an unknown error code, -9.
• This patch merges cleanly.
• This patch adds no public classes.

[HyukjinKwon]

retest this please

[SparkQA]

Test build #96928 has finished for PR 21596 at commit b53c8f2.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[HyukjinKwon]

Merged to master.

[gatorsmile]

It sounds like Jackson have an important behavior change that might impact us.

import com.fasterxml.jackson.annotation.JsonInclude.Include
import com.fasterxml.jackson.databind.{DeserializationFeature, ObjectMapper}
import com.fasterxml.jackson.module.scala.DefaultScalaModule
import com.fasterxml.jackson.module.scala.experimental.ScalaObjectMapper

val mapper = new ObjectMapper with ScalaObjectMapper
mapper.setSerializationInclusion(Include.NON_NULL)
mapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)
mapper.registerModule(DefaultScalaModule)

println(mapper.writeValueAsString(Map("abc" -> null)))

Previously, it outputs {"abc":null}, but now it is {}, unless we explicitly set it to Include.ALWAYS. The upgrade looks risky to me.

[Fokko]

@gatorsmile Are you completely sure about that? The Spark REST API null values are always omitted. Where is this behaviour different?

mapper.setSerializationInclusion(Include.NON_ABSENT):

scala> println(mapper.writeValueAsString(Map("abc" -> null)))
{}

mapper.setSerializationInclusion(Include.ALWAYS):

scala> println(mapper.writeValueAsString(Map("abc" -> null)))
{"abc":null}

If you're doing proper Scala and try to avoid the null's in general, with None in combination with mapper.setSerializationInclusion(Include.NON_ABSENT) you'll get:

scala> println(mapper.writeValueAsString(Map("abc" -> None)))
{"abc":null}