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 a8b84c4862ef44ac0a94d0646233ef718e6465e2 Author: Benoit Tellier <[email protected]> AuthorDate: Mon Mar 30 11:57:17 2020 +0700 JAMES-2632 Avoid reading MailboxPathV1Table if not needed If james starts with an up-to-date Cassandra schema, it can skip reads into MailboxPathV1 According to glowroot, these unneeded operations accounts for 5% of GetMailboxes time. --- .../CassandraMailboxSessionMapperFactory.java | 7 ++-- .../cassandra/mail/CassandraMailboxMapper.java | 36 +++++++++++++++---- .../CassandraSubscriptionManagerTest.java | 3 ++ .../mail/CassandraMailboxMapperGenericTest.java | 40 +++++++++++++++++----- .../cassandra/mail/CassandraMailboxMapperTest.java | 4 ++- .../mail/migration/MailboxPathV2MigrationTest.java | 4 ++- 6 files changed, 75 insertions(+), 19 deletions(-) diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/CassandraMailboxSessionMapperFactory.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/CassandraMailboxSessionMapperFactory.java index 86824de..6086ba4 100644 --- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/CassandraMailboxSessionMapperFactory.java +++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/CassandraMailboxSessionMapperFactory.java @@ -23,6 +23,7 @@ import javax.inject.Inject; import org.apache.james.backends.cassandra.init.configuration.CassandraConfiguration; import org.apache.james.backends.cassandra.utils.CassandraUtils; +import org.apache.james.backends.cassandra.versions.CassandraSchemaVersionDAO; import org.apache.james.blob.api.BlobStore; import org.apache.james.mailbox.MailboxSession; import org.apache.james.mailbox.cassandra.mail.CassandraACLMapper; @@ -92,6 +93,7 @@ public class CassandraMailboxSessionMapperFactory extends MailboxSessionMapperFa private final CassandraAttachmentOwnerDAO ownerDAO; private final CassandraACLMapper aclMapper; private final CassandraUserMailboxRightsDAO userMailboxRightsDAO; + private final CassandraSchemaVersionDAO versionDAO; private final CassandraUtils cassandraUtils; private final CassandraConfiguration cassandraConfiguration; @@ -105,7 +107,7 @@ public class CassandraMailboxSessionMapperFactory extends MailboxSessionMapperFa BlobStore blobStore, CassandraAttachmentMessageIdDAO attachmentMessageIdDAO, CassandraAttachmentOwnerDAO ownerDAO, CassandraACLMapper aclMapper, CassandraUserMailboxRightsDAO userMailboxRightsDAO, - CassandraUtils cassandraUtils, CassandraConfiguration cassandraConfiguration) { + CassandraSchemaVersionDAO versionDAO, CassandraUtils cassandraUtils, CassandraConfiguration cassandraConfiguration) { this.uidProvider = uidProvider; this.modSeqProvider = modSeqProvider; this.session = session; @@ -126,6 +128,7 @@ public class CassandraMailboxSessionMapperFactory extends MailboxSessionMapperFa this.attachmentMessageIdDAO = attachmentMessageIdDAO; this.aclMapper = aclMapper; this.userMailboxRightsDAO = userMailboxRightsDAO; + this.versionDAO = versionDAO; this.cassandraUtils = cassandraUtils; this.ownerDAO = ownerDAO; this.cassandraConfiguration = cassandraConfiguration; @@ -165,7 +168,7 @@ public class CassandraMailboxSessionMapperFactory extends MailboxSessionMapperFa @Override public MailboxMapper createMailboxMapper(MailboxSession mailboxSession) { - return new CassandraMailboxMapper(mailboxDAO, mailboxPathDAO, mailboxPathV2DAO, userMailboxRightsDAO, aclMapper); + return new CassandraMailboxMapper(mailboxDAO, mailboxPathDAO, mailboxPathV2DAO, userMailboxRightsDAO, aclMapper, versionDAO); } @Override 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 7a3f7ea..8e2d0b5 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 @@ -25,6 +25,9 @@ import java.util.List; import javax.inject.Inject; import org.apache.commons.lang3.tuple.Pair; +import org.apache.james.backends.cassandra.versions.CassandraSchemaVersionDAO; +import org.apache.james.backends.cassandra.versions.CassandraSchemaVersionManager; +import org.apache.james.backends.cassandra.versions.SchemaVersion; import org.apache.james.core.Username; import org.apache.james.mailbox.acl.ACLDiff; import org.apache.james.mailbox.cassandra.ids.CassandraId; @@ -54,33 +57,47 @@ public class CassandraMailboxMapper implements MailboxMapper { private static final int MAX_RETRY = 5; private static final Duration MIN_RETRY_BACKOFF = Duration.ofMillis(10); private static final Duration MAX_RETRY_BACKOFF = Duration.ofMillis(1000); + private static final SchemaVersion MAILBOX_PATH_V_2_MIGRATION_PERFORMED_VERSION = new SchemaVersion(6); private final CassandraMailboxDAO mailboxDAO; private final CassandraMailboxPathDAOImpl mailboxPathDAO; private final CassandraMailboxPathV2DAO mailboxPathV2DAO; private final CassandraACLMapper cassandraACLMapper; private final CassandraUserMailboxRightsDAO userMailboxRightsDAO; + private final boolean needMailboxPathV1Support; @Inject - public CassandraMailboxMapper(CassandraMailboxDAO mailboxDAO, CassandraMailboxPathDAOImpl mailboxPathDAO, CassandraMailboxPathV2DAO mailboxPathV2DAO, CassandraUserMailboxRightsDAO userMailboxRightsDAO, CassandraACLMapper aclMapper) { + public CassandraMailboxMapper(CassandraMailboxDAO mailboxDAO, CassandraMailboxPathDAOImpl mailboxPathDAO, CassandraMailboxPathV2DAO mailboxPathV2DAO, CassandraUserMailboxRightsDAO userMailboxRightsDAO, CassandraACLMapper aclMapper, CassandraSchemaVersionDAO versionDAO) { this.mailboxDAO = mailboxDAO; this.mailboxPathDAO = mailboxPathDAO; this.mailboxPathV2DAO = mailboxPathV2DAO; this.userMailboxRightsDAO = userMailboxRightsDAO; this.cassandraACLMapper = aclMapper; + + this.needMailboxPathV1Support = versionDAO.getCurrentSchemaVersion() + .block() + .orElse(CassandraSchemaVersionManager.MIN_VERSION) + .isBefore(MAILBOX_PATH_V_2_MIGRATION_PERFORMED_VERSION); } @Override public void delete(Mailbox mailbox) { CassandraId mailboxId = (CassandraId) mailbox.getMailboxId(); - Flux.merge( - mailboxPathDAO.delete(mailbox.generateAssociatedPath()), - mailboxPathV2DAO.delete(mailbox.generateAssociatedPath())) + deletePath(mailbox) .thenEmpty(mailboxDAO.delete(mailboxId) .retryBackoff(MAX_RETRY, MIN_RETRY_BACKOFF, MAX_RETRY_BACKOFF)) .block(); } + private Flux<Void> deletePath(Mailbox mailbox) { + if (needMailboxPathV1Support) { + return Flux.merge( + mailboxPathDAO.delete(mailbox.generateAssociatedPath()), + mailboxPathV2DAO.delete(mailbox.generateAssociatedPath())); + } + return Flux.from(mailboxPathV2DAO.delete(mailbox.generateAssociatedPath())); + } + @Override public Mono<Mailbox> findMailboxByPath(MailboxPath path) { return mailboxPathV2DAO.retrieveId(path) @@ -137,8 +154,7 @@ public class CassandraMailboxMapper implements MailboxMapper { String fixedNamespace = query.getFixedNamespace(); Username fixedUser = query.getFixedUser(); - return Flux.concat(mailboxPathV2DAO.listUserMailboxes(fixedNamespace, fixedUser), - mailboxPathDAO.listUserMailboxes(fixedNamespace, fixedUser)) + return listPaths(fixedNamespace, fixedUser) .filter(idAndPath -> query.isPathMatch(idAndPath.getMailboxPath())) .distinct(CassandraIdAndPath::getMailboxPath) .concatMap(this::retrieveMailbox) @@ -146,6 +162,14 @@ public class CassandraMailboxMapper implements MailboxMapper { .block(); } + private Flux<CassandraIdAndPath> listPaths(String fixedNamespace, Username fixedUser) { + if (needMailboxPathV1Support) { + return Flux.concat(mailboxPathV2DAO.listUserMailboxes(fixedNamespace, fixedUser), + mailboxPathDAO.listUserMailboxes(fixedNamespace, fixedUser)); + } + return mailboxPathV2DAO.listUserMailboxes(fixedNamespace, fixedUser); + } + private Mono<Mailbox> retrieveMailbox(CassandraIdAndPath idAndPath) { return retrieveMailbox(idAndPath.getCassandraId()) .switchIfEmpty(ReactorUtils.executeAndEmpty( diff --git a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/CassandraSubscriptionManagerTest.java b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/CassandraSubscriptionManagerTest.java index 33f1022..44e1063 100644 --- a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/CassandraSubscriptionManagerTest.java +++ b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/CassandraSubscriptionManagerTest.java @@ -22,6 +22,7 @@ package org.apache.james.mailbox.cassandra; import org.apache.james.backends.cassandra.CassandraClusterExtension; import org.apache.james.backends.cassandra.init.configuration.CassandraConfiguration; import org.apache.james.backends.cassandra.utils.CassandraUtils; +import org.apache.james.backends.cassandra.versions.CassandraSchemaVersionDAO; import org.apache.james.blob.api.BlobStore; import org.apache.james.mailbox.SubscriptionManager; import org.apache.james.mailbox.SubscriptionManagerContract; @@ -86,6 +87,7 @@ class CassandraSubscriptionManagerTest implements SubscriptionManagerContract { BlobStore blobStore = null; CassandraUidProvider uidProvider = null; CassandraModSeqProvider modSeqProvider = null; + CassandraSchemaVersionDAO versionDAO = null; subscriptionManager = new StoreSubscriptionManager( new CassandraMailboxSessionMapperFactory( @@ -110,6 +112,7 @@ class CassandraSubscriptionManagerTest implements SubscriptionManagerContract { ownerDAO, aclMapper, userMailboxRightsDAO, + versionDAO, CassandraUtils.WITH_DEFAULT_CONFIGURATION, CassandraConfiguration.DEFAULT_CONFIGURATION)); } diff --git a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperGenericTest.java b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperGenericTest.java index af2d274..87c6fc3 100644 --- a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperGenericTest.java +++ b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperGenericTest.java @@ -21,7 +21,9 @@ package org.apache.james.mailbox.cassandra.mail; import org.apache.james.backends.cassandra.CassandraClusterExtension; import org.apache.james.backends.cassandra.components.CassandraModule; +import org.apache.james.backends.cassandra.versions.CassandraSchemaVersionDAO; import org.apache.james.backends.cassandra.versions.CassandraSchemaVersionModule; +import org.apache.james.backends.cassandra.versions.SchemaVersion; import org.apache.james.mailbox.cassandra.ids.CassandraId; import org.apache.james.mailbox.cassandra.mail.utils.GuiceUtils; import org.apache.james.mailbox.cassandra.modules.CassandraAclModule; @@ -31,10 +33,10 @@ import org.apache.james.mailbox.cassandra.modules.CassandraUidModule; import org.apache.james.mailbox.model.MailboxId; import org.apache.james.mailbox.store.mail.MailboxMapper; import org.apache.james.mailbox.store.mail.model.MailboxMapperTest; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.extension.RegisterExtension; -class CassandraMailboxMapperGenericTest extends MailboxMapperTest { - +class CassandraMailboxMapperGenericTest { private static final CassandraModule MODULES = CassandraModule.aggregateModules( CassandraSchemaVersionModule.MODULE, CassandraAclModule.MODULE, @@ -45,14 +47,34 @@ class CassandraMailboxMapperGenericTest extends MailboxMapperTest { @RegisterExtension static CassandraClusterExtension cassandraCluster = new CassandraClusterExtension(MODULES); - @Override - protected MailboxMapper createMailboxMapper() { - return GuiceUtils.testInjector(cassandraCluster.getCassandraCluster()) - .getInstance(CassandraMailboxMapper.class); + @Nested + class V5 extends MailboxMapperTest { + @Override + protected MailboxMapper createMailboxMapper() { + return GuiceUtils.testInjector(cassandraCluster.getCassandraCluster()) + .getInstance(CassandraMailboxMapper.class); + } + + @Override + protected MailboxId generateId() { + return CassandraId.timeBased(); + } } - @Override - protected MailboxId generateId() { - return CassandraId.timeBased(); + @Nested + class V7 extends MailboxMapperTest { + @Override + protected MailboxMapper createMailboxMapper() { + new CassandraSchemaVersionDAO(cassandraCluster.getCassandraCluster().getConf()) + .updateVersion(new SchemaVersion(7)) + .block(); + return GuiceUtils.testInjector(cassandraCluster.getCassandraCluster()) + .getInstance(CassandraMailboxMapper.class); + } + + @Override + protected MailboxId generateId() { + return CassandraId.timeBased(); + } } } diff --git a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperTest.java b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperTest.java index 1675f44..9a9fcf2 100644 --- a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperTest.java +++ b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxMapperTest.java @@ -35,6 +35,7 @@ import org.apache.james.backends.cassandra.Scenario; import org.apache.james.backends.cassandra.components.CassandraModule; import org.apache.james.backends.cassandra.init.configuration.CassandraConfiguration; import org.apache.james.backends.cassandra.utils.CassandraUtils; +import org.apache.james.backends.cassandra.versions.CassandraSchemaVersionDAO; import org.apache.james.backends.cassandra.versions.CassandraSchemaVersionModule; import org.apache.james.core.Username; import org.apache.james.mailbox.cassandra.ids.CassandraId; @@ -104,7 +105,8 @@ class CassandraMailboxMapperTest { mailboxPathDAO, mailboxPathV2DAO, userMailboxRightsDAO, - aclMapper); + aclMapper, + new CassandraSchemaVersionDAO(cassandra.getConf())); } @Nested diff --git a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/migration/MailboxPathV2MigrationTest.java b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/migration/MailboxPathV2MigrationTest.java index 81bab1c..d65cb06 100644 --- a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/migration/MailboxPathV2MigrationTest.java +++ b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/migration/MailboxPathV2MigrationTest.java @@ -26,6 +26,7 @@ import org.apache.james.backends.cassandra.CassandraClusterExtension; import org.apache.james.backends.cassandra.components.CassandraModule; import org.apache.james.backends.cassandra.init.configuration.CassandraConfiguration; import org.apache.james.backends.cassandra.utils.CassandraUtils; +import org.apache.james.backends.cassandra.versions.CassandraSchemaVersionDAO; import org.apache.james.backends.cassandra.versions.CassandraSchemaVersionModule; import org.apache.james.core.Username; import org.apache.james.mailbox.cassandra.ids.CassandraId; @@ -83,7 +84,8 @@ class MailboxPathV2MigrationTest { daoV1, daoV2, userMailboxRightsDAO, - new CassandraACLMapper(cassandra.getConf(), userMailboxRightsDAO, CassandraConfiguration.DEFAULT_CONFIGURATION)); + new CassandraACLMapper(cassandra.getConf(), userMailboxRightsDAO, CassandraConfiguration.DEFAULT_CONFIGURATION), + new CassandraSchemaVersionDAO(cassandra.getConf())); } @Test --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
