This is an automated email from the ASF dual-hosted git repository. chibenwa pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 9996d7316340992dcc94956af3e35995e08c6474 Author: Benoit TELLIER <[email protected]> AuthorDate: Wed Jun 17 15:02:17 2026 +0200 [ENHANCEMENT] have SolveMailboxInconsistencies address all possible failures --- .../operate/webadmin/admin-mailboxes-extend.adoc | 40 +++++++++++++++++++--- .../task/SolveMailboxInconsistenciesService.java | 32 +++++++++++++---- .../SolveMailboxInconsistenciesServiceTest.java | 21 ++++++++---- src/site/markdown/server/manage-webadmin.md | 32 ++++++++++++++--- 4 files changed, 102 insertions(+), 23 deletions(-) diff --git a/docs/modules/servers/pages/distributed/operate/webadmin/admin-mailboxes-extend.adoc b/docs/modules/servers/pages/distributed/operate/webadmin/admin-mailboxes-extend.adoc index ecdb8b2006..a1dabd7290 100644 --- a/docs/modules/servers/pages/distributed/operate/webadmin/admin-mailboxes-extend.adoc +++ b/docs/modules/servers/pages/distributed/operate/webadmin/admin-mailboxes-extend.adoc @@ -13,6 +13,32 @@ a task]. The `I-KNOW-WHAT-I-M-DOING` header is mandatory (you can read more information about it in the warning section below). +Optional query parameters: + +* `maxIterations` strictly positive integer, defaults to `1`. +Reconciliation is run up to a fixpoint: fixing an inconsistency in one +pass (dropping a stale path, merging a ghost mailbox...) can surface a +new inconsistency that only a subsequent pass detects. The task re-runs +as long as a pass keeps applying fixes, bounded by `maxIterations` to +guard against oscillation. A value of `1` keeps the historical +single-pass behaviour. +* `autoMerge` boolean, defaults to `false`. When `true`, conflicting +entries where two *different* mailboxes resolve to the same path (the +historical "ghost mailbox") are resolved automatically: the mailbox +registered in the path table (the source of truth) is kept, and the +squatting one is merged into it (its messages and rights are moved over, +then its projection is dropped), reusing the +link:#_correcting_ghost_mailbox[ghost mailbox] merging machinery. This +is *destructive* (mail data is moved), only attempted once a strong read +confirms the loser is a genuine ghost not registered anywhere else, and +left `false` by default so that an admin explicitly opts in. Combine it +with `maxIterations > 1` so that any residual left by a merge converges +within the same task run. + +.... +curl -XPOST '/mailboxes?task=SolveInconsistencies&maxIterations=5&autoMerge=true' +.... + The scheduled task will have the following type `solve-mailbox-inconsistencies` and the following `additionalInformation`: @@ -33,14 +59,18 @@ The scheduled task will have the following type "mailboxPath":"#private:user:mailboxName2", "mailboxId":"464765a0-e4e7-11e4-aba4-710c1de3782b" } - }] + }], + "runningOptions":{ + "maxIterations": 5, + "autoMerge": true + } } .... -Note that conflicting entry inconsistencies will not be fixed and will -require to explicitly use link:#_correcting_ghost_mailbox[ghost mailbox] -endpoint in order to merge the conflicting mailboxes and prevent any -message loss. +Note that, unless `autoMerge` is enabled, conflicting entry +inconsistencies will not be fixed and will require to explicitly use +link:#_correcting_ghost_mailbox[ghost mailbox] endpoint in order to +merge the conflicting mailboxes and prevent any message loss. *WARNING*: this task can cancel concurrently running legitimate user operations upon dirty read. As such this task should be run offline. 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 e345faa0d7..9061e2d818 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 @@ -251,12 +251,11 @@ public class SolveMailboxInconsistenciesService { .flatMap(state -> { boolean cleanGhost = state.getT1() && state.getT2() && state.getT3(); if (!cleanGhost) { - // State no longer matches the clean-ghost picture (already reconciled, or the - // loser owns another path): do not merge. - if (state.getT1() && state.getT2()) { - // Still conflicting but the loser is registered elsewhere: leave it to an admin. - return reportConflict(context); - } + // State no longer matches the clean-ghost picture: either already reconciled, + // or the loser owns another path. In the latter case the loser is a genuine + // mailbox whose stale projection gets realigned onto its registered path by the + // same-mailbox conflict resolution, making this ghost disappear without a merge. + // We therefore leave it to that resolution rather than destroying or reporting it. return Mono.just(Result.COMPLETED); } return mergingRunner.runReactive(loserId, winnerId, new MailboxMergingTask.Context(0)) @@ -316,7 +315,26 @@ public class SolveMailboxInconsistenciesService { })) .thenReturn(Result.COMPLETED); })) - .switchIfEmpty(Mono.defer(() -> reportConflict(context))); + // The projection points to a path it does not own (unregistered, or held by another + // mailbox), while the conflicting path *is* registered to this mailbox: the projection + // is simply stale. Realign it onto the path the source of truth vouches for. + .switchIfEmpty(Mono.defer(() -> realignStaleProjection(context, mailboxDAO, currentProjection, conflictingEntry))); + } + + private Mono<Result> realignStaleProjection(Context context, CassandraMailboxDAO mailboxDAO, + Mailbox currentProjection, Mailbox conflictingEntry) { + MailboxPath stalePath = currentProjection.generateAssociatedPath(); + Mailbox realigned = new Mailbox(conflictingEntry.generateAssociatedPath(), + currentProjection.getUidValidity(), currentProjection.getMailboxId()); + return mailboxDAO.save(realigned) + .then(Mono.fromRunnable(() -> { + LOGGER.info("Inconsistency fixed for mailbox {}: realigned stale projection from path {} to registered path {}", + realigned.getMailboxId().serialize(), + stalePath.asString(), + realigned.generateAssociatedPath().asString()); + context.addFixedInconsistency(realigned.getMailboxId()); + })) + .thenReturn(Result.COMPLETED); } private Mono<Result> reportConflict(Context context) { 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 ea92010ccc..1512595411 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 @@ -458,9 +458,11 @@ class SolveMailboxInconsistenciesServiceTest { } @Test - void fixMailboxInconsistenciesShouldNotAutoMergeWhenLoserIsRegisteredElsewhere() { + void fixMailboxInconsistenciesShouldRealignLoserInsteadOfMergingWhenLoserIsRegisteredElsewhere() { // CASSANDRA_ID_1 squats path "abc" (owned by CASSANDRA_ID_2) but also legitimately owns path - // "xyz": it is not a clean ghost, so the path table vouches for it and it must not be merged away. + // "xyz": it is not a clean ghost. Rather than being merged away, its stale projection is + // realigned onto its registered path "xyz", which makes the ghost on "abc" disappear without + // any merge. The whole picture becomes consistent within a single run. Mailbox pathOwnerProjection = new Mailbox(MAILBOX_PATH, UID_VALIDITY_2, CASSANDRA_ID_2); Mailbox loserOtherPath = new Mailbox(NEW_MAILBOX_PATH, UID_VALIDITY_1, CASSANDRA_ID_1); mailboxDAO.save(MAILBOX).block(); @@ -468,11 +470,16 @@ class SolveMailboxInconsistenciesServiceTest { mailboxPathV3DAO.save(MAILBOX_2).block(); mailboxPathV3DAO.save(loserOtherPath).block(); - testee.fixMailboxInconsistencies(new Context(), new SolveMailboxInconsistenciesService.RunningOptions(1, true)).block(); + Context context = new Context(); + testee.fixMailboxInconsistencies(context, new SolveMailboxInconsistenciesService.RunningOptions(1, true)).block(); - verify(mergingRunner, never()).runReactive(any(), any(), any()); - // The ghost projection is preserved (not destroyed by an auto-merge). - assertThat(mailboxDAO.retrieveAllMailboxes().collectList().block()) - .contains(MAILBOX); + SoftAssertions.assertSoftly(softly -> { + verify(mergingRunner, never()).runReactive(any(), any(), any()); + softly.assertThat(mailboxDAO.retrieveAllMailboxes().collectList().block()) + .containsExactlyInAnyOrder(pathOwnerProjection, loserOtherPath); + softly.assertThat(mailboxPathV3DAO.listAll().collectList().block()) + .containsExactlyInAnyOrder(MAILBOX_2, loserOtherPath); + softly.assertThat(context.snapshot().getConflictingEntries()).isEmpty(); + }); } } \ No newline at end of file diff --git a/src/site/markdown/server/manage-webadmin.md b/src/site/markdown/server/manage-webadmin.md index e338ebc313..24773c9063 100644 --- a/src/site/markdown/server/manage-webadmin.md +++ b/src/site/markdown/server/manage-webadmin.md @@ -849,6 +849,26 @@ Will schedule a task for fixing inconsistencies for the mailbox deduplicated obj The `I-KNOW-WHAT-I-M-DOING` header is mandatory (you can read more information about it in the warning section below). +Optional query parameters: + + - `maxIterations` strictly positive integer, defaults to `1`. Reconciliation is run up to a fixpoint: + fixing an inconsistency in one pass (dropping a stale path, merging a ghost mailbox...) can surface a + new inconsistency that only a subsequent pass detects. The task re-runs as long as a pass keeps applying + fixes, bounded by `maxIterations` to guard against oscillation. A value of `1` keeps the historical + single-pass behaviour. + - `autoMerge` boolean, defaults to `false`. When `true`, conflicting entries where two **different** + mailboxes resolve to the same path (the historical "ghost mailbox") are resolved automatically: the + mailbox registered in the path table (the source of truth) is kept, and the squatting one is merged into + it (its messages and rights are moved over, then its projection is dropped), reusing the + [ghost mailbox](#correcting-ghost-mailbox) merging machinery. This is **destructive** (mail data is + moved), only attempted once a strong read confirms the loser is a genuine ghost not registered anywhere + else, and left `false` by default so that an admin explicitly opts in. Combine it with `maxIterations > 1` + so that any residual left by a merge converges within the same task run. + +``` +curl -XPOST '/mailboxes?task=SolveInconsistencies&maxIterations=5&autoMerge=true' +``` + The scheduled task will have the following type `solve-mailbox-inconsistencies` and the following `additionalInformation`: ``` @@ -867,13 +887,17 @@ The scheduled task will have the following type `solve-mailbox-inconsistencies` "mailboxPath":"#private:user:mailboxName2", "mailboxId":"464765a0-e4e7-11e4-aba4-710c1de3782b" } - }] + }], + "runningOptions":{ + "maxIterations": 5, + "autoMerge": true + } } ``` -Note that conflicting entry inconsistencies will not be fixed and will require to explicitly use -[ghost mailbox](#correcting-ghost-mailbox) endpoint in order to merge the conflicting mailboxes and prevent any message -loss. +Note that, unless `autoMerge` is enabled, conflicting entry inconsistencies will not be fixed and will +require to explicitly use [ghost mailbox](#correcting-ghost-mailbox) endpoint in order to merge the +conflicting mailboxes and prevent any message loss. **WARNING**: this task can cancel concurrently running legitimate user operations upon dirty read. As such this task should be run offline. --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
