You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by bt...@apache.org on 2020/04/10 06:41:42 UTC
[james-project] branch master updated (f6ec49a -> 4d0da67)
This is an automated email from the ASF dual-hosted git repository.
btellier pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git.
from f6ec49a JAMES-3092 Implement VersionParser to parse the version header against an injected set of supported versions
new 4bfe5b5 [Performance] Avoid reading all mailboxes upon GetMessageList
new 4d0da67 JAMES-2774 Avoid nested block in ReactorRabbitMQChannelPool
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../rabbitmq/ReactorRabbitMQChannelPool.java | 24 ++++++---------
.../cassandra/mail/CassandraMailboxMapper.java | 10 +++++++
.../james/mailbox/store/StoreMailboxManager.java | 11 +++++--
.../james/mailbox/store/mail/MailboxMapper.java | 15 ++++++++++
.../store/mail/model/MailboxMapperTest.java | 35 ++++++++++++++++++++++
5 files changed, 78 insertions(+), 17 deletions(-)
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
[james-project] 01/02: [Performance] Avoid reading all mailboxes
upon GetMessageList
Posted by bt...@apache.org.
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 4bfe5b54a7464ae2c649191c1f5dc1aa869c1299
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Mon Apr 6 08:18:11 2020 +0700
[Performance] Avoid reading all mailboxes upon GetMessageList
When the user specify a single mailbox inMailboxes filter, we still read
all mailboxes, resulting in many unwanted queries
---
.../cassandra/mail/CassandraMailboxMapper.java | 10 +++++++
.../james/mailbox/store/StoreMailboxManager.java | 11 +++++--
.../james/mailbox/store/mail/MailboxMapper.java | 15 ++++++++++
.../store/mail/model/MailboxMapperTest.java | 35 ++++++++++++++++++++++
4 files changed, 69 insertions(+), 2 deletions(-)
diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapper.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapper.java
index 75b068a..d74ce1c 100644
--- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapper.java
+++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapper.java
@@ -20,7 +20,9 @@
package org.apache.james.mailbox.cassandra.mail;
import java.time.Duration;
+import java.util.Collection;
import java.util.List;
+import java.util.stream.Stream;
import javax.inject.Inject;
@@ -141,6 +143,14 @@ public class CassandraMailboxMapper implements MailboxMapper {
.orElseThrow(() -> new MailboxNotFoundException(id));
}
+ @Override
+ public Stream<Mailbox> findMailboxesById(Collection<MailboxId> mailboxIds) {
+ return Flux.fromIterable(mailboxIds)
+ .map(CassandraId.class::cast)
+ .concatMap(this::retrieveMailbox)
+ .toStream();
+ }
+
private Mono<Mailbox> retrieveMailbox(CassandraId mailboxId) {
Mono<MailboxACL> acl = cassandraACLMapper.getACL(mailboxId);
Mono<Mailbox> simpleMailbox = mailboxDAO.retrieveMailbox(mailboxId);
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 c2da6e9..c3d7c02 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
@@ -687,10 +687,10 @@ public class StoreMailboxManager implements MailboxManager {
}
private Stream<MailboxId> getInMailboxes(ImmutableSet<MailboxId> inMailboxes, MailboxSession session) throws MailboxException {
- if (inMailboxes.isEmpty()) {
+ if (inMailboxes.isEmpty()) {
return getAllReadableMailbox(session);
} else {
- return getAllReadableMailbox(session).filter(inMailboxes::contains);
+ return filterReadable(inMailboxes, session);
}
}
@@ -700,6 +700,13 @@ public class StoreMailboxManager implements MailboxManager {
.map(Mailbox::getMailboxId);
}
+ private Stream<MailboxId> filterReadable(ImmutableSet<MailboxId> inMailboxes, MailboxSession session) throws MailboxException {
+ return mailboxSessionMapperFactory.getMailboxMapper(session)
+ .findMailboxesById(inMailboxes)
+ .filter(Throwing.<Mailbox>predicate(mailbox -> storeRightManager.hasRight(mailbox, Right.Read, session)).sneakyThrow())
+ .map(Mailbox::getMailboxId);
+ }
+
@Override
public Mono<Boolean> mailboxExists(MailboxPath mailboxPath, MailboxSession session) throws MailboxException {
MailboxMapper mapper = mailboxSessionMapperFactory.getMailboxMapper(session);
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/MailboxMapper.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/MailboxMapper.java
index f0df8d1..7ace127 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/MailboxMapper.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/MailboxMapper.java
@@ -18,7 +18,9 @@
****************************************************************/
package org.apache.james.mailbox.store.mail;
+import java.util.Collection;
import java.util.List;
+import java.util.stream.Stream;
import org.apache.james.core.Username;
import org.apache.james.mailbox.acl.ACLDiff;
@@ -33,6 +35,8 @@ import org.apache.james.mailbox.model.UidValidity;
import org.apache.james.mailbox.model.search.MailboxQuery;
import org.apache.james.mailbox.store.transaction.Mapper;
+import com.github.fge.lambdas.Throwing;
+
import reactor.core.publisher.Mono;
/**
@@ -83,6 +87,17 @@ public interface MailboxMapper extends Mapper {
Mailbox findMailboxById(MailboxId mailboxId)
throws MailboxException, MailboxNotFoundException;
+ default Stream<Mailbox> findMailboxesById(Collection<MailboxId> mailboxIds) throws MailboxException {
+ return mailboxIds.stream()
+ .flatMap(Throwing.<MailboxId, Stream<Mailbox>>function(id -> {
+ try {
+ return Stream.of(findMailboxById(id));
+ } catch (MailboxNotFoundException e) {
+ return Stream.empty();
+ }
+ }).sneakyThrow());
+ }
+
/**
* Return a List of {@link Mailbox} for the given userName and matching the right
*/
diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperTest.java
index 026e402..3fdf808 100644
--- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperTest.java
+++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperTest.java
@@ -25,6 +25,7 @@ import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import java.util.List;
+import java.util.stream.Stream;
import org.apache.james.core.Username;
import org.apache.james.mailbox.exception.MailboxException;
@@ -41,6 +42,8 @@ import org.apache.james.mailbox.store.mail.MailboxMapper;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
+import com.google.common.collect.ImmutableList;
+
/**
* Generic purpose tests for your implementation MailboxMapper.
*
@@ -250,6 +253,38 @@ public abstract class MailboxMapperTest {
Mailbox actual = mailboxMapper.findMailboxById(benwaInboxMailbox.getMailboxId());
assertThat(actual).isEqualTo(benwaInboxMailbox);
}
+
+ @Test
+ void findMailboxesByIdShouldReturnEmptyWhenNoIdSupplied() throws MailboxException {
+ createAll();
+
+ Stream<Mailbox> mailboxes = mailboxMapper.findMailboxesById(ImmutableList.of());
+
+ assertThat(mailboxes).isEmpty();
+ }
+
+ @Test
+ void findMailboxesByIdShouldReturnMailboxOfSuppliedId() throws MailboxException {
+ createAll();
+
+ Stream<Mailbox> mailboxes = mailboxMapper.findMailboxesById(ImmutableList.of(
+ benwaInboxMailbox.getMailboxId(),
+ benwaWorkMailbox.getMailboxId()));
+
+ assertThat(mailboxes).containsOnly(benwaWorkMailbox, benwaInboxMailbox);
+ }
+
+ @Test
+ void findMailboxesByIdShouldFilterOutNonExistingMailbox() throws MailboxException {
+ createAll();
+ mailboxMapper.delete(benwaWorkMailbox);
+
+ Stream<Mailbox> mailboxes = mailboxMapper.findMailboxesById(ImmutableList.of(
+ benwaInboxMailbox.getMailboxId(),
+ benwaWorkMailbox.getMailboxId()));
+
+ assertThat(mailboxes).containsOnly(benwaInboxMailbox);
+ }
@Test
void findMailboxByIdShouldFailWhenAbsent() throws MailboxException {
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
[james-project] 02/02: JAMES-2774 Avoid nested block in
ReactorRabbitMQChannelPool
Posted by bt...@apache.org.
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 4d0da670f68c45776477d90a2617074b9c13adad
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Wed Apr 8 22:00:52 2020 +0700
JAMES-2774 Avoid nested block in ReactorRabbitMQChannelPool
---
.../rabbitmq/ReactorRabbitMQChannelPool.java | 24 ++++++++--------------
1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/backends-common/rabbitmq/src/main/java/org/apache/james/backends/rabbitmq/ReactorRabbitMQChannelPool.java b/backends-common/rabbitmq/src/main/java/org/apache/james/backends/rabbitmq/ReactorRabbitMQChannelPool.java
index db3e900..0459abe 100644
--- a/backends-common/rabbitmq/src/main/java/org/apache/james/backends/rabbitmq/ReactorRabbitMQChannelPool.java
+++ b/backends-common/rabbitmq/src/main/java/org/apache/james/backends/rabbitmq/ReactorRabbitMQChannelPool.java
@@ -138,28 +138,22 @@ public class ReactorRabbitMQChannelPool implements ChannelPool, Startable {
return RabbitFlux.createReceiver(new ReceiverOptions().connectionMono(connectionMono));
}
- public Mono<Connection> getConnectionMono() {
- return connectionMono;
- }
-
@Override
public Mono<? extends Channel> getChannelMono() {
- return Mono.fromCallable(this::borrow);
+ return borrow();
}
- private Channel borrow() {
- Channel channel = tryBorrowFromPool();
- borrowedChannels.add(channel);
- return channel;
- }
-
- private Channel tryBorrowFromPool() {
- return Mono.fromCallable(this::borrowFromPool)
+ private Mono<Channel> borrow() {
+ return tryBorrowFromPool()
.doOnError(throwable -> LOGGER.warn("Cannot borrow channel", throwable))
.retryBackoff(MAX_BORROW_RETRIES, MIN_BORROW_DELAY, FOREVER, Schedulers.elastic())
.onErrorMap(this::propagateException)
.subscribeOn(Schedulers.elastic())
- .block();
+ .doOnNext(borrowedChannels::add);
+ }
+
+ private Mono<Channel> tryBorrowFromPool() {
+ return Mono.fromCallable(this::borrowFromPool);
}
private Throwable propagateException(Throwable throwable) {
@@ -223,7 +217,7 @@ public class ReactorRabbitMQChannelPool implements ChannelPool, Startable {
public boolean tryChannel() {
Channel channel = null;
try {
- channel = borrow();
+ channel = borrow().block();
return channel.isOpen();
} catch (Throwable t) {
return false;
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org