You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by bt...@apache.org on 2020/10/26 05:20:15 UTC
[james-project] 02/08: JAMES-3277 Optimization: JMAP Rely on
MessageManager::moveMessage (Draft)
This is an automated email from the ASF dual-hosted git repository.
btellier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 0b05fa355e1d9c85e0f16f3ecfd241f7dbb823e3
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Thu Oct 22 11:42:48 2020 +0700
JAMES-3277 Optimization: JMAP Rely on MessageManager::moveMessage (Draft)
We notice that many slow traces at the JMAP level are message moves updates, generating a high count of Cassandra queries.
Query count before: 22m+12
- 6 message => 146 queries
- 9 messages => 210 queries
- 12 messages => 276 queries
Query count after: 16m+15
- 6 message => 111 queries
- 9 messages => 159 queries
- 12 messages => 207 queries
---
.../org/apache/james/mailbox/MailboxManager.java | 2 +
.../james/mailbox/store/StoreMailboxManager.java | 8 +++
.../methods/integration/SetMessagesMethodTest.java | 74 ++++++++++++++++++++++
.../draft/methods/SetMessagesUpdateProcessor.java | 60 ++++++++++++++++++
.../james/jmap/draft/model/UpdateMessagePatch.java | 6 ++
5 files changed, 150 insertions(+)
diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/MailboxManager.java b/mailbox/api/src/main/java/org/apache/james/mailbox/MailboxManager.java
index 01740b8..a175127 100644
--- a/mailbox/api/src/main/java/org/apache/james/mailbox/MailboxManager.java
+++ b/mailbox/api/src/main/java/org/apache/james/mailbox/MailboxManager.java
@@ -289,6 +289,8 @@ public interface MailboxManager extends RequestAware, RightManager, MailboxAnnot
*/
List<MessageRange> moveMessages(MessageRange set, MailboxPath from, MailboxPath to, MailboxSession session) throws MailboxException;
+ List<MessageRange> moveMessages(MessageRange set, MailboxId from, MailboxId to, MailboxSession session) throws MailboxException;
+
enum MailboxSearchFetchType {
Minimal,
Counters
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java
index 2ca6eb0..97c1972 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java
@@ -623,6 +623,14 @@ public class StoreMailboxManager implements MailboxManager {
}
@Override
+ public List<MessageRange> moveMessages(MessageRange set, MailboxId from, MailboxId to, MailboxSession session) throws MailboxException {
+ StoreMessageManager toMailbox = (StoreMessageManager) getMailbox(to, session);
+ StoreMessageManager fromMailbox = (StoreMessageManager) getMailbox(from, session);
+
+ return configuration.getMoveBatcher().batchMessages(set, messageRange -> fromMailbox.moveTo(messageRange, toMailbox, session));
+ }
+
+ @Override
public Flux<MailboxMetaData> search(MailboxQuery expression, MailboxSearchFetchType fetchType, MailboxSession session) {
Mono<List<Mailbox>> mailboxesMono = searchMailboxes(expression, session, Right.Lookup).collectList();
diff --git a/server/protocols/jmap-draft-integration-testing/jmap-draft-integration-testing-common/src/test/java/org/apache/james/jmap/draft/methods/integration/SetMessagesMethodTest.java b/server/protocols/jmap-draft-integration-testing/jmap-draft-integration-testing-common/src/test/java/org/apache/james/jmap/draft/methods/integration/SetMessagesMethodTest.java
index 2fbed4fff..fc5d16a 100644
--- a/server/protocols/jmap-draft-integration-testing/jmap-draft-integration-testing-common/src/test/java/org/apache/james/jmap/draft/methods/integration/SetMessagesMethodTest.java
+++ b/server/protocols/jmap-draft-integration-testing/jmap-draft-integration-testing-common/src/test/java/org/apache/james/jmap/draft/methods/integration/SetMessagesMethodTest.java
@@ -1130,6 +1130,80 @@ public abstract class SetMessagesMethodTest {
.body(FIRST_MAILBOX + ".unreadMessages", equalTo(0));
}
+ @Test
+ public void massiveMessageMoveShouldBeApplied() throws MailboxException {
+ mailboxProbe.createMailbox(MailboxConstants.USER_NAMESPACE, USERNAME.asString(), "mailbox");
+ mailboxProbe.createMailbox(MailboxConstants.USER_NAMESPACE, USERNAME.asString(), DefaultMailboxes.DRAFTS);
+ mailboxProbe.createMailbox(MailboxConstants.USER_NAMESPACE, USERNAME.asString(), DefaultMailboxes.OUTBOX);
+ mailboxProbe.createMailbox(MailboxConstants.USER_NAMESPACE, USERNAME.asString(), DefaultMailboxes.SENT);
+ MailboxId mailboxId = mailboxProbe.createMailbox(MailboxConstants.USER_NAMESPACE, USERNAME.asString(), DefaultMailboxes.TRASH);
+ mailboxProbe.createMailbox(MailboxConstants.USER_NAMESPACE, USERNAME.asString(), DefaultMailboxes.SPAM);
+
+ ComposedMessageId message1 = mailboxProbe.appendMessage(USERNAME.asString(), USER_MAILBOX,
+ new ByteArrayInputStream("Subject: test\r\n\r\ntestmail".getBytes(StandardCharsets.UTF_8)), new Date(), false, new Flags());
+ ComposedMessageId message2 = mailboxProbe.appendMessage(USERNAME.asString(), USER_MAILBOX,
+ new ByteArrayInputStream("Subject: test\r\n\r\ntestmail".getBytes(StandardCharsets.UTF_8)), new Date(), false, new Flags());
+ ComposedMessageId message3 = mailboxProbe.appendMessage(USERNAME.asString(), USER_MAILBOX,
+ new ByteArrayInputStream("Subject: test\r\n\r\ntestmail".getBytes(StandardCharsets.UTF_8)), new Date(), false, new Flags(Flags.Flag.SEEN));
+ ComposedMessageId message4 = mailboxProbe.appendMessage(USERNAME.asString(), USER_MAILBOX,
+ new ByteArrayInputStream("Subject: test\r\n\r\ntestmail".getBytes(StandardCharsets.UTF_8)), new Date(), false, new Flags());
+ ComposedMessageId message5 = mailboxProbe.appendMessage(USERNAME.asString(), USER_MAILBOX,
+ new ByteArrayInputStream("Subject: test\r\n\r\ntestmail".getBytes(StandardCharsets.UTF_8)), new Date(), false, new Flags(Flags.Flag.ANSWERED));
+ ComposedMessageId message6 = mailboxProbe.appendMessage(USERNAME.asString(), USER_MAILBOX,
+ new ByteArrayInputStream("Subject: test\r\n\r\ntestmail".getBytes(StandardCharsets.UTF_8)), new Date(), false, new Flags());
+ ComposedMessageId message7 = mailboxProbe.appendMessage(USERNAME.asString(), USER_MAILBOX,
+ new ByteArrayInputStream("Subject: test\r\n\r\ntestmail".getBytes(StandardCharsets.UTF_8)), new Date(), false, new Flags());
+ ComposedMessageId message8 = mailboxProbe.appendMessage(USERNAME.asString(), USER_MAILBOX,
+ new ByteArrayInputStream("Subject: test\r\n\r\ntestmail".getBytes(StandardCharsets.UTF_8)), new Date(), false, new Flags());
+ ComposedMessageId message9 = mailboxProbe.appendMessage(USERNAME.asString(), USER_MAILBOX,
+ new ByteArrayInputStream("Subject: test\r\n\r\ntestmail".getBytes(StandardCharsets.UTF_8)), new Date(), false, new Flags(Flags.Flag.SEEN));
+ ComposedMessageId message10 = mailboxProbe.appendMessage(USERNAME.asString(), USER_MAILBOX,
+ new ByteArrayInputStream("Subject: test\r\n\r\ntestmail".getBytes(StandardCharsets.UTF_8)), new Date(), false, new Flags());
+ ComposedMessageId message11 = mailboxProbe.appendMessage(USERNAME.asString(), USER_MAILBOX,
+ new ByteArrayInputStream("Subject: test\r\n\r\ntestmail".getBytes(StandardCharsets.UTF_8)), new Date(), false, new Flags(Flags.Flag.ANSWERED));
+ ComposedMessageId message12 = mailboxProbe.appendMessage(USERNAME.asString(), USER_MAILBOX,
+ new ByteArrayInputStream("Subject: test\r\n\r\ntestmail".getBytes(StandardCharsets.UTF_8)), new Date(), false, new Flags());
+
+ String serializedMessageId1 = message1.getMessageId().serialize();
+ String serializedMessageId2 = message2.getMessageId().serialize();
+ String serializedMessageId3 = message3.getMessageId().serialize();
+ String serializedMessageId4 = message4.getMessageId().serialize();
+ String serializedMessageId5 = message5.getMessageId().serialize();
+ String serializedMessageId6 = message6.getMessageId().serialize();
+ String serializedMessageId7 = message7.getMessageId().serialize();
+ String serializedMessageId8 = message8.getMessageId().serialize();
+ String serializedMessageId9 = message9.getMessageId().serialize();
+ String serializedMessageId10 = message10.getMessageId().serialize();
+ String serializedMessageId11 = message11.getMessageId().serialize();
+ String serializedMessageId12 = message12.getMessageId().serialize();
+
+ // When
+ given()
+ .header("Authorization", accessToken.asString())
+ .body(String.format("[[\"setMessages\", {\"update\": {" +
+ " \"%s\" : { \"mailboxIds\": [" + mailboxId.serialize() + "]}, " +
+ " \"%s\" : { \"mailboxIds\": [" + mailboxId.serialize() + "]}, " +
+ " \"%s\" : { \"mailboxIds\": [" + mailboxId.serialize() + "]}, " +
+ " \"%s\" : { \"mailboxIds\": [" + mailboxId.serialize() + "]}, " +
+ " \"%s\" : { \"mailboxIds\": [" + mailboxId.serialize() + "]}, " +
+ " \"%s\" : { \"mailboxIds\": [" + mailboxId.serialize() + "]}, " +
+ " \"%s\" : { \"mailboxIds\": [" + mailboxId.serialize() + "]}, " +
+ " \"%s\" : { \"mailboxIds\": [" + mailboxId.serialize() + "]}, " +
+ " \"%s\" : { \"mailboxIds\": [" + mailboxId.serialize() + "]}, " +
+ " \"%s\" : { \"mailboxIds\": [" + mailboxId.serialize() + "]}, " +
+ " \"%s\" : { \"mailboxIds\": [" + mailboxId.serialize() + "]}, " +
+ " \"%s\" : { \"mailboxIds\": [" + mailboxId.serialize() + "]} " +
+ "} }, \"#0\"]]", serializedMessageId1, serializedMessageId2, serializedMessageId3,
+ serializedMessageId4, serializedMessageId5, serializedMessageId6,
+ serializedMessageId7, serializedMessageId8, serializedMessageId9,
+ serializedMessageId10, serializedMessageId11, serializedMessageId12))
+ .when()
+ .post("/jmap")
+ // Then
+ .then()
+ .log().ifValidationFails().body(ARGUMENTS + ".updated", hasSize(12));
+ }
+
@Category(BasicFeature.class)
@Test
public void sendingAMailShouldLeadToAppropriateMailboxCountersOnSent() {
diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/SetMessagesUpdateProcessor.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/SetMessagesUpdateProcessor.java
index 84d31e5..9f4c176 100644
--- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/SetMessagesUpdateProcessor.java
+++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/SetMessagesUpdateProcessor.java
@@ -153,6 +153,8 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor {
if (isAMassiveFlagUpdate(patches, messages)) {
applyRangedFlagUpdate(patches, messages, responseBuilder, mailboxSession);
+ } else if (isAMassiveMove(patches, messages)) {
+ applyMove(patches, messages, responseBuilder, mailboxSession);
} else {
patches.forEach((id, patch) -> {
if (patch.isValid()) {
@@ -172,6 +174,14 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor {
&& messages.size() > 3;
}
+ private boolean isAMassiveMove(Map<MessageId, UpdateMessagePatch> patches, Multimap<MessageId, ComposedMessageIdWithMetaData> messages) {
+ // The same patch, that only represents a flag update, is applied to messages within a single mailbox
+ return StreamUtils.isSingleValued(patches.values().stream())
+ && StreamUtils.isSingleValued(messages.values().stream().map(metaData -> metaData.getComposedMessageId().getMailboxId()))
+ && patches.values().iterator().next().isOnlyAMove()
+ && messages.size() > 3;
+ }
+
private void applyRangedFlagUpdate(Map<MessageId, UpdateMessagePatch> patches, Multimap<MessageId, ComposedMessageIdWithMetaData> messages, SetMessagesResponse.Builder responseBuilder, MailboxSession mailboxSession) {
MailboxId mailboxId = messages.values()
.iterator()
@@ -198,6 +208,56 @@ public class SetMessagesUpdateProcessor implements SetMessagesProcessor {
} catch (MailboxException e) {
messageIds
.forEach(messageId -> handleMessageUpdateException(messageId, responseBuilder, e));
+ } catch (IllegalArgumentException e) {
+ ValidationResult invalidPropertyKeywords = ValidationResult.builder()
+ .property(MessageProperties.MessageProperty.keywords.asFieldName())
+ .message(e.getMessage())
+ .build();
+
+ messageIds
+ .forEach(messageId -> handleInvalidRequest(responseBuilder, messageId, ImmutableList.of(invalidPropertyKeywords), patch));
+ }
+ });
+ } else {
+ messages.keySet()
+ .forEach(messageId -> handleInvalidRequest(responseBuilder, messageId, patch.getValidationErrors(), patch));
+ }
+ }
+
+ private void applyMove(Map<MessageId, UpdateMessagePatch> patches, Multimap<MessageId, ComposedMessageIdWithMetaData> messages, SetMessagesResponse.Builder responseBuilder, MailboxSession mailboxSession) {
+ MailboxId mailboxId = messages.values()
+ .iterator()
+ .next()
+ .getComposedMessageId()
+ .getMailboxId();
+ UpdateMessagePatch patch = patches.values().iterator().next();
+ List<MessageRange> uidRanges = MessageRange.toRanges(messages.values().stream().map(metaData -> metaData.getComposedMessageId().getUid())
+ .distinct()
+ .collect(Guavate.toImmutableList()));
+
+ if (patch.isValid()) {
+ uidRanges.forEach(range -> {
+ ImmutableList<MessageId> messageIds = messages.entries()
+ .stream()
+ .filter(entry -> range.includes(entry.getValue().getComposedMessageId().getUid()))
+ .map(Map.Entry::getKey)
+ .distinct()
+ .collect(Guavate.toImmutableList());
+ try {
+ MailboxId targetId = mailboxIdFactory.fromString(patch.getMailboxIds().get().iterator().next());
+ mailboxManager.moveMessages(range, mailboxId, targetId, mailboxSession);
+ responseBuilder.updated(messageIds);
+ } catch (MailboxException e) {
+ messageIds
+ .forEach(messageId -> handleMessageUpdateException(messageId, responseBuilder, e));
+ } catch (IllegalArgumentException e) {
+ ValidationResult invalidPropertyKeywords = ValidationResult.builder()
+ .property(MessageProperties.MessageProperty.keywords.asFieldName())
+ .message(e.getMessage())
+ .build();
+
+ messageIds
+ .forEach(messageId -> handleInvalidRequest(responseBuilder, messageId, ImmutableList.of(invalidPropertyKeywords), patch));
}
});
} else {
diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/UpdateMessagePatch.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/UpdateMessagePatch.java
index 4aa3663..519a55a 100644
--- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/UpdateMessagePatch.java
+++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/UpdateMessagePatch.java
@@ -144,6 +144,12 @@ public class UpdateMessagePatch {
return !mailboxIds.isPresent() && (oldKeywords.isPresent() || keywords.isPresent());
}
+ public boolean isOnlyAMove() {
+ return mailboxIds.map(list -> list.size() == 1).orElse(false)
+ && oldKeywords.isEmpty()
+ && keywords.isEmpty();
+ }
+
public ImmutableList<ValidationResult> getValidationErrors() {
return validationErrors;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org