Skip to content

[improve][broker] Upgrade avro version to 1.12.0 - #24617

Merged
Technoboy- merged 7 commits into
apache:masterfrom
hangc0276:chenhang/upgrade_avro_to_1.12.0
Aug 14, 2025
Merged

[improve][broker] Upgrade avro version to 1.12.0#24617
Technoboy- merged 7 commits into
apache:masterfrom
hangc0276:chenhang/upgrade_avro_to_1.12.0

Conversation

@hangc0276

@hangc0276 hangc0276 commented Aug 9, 2025

Copy link
Copy Markdown
Contributor

Fixes #xyz

Main Issue: #xyz

PIP: #xyz

Motivation

Upgrade the Avro version.

Modifications

Verifying this change

  • Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

  • Added integration tests for end-to-end deployment with large payloads (10MB)
  • Extended integration test for recovery after broker failure

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository:

@github-actions

github-actions Bot commented Aug 9, 2025

Copy link
Copy Markdown

@hangc0276 Please add the following content to your PR description and select a checkbox:

- [ ] `doc` <!-- Your PR contains doc changes -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->

@hangc0276 hangc0276 added dependencies Pull requests that update a dependency file release/4.0.7 doc-not-needed Your PR changes do not impact docs ready-to-test and removed doc-label-missing labels Aug 9, 2025
@github-actions github-actions Bot added doc-label-missing doc-not-needed Your PR changes do not impact docs and removed doc-not-needed Your PR changes do not impact docs doc-label-missing labels Aug 9, 2025

@horizonzy horizonzy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@lhotari

lhotari commented Aug 9, 2025

Copy link
Copy Markdown
Member

@hangc0276 It seems that there are test failures:

AvroSchemaTest.testTimestampWithJsonDef: pulsar-client/src/test/java/org/apache/pulsar/client/impl/schema/AvroSchemaTest.java#L515Can not set java.time.Instant field org.apache.pulsar.client.impl.schema.AvroSchemaTest$TimestampPojo.value to java.lang.Long

CI - Unit - Pulsar ClientProcess completed with exit code 1.
TopicSchemaTest.testGetSchema: pulsar-broker/src/test/java/org/apache/pulsar/broker/service/schema/TopicSchemaTest.java#L48Namespace part "Request$ServiceRequest" is invalid: Illegal character in: Request$ServiceRequest
CI - Unit - OtherProcess completed with exit code 1.
SchemaDataValidatorTest.testJsonSchemaTypeWithJsonSchemaData: pulsar-broker/src/test/java/org/apache/pulsar/broker/service/schema/validator/SchemaDataValidatorTest.java#L134Invalid schema definition data for JSON schema

AvroSchemaTest.testTimestampWithJsonDef: pulsar-client/src/test/java/org/apache/pulsar/client/impl/schema/AvroSchemaTest.java#L515
Can not set java.time.Instant field org.apache.pulsar.client.impl.schema.AvroSchemaTest$TimestampPojo.value to java.lang.Long
CI - Unit - Pulsar Client
Process completed with exit code 1.
TopicSchemaTest.testGetSchema: pulsar-broker/src/test/java/org/apache/pulsar/broker/service/schema/TopicSchemaTest.java#L48
Namespace part "Request$ServiceRequest" is invalid: Illegal character in: Request$ServiceRequest
CI - Unit - Other
Process completed with exit code 1.
SchemaDataValidatorTest.testJsonSchemaTypeWithJsonSchemaData: pulsar-broker/src/test/java/org/apache/pulsar/broker/service/schema/validator/SchemaDataValidatorTest.java#L134
Invalid schema definition data for JSON schema

@Technoboy-

Technoboy- commented Aug 12, 2025

Copy link
Copy Markdown
Contributor

@coderzc For AvroSchemaTest.testTimestampWithJsonDef, in Avro 1.11.4, it use Unsafe.putObject(object, offset, value) to set the object value when decoding. But in 1.12.0, it changed to use Field.set(object, value), the later one could check the object type, so it could throw below error

Can not set java.time.Instant field org.apache.pulsar.client.impl.schema.AvroSchemaTest$TimestampPojo.value to java.lang.Long

can we delete this assertion, as it may not involve break changes ?

AvroSchema<TimestampPojo> schemaWithJsonDefNoClassLoader =
AvroSchema.of(SchemaDefinition.<TimestampPojo>builder()
.withJsonDef(schemaDefinition)
.withJSR310ConversionEnabled(false).build());
TimestampPojo decodeWithJsonNoClassLoader = schemaWithJsonDefNoClassLoader.decode(encode);
Assert.assertNotEquals(decodeWithJsonNoClassLoader, decodeWithPojo);
Assert.assertNotEquals(Instant.class, decodeWithJsonNoClassLoader.getValue().getClass());

@coderzc

coderzc commented Aug 12, 2025

Copy link
Copy Markdown
Member

@coderzc For AvroSchemaTest.testTimestampWithJsonDef, in Avro 1.11.4, it use Unsafe.putObject(object, offset, value) to set the object value when decoding. But in 1.12.0, it changed to use Field.set(object, value), the later one could check the object type, so it could throw below error

Can not set java.time.Instant field org.apache.pulsar.client.impl.schema.AvroSchemaTest$TimestampPojo.value to java.lang.Long

can we delete this assertion, as it may not involve break changes ?

AvroSchema<TimestampPojo> schemaWithJsonDefNoClassLoader =
AvroSchema.of(SchemaDefinition.<TimestampPojo>builder()
.withJsonDef(schemaDefinition)
.withJSR310ConversionEnabled(false).build());
TimestampPojo decodeWithJsonNoClassLoader = schemaWithJsonDefNoClassLoader.decode(encode);
Assert.assertNotEquals(decodeWithJsonNoClassLoader, decodeWithPojo);
Assert.assertNotEquals(Instant.class, decodeWithJsonNoClassLoader.getValue().getClass());

I think we can delete this assertion, this should be a misuse, If you want to customize the conversion, you can set the classLoader.

@Technoboy- Technoboy- added this to the 4.1.0 milestone Aug 12, 2025
@Technoboy-

Copy link
Copy Markdown
Contributor

@gaoran10 Please help review the fix of SchemaDataValidatorTest

@codecov-commenter

codecov-commenter commented Aug 13, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.31%. Comparing base (3d57c60) to head (a93db06).
⚠️ Report is 33 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #24617       +/-   ##
=============================================
+ Coverage     38.80%   74.31%   +35.50%     
- Complexity    12984    33165    +20181     
=============================================
  Files          1824     1881       +57     
  Lines        142741   146850     +4109     
  Branches      16389    16866      +477     
=============================================
+ Hits          55392   109129    +53737     
+ Misses        79916    29056    -50860     
- Partials       7433     8665     +1232     
Flag Coverage Δ
inttests 26.65% <50.00%> (-0.20%) ⬇️
systests 23.26% <16.66%> (-0.04%) ⬇️
unittests 73.80% <100.00%> (+39.18%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...r/service/schema/JsonSchemaCompatibilityCheck.java 47.05% <100.00%> (+17.64%) ⬆️
...che/pulsar/client/impl/schema/util/SchemaUtil.java 80.55% <100.00%> (+8.33%) ⬆️
.../apache/pulsar/io/elasticsearch/JsonConverter.java 71.55% <100.00%> (+71.55%) ⬆️
...g/apache/pulsar/io/kinesis/json/JsonConverter.java 67.96% <100.00%> (+67.96%) ⬆️

... and 1380 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Technoboy-
Technoboy- merged commit baa288a into apache:master Aug 14, 2025
50 of 51 checks passed
Technoboy- added a commit to Technoboy-/pulsar that referenced this pull request Aug 14, 2025
Co-authored-by: Jiwe Guo <technoboy@apache.org>
@hangc0276

Copy link
Copy Markdown
Contributor Author

When I cherry-pick this PR to branch-4.0, this project build failed with following exception:

[ERROR] Rule 0: org.apache.maven.plugins.enforcer.EnforceBytecodeVersion failed with message: 
[ERROR] Found Banned Dependency: org.apache.avro:avro-protobuf:jar:1.12.0 
[ERROR] Found Banned Dependency: org.apache.avro:avro:jar:1.12.0 
[ERROR] Use 'mvn dependency:tree' to locate the source of the banned dependencies.

Looks like related to branch-4.0 pulsar client enforced bytecode version and the maxJdkVersion is set to 8

      <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-enforcer-plugin</artifactId>
        <version>${maven-enforcer-plugin.version}</version>
        <executions>
          <execution>
            <id>enforce-bytecode-version</id>
            <goals>
              <goal>enforce</goal>
            </goals>
            <configuration>
              <rules>
                <enforceBytecodeVersion>
                  <maxJdkVersion>${pulsar.client.compiler.release}</maxJdkVersion>
                  <ignoredScopes>
                    <ignoreScope>test</ignoreScope>
                  </ignoredScopes>
                </enforceBytecodeVersion>
              </rules>
            </configuration>
          </execution>
        </executions>

Technoboy- added a commit that referenced this pull request Aug 18, 2025
Co-authored-by: Jiwe Guo <technoboy@apache.org>
Technoboy- added a commit that referenced this pull request Aug 18, 2025
@Technoboy-

Copy link
Copy Markdown
Contributor

When I cherry-pick this PR to branch-4.0, this project build failed with following exception:

[ERROR] Rule 0: org.apache.maven.plugins.enforcer.EnforceBytecodeVersion failed with message: 
[ERROR] Found Banned Dependency: org.apache.avro:avro-protobuf:jar:1.12.0 
[ERROR] Found Banned Dependency: org.apache.avro:avro:jar:1.12.0 
[ERROR] Use 'mvn dependency:tree' to locate the source of the banned dependencies.

Looks like related to branch-4.0 pulsar client enforced bytecode version and the maxJdkVersion is set to 8

      <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-enforcer-plugin</artifactId>
        <version>${maven-enforcer-plugin.version}</version>
        <executions>
          <execution>
            <id>enforce-bytecode-version</id>
            <goals>
              <goal>enforce</goal>
            </goals>
            <configuration>
              <rules>
                <enforceBytecodeVersion>
                  <maxJdkVersion>${pulsar.client.compiler.release}</maxJdkVersion>
                  <ignoredScopes>
                    <ignoreScope>test</ignoreScope>
                  </ignoredScopes>
                </enforceBytecodeVersion>
              </rules>
            </configuration>
          </execution>
        </executions>

Reverted from branch-4.0.

manas-ctds pushed a commit to datastax/pulsar that referenced this pull request Aug 21, 2025
Co-authored-by: Jiwe Guo <technoboy@apache.org>
(cherry picked from commit 36cd2c2)
manas-ctds pushed a commit to datastax/pulsar that referenced this pull request Aug 21, 2025
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Aug 26, 2025
Co-authored-by: Jiwe Guo <technoboy@apache.org>
(cherry picked from commit 36cd2c2)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Aug 26, 2025
Technoboy- added a commit to Technoboy-/pulsar that referenced this pull request Sep 10, 2025
nborisov pushed a commit to nborisov/pulsar that referenced this pull request Sep 12, 2025
KannarFr pushed a commit to CleverCloud/pulsar that referenced this pull request Sep 22, 2025
Co-authored-by: Jiwe Guo <technoboy@apache.org>
walkinggo pushed a commit to walkinggo/pulsar that referenced this pull request Oct 8, 2025
Co-authored-by: Jiwe Guo <technoboy@apache.org>
@lhotari

lhotari commented Feb 24, 2026

Copy link
Copy Markdown
Member

branch-4.0 cherry-pick tag removed. this PR was reverted from branch-4.0 in 80e68e1

lhotari added a commit that referenced this pull request May 15, 2026
The invalid test was removed by #24617 for Pulsar 4.1.0
lhotari added a commit that referenced this pull request May 15, 2026
The invalid test was removed by #24617 for Pulsar 4.1.0
nodece pushed a commit to ascentstream/pulsar that referenced this pull request May 19, 2026
nodece pushed a commit to ascentstream/pulsar that referenced this pull request May 27, 2026
manas-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 1, 2026
The invalid test was removed by apache#24617 for Pulsar 4.1.0

(cherry picked from commit d8dbb09)
priyanshu-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 9, 2026
The invalid test was removed by apache#24617 for Pulsar 4.1.0

(cherry picked from commit 578e782)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file doc-not-needed Your PR changes do not impact docs ready-to-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants