This is an automated email from the ASF dual-hosted git repository.
quantranhong1999 pushed a commit to branch 3.9.x
in repository https://gitbox.apache.org/repos/asf/james-project.git
The following commit(s) were added to refs/heads/3.9.x by this push:
new b111a61a85 Leverage strong consistency within
SolveMailboxInconsistenciesService (3.9.x) (#3100)
b111a61a85 is described below
commit b111a61a85b5ae9e8c8ce1c01b913643d16945df
Author: Benoit TELLIER <[email protected]>
AuthorDate: Wed Jul 29 09:54:42 2026 +0200
Leverage strong consistency within SolveMailboxInconsistenciesService
(3.9.x) (#3100)
Improve SolveMailboxInconsistenciesService
- Confirm the incosistency prior fixing
- Systematic use of STRONG consistency when reading pathDao
- Re-introduce path entry
---
.../cassandra/mail/CassandraMailboxPathV3DAO.java | 12 +++++++++++-
.../task/SolveMailboxInconsistenciesService.java | 13 ++++++++-----
.../cassandra/mail/CassandraMailboxMapperTest.java | 13 +++++++++----
.../SolveMailboxInconsistenciesServiceTest.java | 21 +++++++++++++++------
.../rabbitmq/ConsistencyTasksIntegrationTest.java | 12 ++++++++++--
5 files changed, 53 insertions(+), 18 deletions(-)
diff --git
a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxPathV3DAO.java
b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxPathV3DAO.java
index 7663a18ec0..c17393c684 100644
---
a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxPathV3DAO.java
+++
b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMailboxPathV3DAO.java
@@ -175,7 +175,17 @@ public class CassandraMailboxPathV3DAO {
}
public Flux<Mailbox> listAll() {
- return cassandraAsyncExecutor.executeRows(selectAll.bind())
+ return listAll(JamesExecutionProfiles.ConsistencyChoice.WEAK);
+ }
+
+ public Flux<Mailbox> listAll(JamesExecutionProfiles.ConsistencyChoice
consistencyChoice) {
+ BoundStatementBuilder statementBuilder =
selectAll.boundStatementBuilder();
+
+ if (consistencyChoice.equals(STRONG)) {
+ statementBuilder.setExecutionProfile(lwtProfile);
+ }
+
+ return cassandraAsyncExecutor.executeRows(statementBuilder.build())
.map(this::fromRowToCassandraIdAndPath)
.map(FunctionalUtils.toFunction(this::logReadSuccess));
}
diff --git
a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesService.java
b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesService.java
index 9061e2d818..654f631469 100644
---
a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesService.java
+++
b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesService.java
@@ -148,9 +148,11 @@ public class SolveMailboxInconsistenciesService {
*
* This inconsistency arise if mailbox creation fails or upon partial
deletes.
*
- * In both case removing the dandling path registration solves the
inconsistency
- *
- * In order to solve this inconsistency, we can simply re-reference the
mailboxPath.
+ * The path table being the source of truth, we re-create the missing
MailboxDao projection
+ * from the registered path entry rather than dropping the path
registration: the mailbox may
+ * still hold messages (keyed by id) that would otherwise become
unreachable. We re-read the
+ * path entry with STRONG consistency first, so a registration that has
meanwhile been removed
+ * is left untouched.
*/
private static class OrphanMailboxPathDAOEntry implements Inconsistency {
private final Mailbox mailbox;
@@ -161,7 +163,8 @@ public class SolveMailboxInconsistenciesService {
@Override
public Mono<Result> fix(Context context, CassandraMailboxDAO
mailboxDAO, CassandraMailboxPathV3DAO pathV3DAO) {
- return pathV3DAO.delete(mailbox.generateAssociatedPath())
+ return pathV3DAO.retrieve(mailbox.generateAssociatedPath(), STRONG)
+ .flatMap(mailboxDAO::save)
.doOnSuccess(any -> {
LOGGER.info("Inconsistency fixed for orphan mailboxPath {}
- {}",
mailbox.getMailboxId().serialize(),
@@ -628,7 +631,7 @@ public class SolveMailboxInconsistenciesService {
}
private Flux<Result> processMailboxPathDaoInconsistencies(Context context,
boolean autoMerge) {
- return mailboxPathV3DAO.listAll()
+ return mailboxPathV3DAO.listAll(STRONG)
.flatMap(entry -> detectMailboxPathDaoInconsistency(entry,
autoMerge), DEFAULT_CONCURRENCY)
.doOnNext(any -> context.incrementProcessedMailboxPathEntries())
// Detect every inconsistency first, then fix them one at a time.
Resolving a same-mailbox
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 c27ffd68b0..c8ce2bf8a0 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
@@ -342,11 +342,16 @@ class CassandraMailboxMapperTest {
.onErrorResume(e -> Mono.empty())
.block());
+ // The path table is the source of truth: the missing mailbox
projection is re-created
+ // from the registered path entry, rather than dropping that
registration, so that
+ // messages held by this mailbox (keyed by id) do not become
unreachable.
SoftAssertions.assertSoftly(Throwing.consumer(softly -> {
- softly.assertThatThrownBy(() ->
MailboxReactorUtils.blockOptional(testee.findMailboxById(MAILBOX_ID)))
- .isInstanceOf(MailboxNotFoundException.class);
-
softly.assertThat(MailboxReactorUtils.blockOptional(testee.findMailboxByPath(MAILBOX_PATH)))
- .isEmpty();
+ softly(softly)
+ .assertThat(testee.findMailboxById(MAILBOX_ID).block())
+ .isEqualTo(MAILBOX);
+ softly(softly)
+
.assertThat(testee.findMailboxByPath(MAILBOX_PATH).block())
+ .isEqualTo(MAILBOX);
}));
}
}
diff --git
a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesServiceTest.java
b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesServiceTest.java
index 1512595411..a2cd31802e 100644
---
a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesServiceTest.java
+++
b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesServiceTest.java
@@ -314,11 +314,14 @@ class SolveMailboxInconsistenciesServiceTest {
testee.fixMailboxInconsistencies(new Context()).block();
+ // The path table is the source of truth: rather than dropping the
dangling path registration
+ // (and losing the reference to a mailbox that may still hold
messages), the missing projection
+ // is re-created from it, yielding a consistent registration on both
sides.
SoftAssertions.assertSoftly(softly -> {
softly.assertThat(mailboxDAO.retrieveAllMailboxes().collectList().block())
- .isEmpty();
+ .containsExactlyInAnyOrder(MAILBOX);
softly.assertThat(mailboxPathV3DAO.listAll().collectList().block())
- .isEmpty();
+ .containsExactlyInAnyOrder(MAILBOX);
});
}
@@ -386,11 +389,15 @@ class SolveMailboxInconsistenciesServiceTest {
testee.fixMailboxInconsistencies(new Context()).block();
+ // The path registration is the source of truth and is thus preserved:
the missing projection
+ // is re-created for the registered mailbox instead of dropping the
registration. The stale
+ // projection squatting that path is left untouched and reported as a
conflicting entry, as
+ // merging the two mailboxes requires the admin (or auto-merge) to
arbitrate.
SoftAssertions.assertSoftly(softly -> {
softly.assertThat(mailboxDAO.retrieveAllMailboxes().collectList().block())
- .containsExactlyInAnyOrder(MAILBOX);
+ .containsExactlyInAnyOrder(MAILBOX, MAILBOX_2);
softly.assertThat(mailboxPathV3DAO.listAll().collectList().block())
- .isEmpty();
+ .containsExactlyInAnyOrder(MAILBOX_2);
});
}
@@ -402,11 +409,13 @@ class SolveMailboxInconsistenciesServiceTest {
testee.fixMailboxInconsistencies(new Context()).block();
testee.fixMailboxInconsistencies(new Context()).block();
+ // Re-creating the missing projection reaches a stable state: further
runs only keep reporting
+ // the unresolved conflict, they do not alter the data any more.
SoftAssertions.assertSoftly(softly -> {
softly.assertThat(mailboxDAO.retrieveAllMailboxes().collectList().block())
- .containsExactlyInAnyOrder(MAILBOX);
+ .containsExactlyInAnyOrder(MAILBOX, MAILBOX_2);
softly.assertThat(mailboxPathV3DAO.listAll().collectList().block())
- .containsExactlyInAnyOrder(MAILBOX);
+ .containsExactlyInAnyOrder(MAILBOX_2);
});
}
diff --git
a/server/protocols/webadmin-integration-test/distributed-webadmin-integration-test/src/test/java/org/apache/james/webadmin/integration/rabbitmq/ConsistencyTasksIntegrationTest.java
b/server/protocols/webadmin-integration-test/distributed-webadmin-integration-test/src/test/java/org/apache/james/webadmin/integration/rabbitmq/ConsistencyTasksIntegrationTest.java
index c0eef95628..0580e7d025 100644
---
a/server/protocols/webadmin-integration-test/distributed-webadmin-integration-test/src/test/java/org/apache/james/webadmin/integration/rabbitmq/ConsistencyTasksIntegrationTest.java
+++
b/server/protocols/webadmin-integration-test/distributed-webadmin-integration-test/src/test/java/org/apache/james/webadmin/integration/rabbitmq/ConsistencyTasksIntegrationTest.java
@@ -33,6 +33,7 @@ import static
org.apache.james.jmap.JMAPTestingConstants.LOCALHOST_IP;
import static org.apache.james.webadmin.Constants.SEPARATOR;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.hamcrest.CoreMatchers.hasItems;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.is;
@@ -217,7 +218,7 @@ class ConsistencyTasksIntegrationTest {
}
@Test
- void shouldSolveMailboxesInconsistency(GuiceJamesServer server) {
+ void shouldSolveMailboxesInconsistency(GuiceJamesServer server) throws
Exception {
// schema version 6 or higher required to run solve mailbox
inconsistencies task
String upgradeTaskId = with().post(UPGRADE_TO_LATEST_VERSION)
.jsonPath()
@@ -257,7 +258,14 @@ class ConsistencyTasksIntegrationTest {
.basePath(TasksRoutes.BASE)
.get(solveConsistenciesTaskId + "/await");
- assertThatCode(() ->
testIMAPClient.create(TEST_MAILBOX)).doesNotThrowAnyException();
+ // The path registration is the source of truth: the task re-creates
the missing mailbox
+ // projection from it rather than dropping the registration, as the
mailbox may already hold
+ // messages (keyed by id) that would otherwise become unreachable. The
mailbox thus ends up
+ // fully usable - hence selectable, and no longer creatable.
+ assertThat(testIMAPClient.sendCommand("SELECT " + TEST_MAILBOX))
+ .contains("SELECT completed.");
+ assertThatThrownBy(() -> testIMAPClient.create(TEST_MAILBOX))
+ .hasMessageContaining("Mailbox already exists");
}
@Tag(Unstable.TAG)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
