[HUDI-5191] Fix compatibility with avro 1.10 #7175

Zouxxyy · 2022-11-10T10:03:02Z

Change Logs

After spark3.2, avro was upgraded to 1.10.2+, some functions changed, which which can cause some test failures.
E.g:

If the Key is not found, GenericData#get will throw a AvroRuntimeException("Not a valid schema field: " + key), while it will return null before;
When Schema is initialized, the default value is checked for validity;
Schema object's size changed;

Impact

Fix test cases with avro 1.10.2+
Add hudi-commen test in CI

Risk level (write none, low medium or high below)

low

Documentation Update

None

Contributor's checklist

Read through contributor's guide
Change Logs and Impact were stated clearly
Adequate tests were added if applicable
CI passed

YannByron · 2022-11-11T02:02:24Z

nice work. Checks and CI haven't covered spark-common and spark-client yet. cc @xushiyan

YannByron · 2022-11-11T02:13:53Z

hudi-common/src/test/java/org/apache/hudi/avro/TestHoodieAvroUtils.java

@@ -253,7 +254,13 @@ public void testRemoveFields() {
    assertEquals("key1", rec1.get("_row_key"));
    assertEquals("val1", rec1.get("non_pii_col"));
    assertEquals(3.5, rec1.get("timestamp"));
-    assertNull(rec1.get("pii_col"));
+    Object removed;


maybe test by the different avro version, like:

if (HoodieAvroUtils.gteqAvro1_10()) { // assert to throw AvroRuntimeException. // some codes... } else { assertNull(rec1.get("pii_col")); }

YannByron · 2022-11-11T02:22:35Z

...rc/test/java/org/apache/hudi/common/model/TestOverwriteNonDefaultsWithLatestAvroPayload.java

-            new Schema.Field("name", Schema.createUnion(Schema.create(Schema.Type.STRING), Schema.create(Schema.Type.NULL)), "", JsonProperties.NULL_VALUE),
-            new Schema.Field("age", Schema.createUnion(Schema.create(Schema.Type.STRING), Schema.create(Schema.Type.NULL)), "", JsonProperties.NULL_VALUE),
-            new Schema.Field("job", Schema.createUnion(Schema.create(Schema.Type.STRING), Schema.create(Schema.Type.NULL)), "", JsonProperties.NULL_VALUE)
+            new Schema.Field("id", Schema.createUnion(Schema.create(Schema.Type.NULL), Schema.create(Schema.Type.STRING)), "", JsonProperties.NULL_VALUE),


why we need to change this.

After avro 1.10, the defaultValue is checked when the Schema is initialized, and here are the rules of the union

Unions, as mentioned above, are represented using JSON arrays. For example, ["null", "string"] declares a schema which may be either a null or string.
(Note that when a default value is specified for a record field whose type is a union, the type of the default value must match the first element of the union. Thus, for unions containing “null”, the “null” is usually listed first, since the default value of such unions is typically null.)

YannByron · 2022-11-11T02:22:45Z

hudi-common/src/test/java/org/apache/hudi/common/model/TestPartialUpdateAvroPayload.java

@@ -55,7 +55,7 @@ public class TestPartialUpdateAvroPayload {
      + "    {\"name\": \"id\", \"type\": [\"null\", \"string\"]},\n"
      + "    {\"name\": \"partition\", \"type\": [\"null\", \"string\"]},\n"
      + "    {\"name\": \"ts\", \"type\": [\"null\", \"long\"]},\n"
-      + "    {\"name\": \"_hoodie_is_deleted\", \"type\": [\"null\", \"boolean\"], \"default\":false},\n"
+      + "    {\"name\": \"_hoodie_is_deleted\", \"type\": [\"boolean\", \"null\"], \"default\":false},\n"


actually _hoodie_is_deleted should never be null. let's make it just boolean type?

actually _hoodie_is_deleted should never be null. let's make it just boolean type?

done

YannByron · 2022-11-11T02:24:01Z

hudi-common/src/test/java/org/apache/hudi/common/util/TestObjectSizeCalculator.java

@@ -73,7 +74,8 @@ public void testGetObjectSize() {
    assertEquals(32, getObjectSize(emptyClass));
    assertEquals(40, getObjectSize(stringClass));
    assertEquals(40, getObjectSize(payloadClass));
-    assertEquals(1240, getObjectSize(Schema.create(Schema.Type.STRING)));
+    assertEquals(HoodieAvroUtils.gteqAvro1_10() ? 1320 : 1240,


please comment the avro pr to explain this change.

YannByron · 2022-11-11T02:25:06Z

...-common/src/main/java/org/apache/hudi/common/model/debezium/AbstractDebeziumAvroPayload.java

+      Object value;
+      try {
+        value = record.get(DebeziumConstants.FLATTENED_OP_COL_NAME);
+      } catch (AvroRuntimeException e) {


maybe wrap a static method that have a param to indicate whether return null if key is not exists.

xushiyan

would be great to cover these in CI. can you add a section in .github/workflows/bot.yml before Spark SQL Test ? we should run mvn test -Punit-tests -pl hudi-common for these changes

xushiyan · 2022-11-11T15:47:57Z

hudi-common/src/test/java/org/apache/hudi/common/model/TestPartialUpdateAvroPayload.java

@@ -55,7 +55,7 @@ public class TestPartialUpdateAvroPayload {
      + "    {\"name\": \"id\", \"type\": [\"null\", \"string\"]},\n"
      + "    {\"name\": \"partition\", \"type\": [\"null\", \"string\"]},\n"
      + "    {\"name\": \"ts\", \"type\": [\"null\", \"long\"]},\n"
-      + "    {\"name\": \"_hoodie_is_deleted\", \"type\": [\"null\", \"boolean\"], \"default\":false},\n"
+      + "    {\"name\": \"_hoodie_is_deleted\", \"type\": [\"boolean\", \"null\"], \"default\":false},\n"


actually _hoodie_is_deleted should never be null. let's make it just boolean type?

Zouxxyy · 2022-11-12T03:35:56Z

would be great to cover these in CI. can you add a section in .github/workflows/bot.yml before Spark SQL Test ? we should run mvn test -Punit-tests -pl hudi-common for these changes

@xushiyan done

hudi-bot · 2022-11-12T07:40:44Z

CI report:

700eaef Azure: SUCCESS

Bot commands

@hudi-bot supports the following commands:

@hudi-bot run azure re-run the last Azure build

YannByron · 2022-11-12T15:25:20Z

LGTM. Let's land this.

[HUDI-5191] Fix avro compatibility with spark3.2+

alexeykudinkin · 2022-12-12T23:32:28Z

.github/workflows/bot.yml

@@ -73,6 +73,14 @@ jobs:
        run: |
          HUDI_VERSION=$(mvn help:evaluate -Dexpression=project.version -q -DforceStdout)
          ./packaging/bundle-validation/ci_run.sh $HUDI_VERSION
+      - name: Common Test


@Zouxxyy what are we specifically looking for to be tested in here? We need to be careful in expanding the scope here

From the cmd, it seems to test the hudi-common module specifically.

@alexeykudinkin Because the patch repairs the avro compatibility of the test cases in the hudi-common module under spark3, the test of the hudi-common module is added in bot

@Zouxxyy my point being -- hudi-common components (or tests) should not depend on particular Spark version. If something like in this case relies on particular Spark version (or other dependency that Spark brings) we should move it from hudi-common to hudi-spark

there is a deeper issue with this - hudi common is tightly coupled with avro models, which variates wrt spark profiles. currently hudi-common jar won't be compatible across all engine profiles: e.g., if built with spark3.3 (avro 1.11), it won't work with spark 2 (avro 1.8) or flink (avro 1.10). this needs to be decoupled first.

[HUDI-5191] Fix avro compatibility with spark3.2+

[5191] Fix avro compatibility with spark3.2+

82349c0

Zouxxyy changed the title ~~[5191] Fix avro compatibility with spark3.2+~~ [HUDI-5191] Fix avro compatibility with spark3.2+ Nov 10, 2022

Zouxxyy changed the title ~~[HUDI-5191] Fix avro compatibility with spark3.2+~~ [HUDI-5191] Fix avro compatibility with 1.10 Nov 10, 2022

Zouxxyy changed the title ~~[HUDI-5191] Fix avro compatibility with 1.10~~ [HUDI-5191] Fix compatibility with avro 1.10 Nov 10, 2022

update

fe4f220

YannByron reviewed Nov 11, 2022

View reviewed changes

Zouxxyy added 2 commits November 11, 2022 17:35

fix comments

05ddfad

fix

dccac34

xushiyan reviewed Nov 11, 2022

View reviewed changes

fix for comments

700eaef

apache deleted a comment from hudi-bot Nov 12, 2022

xushiyan added priority:critical production down; pipelines stalled; Need help asap. spark Issues related to spark labels Nov 12, 2022

xushiyan self-assigned this Nov 12, 2022

YannByron approved these changes Nov 12, 2022

View reviewed changes

YannByron merged commit 4da18ab into apache:master Nov 12, 2022

satishkotha pushed a commit to satishkotha/incubator-hudi that referenced this pull request Dec 12, 2022

[HUDI-5191] Fix compatibility with avro 1.10 (apache#7175)

ceb21f2

[HUDI-5191] Fix avro compatibility with spark3.2+

alexeykudinkin reviewed Dec 12, 2022

View reviewed changes

alexeykudinkin pushed a commit to onehouseinc/hudi that referenced this pull request Dec 14, 2022

[HUDI-5191] Fix compatibility with avro 1.10 (apache#7175)

060b163

[HUDI-5191] Fix avro compatibility with spark3.2+

alexeykudinkin pushed a commit to onehouseinc/hudi that referenced this pull request Dec 14, 2022

[HUDI-5191] Fix compatibility with avro 1.10 (apache#7175)

ed3c3b6

[HUDI-5191] Fix avro compatibility with spark3.2+

alexeykudinkin pushed a commit to onehouseinc/hudi that referenced this pull request Dec 14, 2022

[HUDI-5191] Fix compatibility with avro 1.10 (apache#7175)

f6f0ca7

[HUDI-5191] Fix avro compatibility with spark3.2+

alexeykudinkin pushed a commit to onehouseinc/hudi that referenced this pull request Dec 14, 2022

[HUDI-5191] Fix compatibility with avro 1.10 (apache#7175)

bbdb77e

[HUDI-5191] Fix avro compatibility with spark3.2+

fengjian428 pushed a commit to fengjian428/hudi that referenced this pull request Apr 5, 2023

[HUDI-5191] Fix compatibility with avro 1.10 (apache#7175)

462566a

[HUDI-5191] Fix avro compatibility with spark3.2+

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HUDI-5191] Fix compatibility with avro 1.10 #7175

[HUDI-5191] Fix compatibility with avro 1.10 #7175

Zouxxyy commented Nov 10, 2022 •

edited

Loading

YannByron commented Nov 11, 2022

YannByron Nov 11, 2022

Zouxxyy Nov 11, 2022

YannByron Nov 11, 2022

Zouxxyy Nov 11, 2022

YannByron Nov 11, 2022

xushiyan Nov 11, 2022

Zouxxyy Nov 12, 2022

YannByron Nov 11, 2022

Zouxxyy Nov 11, 2022

YannByron Nov 11, 2022

Zouxxyy Nov 11, 2022

xushiyan left a comment

xushiyan Nov 11, 2022

Zouxxyy commented Nov 12, 2022

hudi-bot commented Nov 12, 2022

YannByron commented Nov 12, 2022

alexeykudinkin Dec 12, 2022

danny0405 Dec 13, 2022

Zouxxyy Dec 14, 2022

alexeykudinkin Dec 14, 2022

xushiyan Dec 15, 2022

[HUDI-5191] Fix compatibility with avro 1.10 #7175

[HUDI-5191] Fix compatibility with avro 1.10 #7175

Conversation

Zouxxyy commented Nov 10, 2022 • edited Loading

Change Logs

Impact

Risk level (write none, low medium or high below)

Documentation Update

Contributor's checklist

YannByron commented Nov 11, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xushiyan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zouxxyy commented Nov 12, 2022

hudi-bot commented Nov 12, 2022

CI report:

YannByron commented Nov 12, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zouxxyy commented Nov 10, 2022 •

edited

Loading