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 2021/10/12 01:54:46 UTC
[james-project] branch master updated: JAMES-3660 Cassandra mailbox
creation unstable when high concurency (#686)
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
The following commit(s) were added to refs/heads/master by this push:
new c16e4c6 JAMES-3660 Cassandra mailbox creation unstable when high concurency (#686)
c16e4c6 is described below
commit c16e4c69e5350861299989291aac412fe79e0840
Author: Benoit TELLIER <bt...@linagora.com>
AuthorDate: Tue Oct 12 08:54:42 2021 +0700
JAMES-3660 Cassandra mailbox creation unstable when high concurency (#686)
org.apache.james.mailbox.cassandra.CassandraMailboxManagerTest$WithBatchSize.creatingConcurrentlyMailboxesWithSameParentShouldNotFail
tests is enough to trigger instability on the Apache CI
https://ci-builds.apache.org/job/james/job/ApacheJames/job/PR-685/1/
In short, the LWT usage is enough to create contention.
Looking closer at the issue, StoreMailboxManager does numerous defensive SERIAL reads (doing empty paxos commits) which ends up further degrading performance and increase contention.
I believe removing these defensive reads would make our code more stable.
It resulted in faster (x2) test for gConcurrentlyMailboxesWithSameParentShouldNotFail
---
.../org/apache/james/mailbox/MailboxManagerTest.java | 18 +++++++++++++++++-
.../james/mailbox/store/StoreMailboxManager.java | 8 ++------
2 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/mailbox/api/src/test/java/org/apache/james/mailbox/MailboxManagerTest.java b/mailbox/api/src/test/java/org/apache/james/mailbox/MailboxManagerTest.java
index 81e8c5c..cd94570 100644
--- a/mailbox/api/src/test/java/org/apache/james/mailbox/MailboxManagerTest.java
+++ b/mailbox/api/src/test/java/org/apache/james/mailbox/MailboxManagerTest.java
@@ -95,6 +95,7 @@ import org.assertj.core.api.SoftAssertions;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.RepeatedTest;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
@@ -164,7 +165,7 @@ public abstract class MailboxManagerTest<T extends MailboxManager> {
mailboxManager.endProcessingRequest(session);
}
- @Test
+ @RepeatedTest(10)
protected void creatingConcurrentlyMailboxesWithSameParentShouldNotFail() throws Exception {
MailboxSession session = mailboxManager.createSystemSession(USER_1);
String mailboxName = "a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p.q.r.s.t.u.v.w.x.y.z";
@@ -188,6 +189,21 @@ public abstract class MailboxManagerTest<T extends MailboxManager> {
assertThat(mailboxId.get()).isEqualTo(retrievedMailbox.getId());
}
+ @Test
+ void createShouldSucceedWhenSubFolderExists() throws Exception {
+ session = mailboxManager.createSystemSession(USER_1);
+ mailboxManager.startProcessingRequest(session);
+
+ MailboxId parentId = mailboxManager.createMailbox(MailboxPath.forUser(USER_1, "name"), session).get();
+ MailboxPath mailboxPath = MailboxPath.forUser(USER_1, "name.subfolder");
+ Optional<MailboxId> mailboxId = mailboxManager.createMailbox(mailboxPath, session);
+ MessageManager retrievedMailbox = mailboxManager.getMailbox(mailboxPath, session);
+
+ assertThat(mailboxId.isPresent()).isTrue();
+ assertThat(mailboxId.get()).isEqualTo(retrievedMailbox.getId());
+ assertThat(mailboxManager.getMailbox(MailboxPath.forUser(USER_1, "name"), session).getId()).isEqualTo(parentId);
+ }
+
@Nested
class MailboxCreationTests {
@Test
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 5912b9e..db2f61b 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
@@ -89,7 +89,6 @@ import org.apache.james.mailbox.store.quota.QuotaComponents;
import org.apache.james.mailbox.store.search.MessageSearchIndex;
import org.apache.james.mailbox.store.user.SubscriptionMapper;
import org.apache.james.mailbox.store.user.model.Subscription;
-import org.apache.james.util.FunctionalUtils;
import org.reactivestreams.Publisher;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -397,9 +396,7 @@ public class StoreMailboxManager implements MailboxManager {
List<MailboxId> mailboxIds = new ArrayList<>();
MailboxMapper mapper = mailboxSessionMapperFactory.getMailboxMapper(mailboxSession);
locker.executeWithLock(mailboxPath, () ->
- block(mapper.pathExists(mailboxPath)
- .filter(FunctionalUtils.identityPredicate().negate())
- .flatMap(any -> mapper.executeReactive(mapper.create(mailboxPath, UidValidity.generate())
+ block(mapper.executeReactive(mapper.create(mailboxPath, UidValidity.generate())
.doOnNext(mailbox -> mailboxIds.add(mailbox.getMailboxId()))
.flatMap(mailbox ->
// notify listeners
@@ -416,8 +413,7 @@ public class StoreMailboxManager implements MailboxManager {
return Mono.error(e);
}
return Mono.empty();
- }))
- .then()), MailboxPathLocker.LockType.Write);
+ })), MailboxPathLocker.LockType.Write);
return mailboxIds;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org