Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CAMEL-19456] The invocation of the removeRoute() method is too slow … #10399

Merged
merged 6 commits into from
Jun 20, 2023

Conversation

Luke-hbk
Copy link
Contributor

…when using RAW().

Description

Working on that issue about "The invocation of the removeRoute() method is too slow when using RAW()."

Target

  • I checked that the commit is targeting the correct branch (note that Camel 3 uses camel-3.x, whereas Camel 4 uses the main branch)

Tracking

  • If this is a large change, bug fix, or code improvement, I checked there is a JIRA issue filed for the change (usually before you start working on it).

Apache Camel coding standards and style

  • I checked that each commit in the pull request has a meaningful subject line and body.
  • I formatted the code using mvn -Pformat,fastinstall install && mvn -Psourcecheck

@github-actions
Copy link
Contributor

🌟 Thank you for your contribution to the Apache Camel project! 🌟

🐫 Maintainers, please note that first-time contributors require manual approval for the GitHub Actions to run.

⚠️ Please note that the changes on this PR may be tested automatically if they change components.

🤖 Use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot.

If necessary Apache Camel Committers may access logs and test results in the job summaries!

Copy link
Contributor

@oscerd oscerd left a comment

Choose a reason for hiding this comment

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

Minor findings.

@Luke-hbk Luke-hbk requested a review from oscerd June 15, 2023 07:40
@github-actions
Copy link
Contributor

Components test results:

Total Tested Failed ❌ Passed ✅
7 7 1 6

@github-actions
Copy link
Contributor

Core test results:

Tested Failed ❌ Passed ✅
1 1 0

Copy link
Contributor

@orpiske orpiske left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution. Performance changes are always interesting.

I am requesting just a minor set of fixes.

@orpiske
Copy link
Contributor

orpiske commented Jun 15, 2023

Also, out of curiosity: do you have any numbers? A JMH test, maybe?

@Luke-hbk
Copy link
Contributor Author

Luke-hbk commented Jun 16, 2023

I apologize, but I am currently in the process of making some changes to the code. I will commit it again after the modifications are complete.

@orpiske
Copy link
Contributor

orpiske commented Jun 16, 2023

I apologize, but I am currently in the process of making some changes to the code. I will commit it again after the modifications are complete.

No worries, we are happy to receive contributions! Looking forward to the updated one!

@Luke-hbk Luke-hbk requested a review from orpiske June 17, 2023 19:03
Copy link
Contributor

@orpiske orpiske left a comment

Choose a reason for hiding this comment

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

Thanks for doing the changes. There's one problem with star imports, but I am fine with the rest of the approach.

So, please, just adjust the star imports I mentioned and, IMHO, it should be good to go.

@github-actions
Copy link
Contributor

Components test results:

Total Tested Failed ❌ Passed ✅
7 7 7 0

@github-actions
Copy link
Contributor

Core test results:

Tested Failed ❌ Passed ✅
1 1 0

@github-actions
Copy link
Contributor

Components test results:

Total Tested Failed ❌ Passed ✅
7 7 7 0

@github-actions
Copy link
Contributor

Core test results:

Tested Failed ❌ Passed ✅
1 1 0

@github-actions
Copy link
Contributor

Components test results:

Total Tested Failed ❌ Passed ✅
7 7 7 0

@github-actions
Copy link
Contributor

Core test results:

Tested Failed ❌ Passed ✅
1 1 0

@Luke-hbk
Copy link
Contributor Author

Luke-hbk commented Jun 20, 2023

Unfortunately, the tests keep failing, so I decided to check the main branch without making any modifications, and it turns out that an error occurs in camel-core.

[ERROR] Failures:
[ERROR] RecipientListWithSimpleExpressionTest.testRecipientList:66->ContextTestSupport.assertMockEndpointsSatisfied:357 mock://0 Received message count. Expected: <50> but was: <49>
[ERROR] RecipientListWithSimpleExpressionTest.testStatic:119->ContextTestSupport.assertMockEndpointsSatisfied:357 mock://8 Received message count. Expected: <50> but was: <48>
[INFO]
[ERROR] Tests run: 6100, Failures: 2, Errors: 0, Skipped: 36

[INFO] Camel :: Core ...................................... FAILURE [03:47 min]

@orpiske
Can I assist you in resolving the test failures?

@orpiske
Copy link
Contributor

orpiske commented Jun 20, 2023

@Luke-hbk yes, I will take a look today.

@orpiske
Copy link
Contributor

orpiske commented Jun 20, 2023

@Luke-hbk It looks fine here. It seems like a flaky test - We still have a few as we are working on the final bits of Camel 4.

The PR should be ready to go (c/c @oscerd).

@orpiske
Copy link
Contributor

orpiske commented Jun 20, 2023

Let's get this one in. It would be good to have some tests for it on the CI. Merging ...

@orpiske orpiske merged commit e63792f into apache:main Jun 20, 2023
3 of 6 checks passed
@orpiske
Copy link
Contributor

orpiske commented Jun 20, 2023

Thanks for your contribution @Luke-hbk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants