[ 
https://issues.apache.org/jira/browse/ARTEMIS-4668?focusedWorklogId=908357&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-908357
 ]

ASF GitHub Bot logged work on ARTEMIS-4668:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 05/Mar/24 15:42
            Start Date: 05/Mar/24 15:42
    Worklog Time Spent: 10m 
      Work Description: tabish121 commented on code in PR #4840:
URL: https://github.com/apache/activemq-artemis/pull/4840#discussion_r1513036542


##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPLargeMessageReader.java:
##########
@@ -77,39 +73,73 @@ public AMQPLargeMessageReader open() {
    }
 
    @Override
-   public Message readBytes(Delivery delivery) throws Exception {
+   public boolean readBytes(Delivery delivery) throws Exception {
       if (closed) {
          throw new IllegalStateException("AMQP Large Message Reader is closed 
and read cannot proceed");
       }
 
+      serverReceiver.connection.requireInHandler();
+
       final Receiver receiver = ((Receiver) delivery.getLink());
       final ReadableBuffer dataBuffer = receiver.recv();
 
+      final AMQPSessionCallback sessionSPI = 
serverReceiver.getSessionContext().getSessionSPI();
+
       if (currentMessage == null) {
-         final AMQPSessionCallback sessionSPI = 
serverReceiver.getSessionContext().getSessionSPI();
          final long id = sessionSPI.getStorageManager().generateID();
          currentMessage = new AMQPLargeMessage(id, 
delivery.getMessageFormat(), null,
                                                
sessionSPI.getCoreMessageObjectPools(),
                                                sessionSPI.getStorageManager());
          currentMessage.parseHeader(dataBuffer);
-
+         logger.trace("Initializing current message {} on {}", currentMessage, 
this);
          sessionSPI.getStorageManager().largeMessageCreated(id, 
currentMessage);
+         sessionSPI.execute(() -> validateFile(currentMessage));

Review Comment:
   This check seems to be mostly pointless now as the code doesn't use a 
continuation to resume delivery after validation it just throws it into another 
thread and then continues on as if it was successful  



##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPTunneledCoreMessageReader.java:
##########
@@ -155,6 +149,10 @@ public Message readBytes(Delivery delivery) {
       coreMessage.reloadPersistence(buffer, 
sessionSPI.getCoreMessageObjectPools());
       coreMessage.setMessageID(sessionSPI.getStorageManager().generateID());
 
-      return coreMessage;
+      serverReceiver.onMessageComplete(delivery, coreMessage, 
deliveryAnnotations);
+
+      close();

Review Comment:
   The receiver should be closing this in onMessageComplete, a reader shouldn't 
close itself. 





Issue Time Tracking
-------------------

    Worklog Id:     (was: 908357)
    Time Spent: 2h 50m  (was: 2h 40m)

> Move AMQP Large Message File Handling away from Netty thread
> ------------------------------------------------------------
>
>                 Key: ARTEMIS-4668
>                 URL: https://issues.apache.org/jira/browse/ARTEMIS-4668
>             Project: ActiveMQ Artemis
>          Issue Type: Bug
>    Affects Versions: 2.32.0
>            Reporter: Clebert Suconic
>            Assignee: Clebert Suconic
>            Priority: Major
>             Fix For: 2.33.0
>
>          Time Spent: 2h 50m
>  Remaining Estimate: 0h
>
> Operations like file.open, file.close, and file.sync should be moved away 
> from the Netty Thread for AMQP Large Messages
> This task now is about moving the processing for AMQP Messages. we may in a 
> near future also improve tunneled large messages. For now we will do for AMQP 
> messages only.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to