[HUDI-5681] Fixing Kryo being instantiated w/ invalid SparkConf
#7821
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -188,7 +188,6 @@ trait ProvidesHoodieConfig extends Logging { | |||
| PRECOMBINE_FIELD.key -> preCombineField, | |||
| PARTITIONPATH_FIELD.key -> partitionFieldsStr, | |||
| PAYLOAD_CLASS_NAME.key -> payloadClassName, | |||
| HoodieWriteConfig.COMBINE_BEFORE_INSERT.key -> String.valueOf(hasPrecombineColumn), | |||
There was a problem hiding this comment.
why do we change this file for this PR?
There was a problem hiding this comment.
Stacked on top of another (for testing), will be cleaned up
| private val SERIALIZER_THREAD_LOCAL = new ThreadLocal[SerializerInstance] { | ||
| private lazy val conf = { | ||
| val conf = Option(SparkEnv.get) | ||
| // TODO elaborate |
alexeykudinkin
changed the title
SparkConfSparkConf
added 2 commits
February 1, 2023 14:55
…ts proliferation elsewhere; Tidying up
alexeykudinkin
force-pushed
the
ak/kryo-cls-ld-fix
branch
from
eef9f03
to
e99119a
Compare
4 tasks
yihua
approved these changes
Feb 2, 2023
There was a problem hiding this comment.
Looks ok to me. @alexeykudinkin we need to test the PR thoroughly before merging it. @xushiyan @YannByron could you also take another look?
| @@ -328,7 +328,7 @@ case class MergeIntoHoodieTableCommand(mergeInto: MergeIntoTable) extends Hoodie | |||
| }).toMap | |||
| // Serialize the Map[UpdateCondition, UpdateAssignments] to base64 string | |||
| val serializedUpdateConditionAndExpressions = Base64.getEncoder | |||
| .encodeToString(SerDeUtils.toBytes(updateConditionToAssignments)) | |||
| .encodeToString(Serializer.toBytes(updateConditionToAssignments)) | |||
There was a problem hiding this comment.
Does this work for all Spark versions?
There was a problem hiding this comment.
What exactly are you referring to?
Comment on lines
+472
to
+473
| private[hudi] object Serializer { | ||
|
|
There was a problem hiding this comment.
Have you tested this on all Spark versions (Spark 2.4, 3.1, 3.2, 3.3) in cluster environment (multiple nodes)?
There was a problem hiding this comment.
Checked Spark 3.1, 3.2 and 3.3, working fine
nsivabalan
pushed a commit
to nsivabalan/hudi
that referenced
this pull request
Mar 22, 2023
…ache#7821) This is addressing misconfiguration of the Kryo object used specifically to serialize Spark's internal structures (like `Expression`s): previously we're using default `SparkConf` instance to configure it, while instead we should have used the one provided by `SparkEnv`
fengjian428
pushed a commit
to fengjian428/hudi
that referenced
this pull request
Apr 5, 2023
…ache#7821) This is addressing misconfiguration of the Kryo object used specifically to serialize Spark's internal structures (like `Expression`s): previously we're using default `SparkConf` instance to configure it, while instead we should have used the one provided by `SparkEnv`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Change Logs
This is addressing misconfiguration of the Kryo object used specifically to serialize Spark's internal structures (like
Expressions): previously we're using defaultSparkConfinstance to configure it, while instead we should have used the one provided bySparkEnvImpact
Addresses NPE/ClassCastException occurring when trying to run Merge Into statements in Spark SQL
Risk level (write none, low medium or high below)
Low
Documentation Update
N/A
Contributor's checklist