You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@james.apache.org by "chibenwa (via GitHub)" <gi...@apache.org> on 2024/02/18 20:47:13 UTC

[PR] [FIX] Mime parsing on parallel scheduler [james-project]

chibenwa opened a new pull request, #2015:
URL: https://github.com/apache/james-project/pull/2015

   For 20MB + files this takes time.
   This approach avoids over-switching by doing it
   only after fetching items...


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org

Re: [PR] [FIX] Mime parsing on parallel scheduler [james-project]

Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.

quantranhong1999 commented on PR #2015:
URL: https://github.com/apache/james-project/pull/2015#issuecomment-1952157029

   > If not doable then let's BLOCKING_CALL_WRAPPER this...
   
   The thing is `.publishOn(parallel)` had a higher priority (if am not wrong) and made the latter `.subscribeOn(BLOCKING_CALL_WRAPPER)` have no impact.
   
   Anyway just my quick look, I can try again later.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org

Re: [PR] [FIX] Mime parsing on parallel scheduler [james-project]

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.

Arsnael commented on code in PR #2015:
URL: https://github.com/apache/james-project/pull/2015#discussion_r1493992542


##########
mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageIdManager.java:
##########
@@ -172,16 +174,25 @@ public List<MessageResult> getMessages(Collection<MessageId> messageIds, FetchGr
 
     @Override
     public Flux<MessageResult> getMessagesReactive(Collection<MessageId> messageIds, FetchGroup fetchGroup, MailboxSession mailboxSession) {
+        // Here

Review Comment:
   Unnecessary comment?



##########
mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageIdManager.java:
##########
@@ -172,16 +174,25 @@ public List<MessageResult> getMessages(Collection<MessageId> messageIds, FetchGr
 
     @Override
     public Flux<MessageResult> getMessagesReactive(Collection<MessageId> messageIds, FetchGroup fetchGroup, MailboxSession mailboxSession) {
+        // Here
         MessageIdMapper messageIdMapper = mailboxSessionMapperFactory.getMessageIdMapper(mailboxSession);
 
         MessageMapper.FetchType fetchType = FetchGroupConverter.getFetchType(fetchGroup);
         return messageIdMapper.findReactive(messageIds, fetchType)
             .groupBy(MailboxMessage::getMailboxId)
             .filterWhen(groupedFlux -> hasRightsOnMailboxReactive(mailboxSession, Right.Read).apply(groupedFlux.key()), DEFAULT_CONCURRENCY)
             .flatMap(Function.identity(), DEFAULT_CONCURRENCY)
+            .publishOn(forFetchType(fetchType))
             .map(Throwing.function(messageResultConverter(fetchGroup)).sneakyThrow());
     }
 
+    private Scheduler forFetchType(MessageMapper.FetchType fetchType) {

Review Comment:
   Duplicate. Do you think this could have its place in `MailboxReactorUtils` for example?
   
   Not a big deal though



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org

Re: [PR] [FIX] Mime parsing on parallel scheduler [james-project]

Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.

quantranhong1999 commented on PR #2015:
URL: https://github.com/apache/james-project/pull/2015#issuecomment-1951876588

   > Existing failures - 1
   importShouldSucceedWhenBlobDelegated(GuiceJamesServer) – org.apache.james.jmap.rfc8621.memory.MemoryEmailImportTest
   
   ```json
       "methodResponses": [
           [
               "Email/import",
               {
                   "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
                   "oldState": "2c9f1b12-b35a-43e6-9af2-0106fb53a943",
                   "newState": "2c9f1b12-b35a-43e6-9af2-0106fb53a943",
                   "notCreated": {
                       "C42": {
                           "type": "serverFail",
                           "description": "block()/blockFirst()/blockLast() are blocking, which is not supported in thread parallel-4"
                       }
                   }
               },
               "c1"
           ]
       ]
   ```
   
   Reason: code after  `.getMessagesReactive` executes on the parallel threads, while James code still has some parts that are blocking and should be executed on the blocking thread instead of the parallel thread.
   
   I am not sure this change is easily achievable.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org

Re: [PR] [FIX] Mime parsing on parallel scheduler [james-project]

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.

Arsnael commented on PR #2015:
URL: https://github.com/apache/james-project/pull/2015#issuecomment-1958674291

   ```
   00:57:22,351 [ERROR] Failed to execute goal io.github.evis:scalafix-maven-plugin_2.13:0.1.8_0.11.0:scalafix (scala-check-style) on project james-server-jmap-rfc-8621: Scalafix invoked with errors. Check logs for details. -> [Help 1]
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org