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.

Sign up for GitHub

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

Jump to bottom

ARTEMIS-4668 Moving AMQP Large Message file handling away from Netty thread #4840

Merged
merged 1 commit into from
Mar 6, 2024
Merged

ARTEMIS-4668 Moving AMQP Large Message file handling away from Netty thread #4840

merged 1 commit into from
Mar 6, 2024

Conversation

clebertsuconic
Copy link
Contributor

No description provided.

@clebertsuconic clebertsuconic force-pushed the ARTEMIS-4668 branch 2 times, most recently from 1ec077f to 274e93b Compare March 4, 2024 23:17
tabish121
tabish121 reviewed Mar 4, 2024
View reviewed changes
Copy link
Contributor

@tabish121 tabish121 left a comment

Choose a reason for hiding this comment

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

There are a number of unused imports left scattered about now that need cleaned up besides the items listed in the review.

...ol/src/main/java/org/apache/activemq/artemis/protocol/amqp/connect/AMQPBrokerConnection.java Outdated Show resolved Hide resolved
...he/activemq/artemis/protocol/amqp/connect/federation/AMQPFederationBaseSenderController.java Outdated Show resolved Hide resolved
...ol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPConnectionContext.java Outdated Show resolved Hide resolved
...l/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPLargeMessageReader.java Outdated Show resolved Hide resolved
...l/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPLargeMessageReader.java Outdated Show resolved Hide resolved
...rtemis-test-support/src/main/java/org/apache/activemq/transport/amqp/client/AmqpMessage.java Outdated Show resolved Hide resolved
...l/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPLargeMessageWriter.java Outdated Show resolved Hide resolved
...l/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPLargeMessageWriter.java Show resolved Hide resolved
...l/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPLargeMessageReader.java Outdated Show resolved Hide resolved
...l/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPLargeMessageReader.java Outdated Show resolved Hide resolved
@clebertsuconic clebertsuconic force-pushed the ARTEMIS-4668 branch 12 times, most recently from 82ffae6 to 8f9e629 Compare March 5, 2024 14:40
gemmellr
gemmellr reviewed Mar 5, 2024
View reviewed changes
...ient/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyConnection.java Outdated Show resolved Hide resolved
...ol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPConnectionContext.java Outdated Show resolved Hide resolved
...ol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPConnectionContext.java Outdated Show resolved Hide resolved
...l/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPLargeMessageReader.java Outdated Show resolved Hide resolved
...l/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPLargeMessageReader.java Outdated Show resolved Hide resolved
...l/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPLargeMessageWriter.java Outdated Show resolved Hide resolved
...ava/org/apache/activemq/artemis/protocol/amqp/proton/AMQPTunneledCoreLargeMessageReader.java Outdated Show resolved Hide resolved
...ain/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPTunneledCoreMessageReader.java Outdated Show resolved Hide resolved
...p-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/MessageReader.java Outdated Show resolved Hide resolved
...n/java/org/apache/activemq/artemis/core/persistence/impl/journal/LargeServerMessageImpl.java Outdated Show resolved Hide resolved
@clebertsuconic clebertsuconic marked this pull request as draft March 5, 2024 17:58
@clebertsuconic
Copy link
Contributor Author

@gemmellr , @tabish121 I added 3 temporary commits to this PR. Let me know if you have any questions and I will squash them before setting it ready to merge.

@clebertsuconic
Copy link
Contributor Author

I'm thinking on reverting some of the readBytes(Delivery) methods and only call the onMessageComplete for the Large Message case.

will be working on it now.

@clebertsuconic clebertsuconic force-pushed the ARTEMIS-4668 branch 5 times, most recently from 6e2966d to a752e17 Compare March 5, 2024 22:28
@clebertsuconic clebertsuconic marked this pull request as ready for review March 5, 2024 22:28
@clebertsuconic
Copy link
Contributor Author

@tabish121 , @gemmellr I tried simplifying the changes as much as possible here...

@clebertsuconic clebertsuconic force-pushed the ARTEMIS-4668 branch 4 times, most recently from 9dad3b6 to 9812496 Compare March 6, 2024 15:45
@clebertsuconic clebertsuconic force-pushed the ARTEMIS-4668 branch 2 times, most recently from 5329988 to 4a2afbf Compare March 6, 2024 17:07
@clebertsuconic clebertsuconic merged commit c83ed89 into apache:main Mar 6, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tabish121 tabish121 tabish121 left review comments

@gemmellr gemmellr gemmellr left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@clebertsuconic @tabish121 @gemmellr