Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Update Traffic Router Java version to 11#6210

Merged
rawlinp merged 23 commits intoapache:masterfrom
zrhoffman:java-11
Dec 3, 2021
Merged

Update Traffic Router Java version to 11#6210
rawlinp merged 23 commits intoapache:masterfrom
zrhoffman:java-11

Conversation

@zrhoffman
Copy link
Copy Markdown
Member

Closes: #3196

This PR makes Traffic Router use Java 11 by default for compilation, testing, and running.

Big thanks to @ARMmaster17 for getting Traffic Router itself to work using Java 11. Besides changing Java versions around the project, this PR mainly just updates PMD to a version that supports targeting Java 11, which involved fixing new PMD "failures" that PMD 6.29.0 (up from PMD 5.3.5 in maven-pmd-plugin version 3.6) finds.


Which Traffic Control components are affected by this PR?

  • Documentation
  • Traffic Router
  • Automation (GitHub Actions, Ansible Roles)

What is the best way to verify this PR?

  • Run tests using Java 11
    docker-compose -f traffic_router/tests/docker-compose.yml up --build unit_router
  • Compile Traffic Router using Java 11:
    ./pkg -bv traffic_router_build
  • Verify that Traffic Router, including the resource watchers, work as expected by running the TR external and integration tests

PR submission checklist

@zrhoffman zrhoffman added Traffic Router related to Traffic Router tech debt rework due to choosing easy/limited solution automation related to automated testing/deployment/packaging etc. dependencies Pull requests that update a dependency file ansible Related to the Ansible roles labels Sep 14, 2021
Comment thread .github/actions/build-rpms/build-rpms.sh Outdated
@zrhoffman zrhoffman requested a review from rawlinp September 15, 2021 22:08
Copy link
Copy Markdown
Contributor

@rawlinp rawlinp left a comment

Choose a reason for hiding this comment

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

I'm getting some interesting differences in performance with testing this PR. Specifically, TR with this PR appears to handle about 3365 req/s in my shadow testing environment, whereas master handles 4549 req/s when compiled and run using Java 8 and 4959 req/s when compiled using Java 8 but run with Java 11.

So, this PR appears to decrease performance by 25% in terms of req/s handled compared to master on Java 8. If we just upgrade to java 11 on the server, we get a performance increase of 9%.

@zrhoffman
Copy link
Copy Markdown
Member Author

Rebased to resolve merge conflicts

        both modified:   CHANGELOG.md
        deleted by us:   docs/source/admin/traffic_router/migrationto2-3.rst

@zrhoffman
Copy link
Copy Markdown
Member Author

Rebased to fix a merge conflict from #6278

@zrhoffman
Copy link
Copy Markdown
Member Author

Rebased to fix a merge conflict from #6303

@zrhoffman zrhoffman added the performance impacts/improves/measures performance label Nov 22, 2021
@zrhoffman
Copy link
Copy Markdown
Member Author

So, this PR appears to decrease performance by 25% in terms of req/s handled compared to master on Java 8. If we just upgrade to java 11 on the server, we get a performance increase of 9%.

There was a NullPointerException due to trying to get the socket from socketWrapper when it was null, which decreased performance significantly. Fixed in 35bd577, performance is back up for me

@zrhoffman zrhoffman requested a review from rawlinp December 3, 2021 17:52
@rawlinp rawlinp merged commit 31c4f96 into apache:master Dec 3, 2021
@zrhoffman zrhoffman deleted the java-11 branch December 3, 2021 23:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

ansible Related to the Ansible roles automation related to automated testing/deployment/packaging etc. dependencies Pull requests that update a dependency file performance impacts/improves/measures performance tech debt rework due to choosing easy/limited solution Traffic Router related to Traffic Router

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TR requirements for JDK1.8 allow OpenJDK1.7

2 participants