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 743e635413e1f957873008893d9deb26db635ecd Author: Benoit TELLIER <[email protected]> AuthorDate: Wed Jun 17 10:44:24 2026 +0200 [FIX] SolveMailboxInconsistencies: several runs until convergence --- .../task/SolveMailboxInconsistenciesService.java | 56 ++++++++++++++++++++++ .../mail/task/SolveMailboxInconsistenciesTask.java | 22 +++++++-- ...nconsistenciesTaskAdditionalInformationDTO.java | 19 ++++++-- .../task/SolveMailboxInconsistenciesTaskDTO.java | 20 ++++++-- ...ailboxInconsistenciesTaskSerializationTest.java | 11 +++-- .../SolveMailboxInconsistenciesRequestToTask.java | 7 ++- ...lveMailboxInconsistenciesRequestToTaskTest.java | 2 +- 7 files changed, 121 insertions(+), 16 deletions(-) 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 0a76fab0d9..d7f71b2042 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 @@ -263,6 +263,36 @@ public class SolveMailboxInconsistenciesService { } } + public static class RunningOptions { + public static final int DEFAULT_MAX_ITERATIONS = 1; + public static final RunningOptions DEFAULT = new RunningOptions(DEFAULT_MAX_ITERATIONS); + + private final int maxIterations; + + public RunningOptions(int maxIterations) { + Preconditions.checkArgument(maxIterations >= 1, "'maxIterations' must be strictly positive"); + this.maxIterations = maxIterations; + } + + public int getMaxIterations() { + return maxIterations; + } + + @Override + public final boolean equals(Object o) { + if (o instanceof RunningOptions) { + RunningOptions that = (RunningOptions) o; + return this.maxIterations == that.maxIterations; + } + return false; + } + + @Override + public final int hashCode() { + return Objects.hash(maxIterations); + } + } + public static class Context { static class Builder { private Optional<Long> processedMailboxEntries; @@ -457,7 +487,33 @@ public class SolveMailboxInconsistenciesService { } public Mono<Result> fixMailboxInconsistencies(Context context) { + return fixMailboxInconsistencies(context, RunningOptions.DEFAULT); + } + + public Mono<Result> fixMailboxInconsistencies(Context context, RunningOptions runningOptions) { assertValidVersion(); + return fixUntilStable(context, runningOptions.getMaxIterations()); + } + + // Reconciliation is run 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. We re-run as long as a pass keeps applying fixes, bounded by maxIterations to guard + // against oscillation. As fixes only ever grow the fixedInconsistencies list, a pass that adds + // none means we reached a stable state. + private Mono<Result> fixUntilStable(Context context, int remainingIterations) { + int fixedBefore = context.snapshot().getFixedInconsistencies().size(); + return runOnePass(context) + .flatMap(result -> { + boolean appliedFixes = context.snapshot().getFixedInconsistencies().size() > fixedBefore; + if (appliedFixes && remainingIterations > 1) { + return fixUntilStable(context, remainingIterations - 1) + .map(nextResult -> Task.combine(result, nextResult)); + } + return Mono.just(result); + }); + } + + private Mono<Result> runOnePass(Context context) { return Flux.concat( processMailboxDaoInconsistencies(context), processMailboxPathDaoInconsistencies(context)) diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesTask.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesTask.java index 8c4d679384..6e34f7e250 100644 --- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesTask.java +++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesTask.java @@ -23,6 +23,7 @@ import java.time.Clock; import java.time.Instant; import java.util.Optional; +import org.apache.james.mailbox.cassandra.mail.task.SolveMailboxInconsistenciesService.RunningOptions; import org.apache.james.mailbox.model.MailboxId; import org.apache.james.task.Task; import org.apache.james.task.TaskExecutionDetails; @@ -44,15 +45,17 @@ public class SolveMailboxInconsistenciesTask implements Task { private final ImmutableList<String> fixedInconsistencies; private final ImmutableList<ConflictingEntry> conflictingEntries; private final long errors; + private final RunningOptions runningOptions; Details(Instant instant, long processedMailboxEntries, long processedMailboxPathEntries, ImmutableList<String> fixedInconsistencies, - ImmutableList<ConflictingEntry> conflictingEntries, long errors) { + ImmutableList<ConflictingEntry> conflictingEntries, long errors, RunningOptions runningOptions) { this.instant = instant; this.processedMailboxEntries = processedMailboxEntries; this.processedMailboxPathEntries = processedMailboxPathEntries; this.fixedInconsistencies = fixedInconsistencies; this.conflictingEntries = conflictingEntries; this.errors = errors; + this.runningOptions = runningOptions; } @Override @@ -79,18 +82,24 @@ public class SolveMailboxInconsistenciesTask implements Task { long getErrors() { return errors; } + + public RunningOptions getRunningOptions() { + return runningOptions; + } } private final SolveMailboxInconsistenciesService service; + private final RunningOptions runningOptions; - public SolveMailboxInconsistenciesTask(SolveMailboxInconsistenciesService service) { + public SolveMailboxInconsistenciesTask(SolveMailboxInconsistenciesService service, RunningOptions runningOptions) { this.service = service; + this.runningOptions = runningOptions; this.context = new SolveMailboxInconsistenciesService.Context(); } @Override public Result run() { - return service.fixMailboxInconsistencies(context) + return service.fixMailboxInconsistencies(context, runningOptions) .block(); } @@ -99,6 +108,10 @@ public class SolveMailboxInconsistenciesTask implements Task { return SOLVE_MAILBOX_INCONSISTENCIES; } + public RunningOptions getRunningOptions() { + return runningOptions; + } + @Override public Optional<TaskExecutionDetails.AdditionalInformation> details() { SolveMailboxInconsistenciesService.Context.Snapshot snapshot = context.snapshot(); @@ -109,6 +122,7 @@ public class SolveMailboxInconsistenciesTask implements Task { .map(MailboxId::serialize) .collect(ImmutableList.toImmutableList()), snapshot.getConflictingEntries(), - snapshot.getErrors())); + snapshot.getErrors(), + runningOptions)); } } diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesTaskAdditionalInformationDTO.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesTaskAdditionalInformationDTO.java index 5811a68fcf..e484e540d3 100644 --- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesTaskAdditionalInformationDTO.java +++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesTaskAdditionalInformationDTO.java @@ -20,8 +20,10 @@ package org.apache.james.mailbox.cassandra.mail.task; import java.time.Instant; +import java.util.Optional; import org.apache.james.json.DTOModule; +import org.apache.james.mailbox.cassandra.mail.task.SolveMailboxInconsistenciesService.RunningOptions; import org.apache.james.server.task.json.dto.AdditionalInformationDTO; import org.apache.james.server.task.json.dto.AdditionalInformationDTOModule; @@ -37,7 +39,8 @@ public class SolveMailboxInconsistenciesTaskAdditionalInformationDTO implements details.getFixedInconsistencies(), details.getConflictingEntries(), details.getErrors(), - details.timestamp()); + details.timestamp(), + Optional.of(SolveMailboxInconsistenciesRunningOptionsDTO.asDTO(details.getRunningOptions()))); } public static AdditionalInformationDTOModule<SolveMailboxInconsistenciesTask.Details, SolveMailboxInconsistenciesTaskAdditionalInformationDTO> module() { @@ -57,6 +60,7 @@ public class SolveMailboxInconsistenciesTaskAdditionalInformationDTO implements private final ImmutableList<ConflictingEntry> conflictingEntries; private final long errors; private final Instant timestamp; + private final Optional<SolveMailboxInconsistenciesRunningOptionsDTO> runningOptions; public SolveMailboxInconsistenciesTaskAdditionalInformationDTO(@JsonProperty("type") String type, @JsonProperty("processedMailboxEntries") long processedMailboxEntries, @@ -64,7 +68,8 @@ public class SolveMailboxInconsistenciesTaskAdditionalInformationDTO implements @JsonProperty("fixedInconsistencies") ImmutableList<String> fixedInconsistencies, @JsonProperty("conflictingEntries") ImmutableList<ConflictingEntry> conflictingEntries, @JsonProperty("errors") long errors, - @JsonProperty("timestamp") Instant timestamp) { + @JsonProperty("timestamp") Instant timestamp, + @JsonProperty("runningOptions") Optional<SolveMailboxInconsistenciesRunningOptionsDTO> runningOptions) { this.type = type; this.processedMailboxEntries = processedMailboxEntries; this.timestamp = timestamp; @@ -72,6 +77,7 @@ public class SolveMailboxInconsistenciesTaskAdditionalInformationDTO implements this.fixedInconsistencies = fixedInconsistencies; this.conflictingEntries = conflictingEntries; this.errors = errors; + this.runningOptions = runningOptions; } public long getProcessedMailboxEntries() { @@ -94,6 +100,10 @@ public class SolveMailboxInconsistenciesTaskAdditionalInformationDTO implements return errors; } + public Optional<SolveMailboxInconsistenciesRunningOptionsDTO> getRunningOptions() { + return runningOptions; + } + @Override public Instant getTimestamp() { return timestamp; @@ -110,6 +120,9 @@ public class SolveMailboxInconsistenciesTaskAdditionalInformationDTO implements processedMailboxPathEntries, fixedInconsistencies, conflictingEntries, - errors); + errors, + runningOptions + .map(SolveMailboxInconsistenciesRunningOptionsDTO::asDomainObject) + .orElse(RunningOptions.DEFAULT)); } } diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesTaskDTO.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesTaskDTO.java index aa22280989..265d1a3030 100644 --- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesTaskDTO.java +++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesTaskDTO.java @@ -18,7 +18,10 @@ ****************************************************************/ package org.apache.james.mailbox.cassandra.mail.task; +import java.util.Optional; + import org.apache.james.json.DTOModule; +import org.apache.james.mailbox.cassandra.mail.task.SolveMailboxInconsistenciesService.RunningOptions; import org.apache.james.server.task.json.dto.TaskDTO; import org.apache.james.server.task.json.dto.TaskDTOModule; @@ -26,7 +29,8 @@ import com.fasterxml.jackson.annotation.JsonProperty; public class SolveMailboxInconsistenciesTaskDTO implements TaskDTO { private static SolveMailboxInconsistenciesTaskDTO toDTO(SolveMailboxInconsistenciesTask domainObject, String typeName) { - return new SolveMailboxInconsistenciesTaskDTO(typeName); + return new SolveMailboxInconsistenciesTaskDTO(typeName, + Optional.of(SolveMailboxInconsistenciesRunningOptionsDTO.asDTO(domainObject.getRunningOptions()))); } public static TaskDTOModule<SolveMailboxInconsistenciesTask, SolveMailboxInconsistenciesTaskDTO> module(SolveMailboxInconsistenciesService service) { @@ -40,17 +44,27 @@ public class SolveMailboxInconsistenciesTaskDTO implements TaskDTO { } private final String type; + private final Optional<SolveMailboxInconsistenciesRunningOptionsDTO> runningOptions; - public SolveMailboxInconsistenciesTaskDTO(@JsonProperty("type") String type) { + public SolveMailboxInconsistenciesTaskDTO(@JsonProperty("type") String type, + @JsonProperty("runningOptions") Optional<SolveMailboxInconsistenciesRunningOptionsDTO> runningOptions) { this.type = type; + this.runningOptions = runningOptions; } private SolveMailboxInconsistenciesTask toDomainObject(SolveMailboxInconsistenciesService service) { - return new SolveMailboxInconsistenciesTask(service); + return new SolveMailboxInconsistenciesTask(service, + runningOptions + .map(SolveMailboxInconsistenciesRunningOptionsDTO::asDomainObject) + .orElse(RunningOptions.DEFAULT)); } @Override public String getType() { return type; } + + public Optional<SolveMailboxInconsistenciesRunningOptionsDTO> getRunningOptions() { + return runningOptions; + } } diff --git a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesTaskSerializationTest.java b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesTaskSerializationTest.java index e44d3474ad..85d25369eb 100644 --- a/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesTaskSerializationTest.java +++ b/mailbox/cassandra/src/test/java/org/apache/james/mailbox/cassandra/mail/task/SolveMailboxInconsistenciesTaskSerializationTest.java @@ -41,14 +41,16 @@ class SolveMailboxInconsistenciesTaskSerializationTest { public static final String MAILBOX_ID_AS_STRING = "464765a0-e4e7-11e4-aba4-710c1de3782b"; private static final CassandraId MAILBOX_ID = CassandraId.of(UUID.fromString(MAILBOX_ID_AS_STRING)); + private static final SolveMailboxInconsistenciesService.RunningOptions RUNNING_OPTIONS = new SolveMailboxInconsistenciesService.RunningOptions(3); + private static final SolveMailboxInconsistenciesService SERVICE = mock(SolveMailboxInconsistenciesService.class); - private static final SolveMailboxInconsistenciesTask TASK = new SolveMailboxInconsistenciesTask(SERVICE); - private static final String SERIALIZED_TASK = "{\"type\": \"solve-mailbox-inconsistencies\"}"; + private static final SolveMailboxInconsistenciesTask TASK = new SolveMailboxInconsistenciesTask(SERVICE, RUNNING_OPTIONS); + private static final String SERIALIZED_TASK = "{\"type\": \"solve-mailbox-inconsistencies\", \"runningOptions\":{\"maxIterations\":3}}"; private static final ConflictingEntry CONFLICTING_ENTRY = ConflictingEntry.builder() .mailboxDaoEntry(MAILBOX_PATH, MAILBOX_ID) .mailboxPathDaoEntry(MAILBOX_PATH_2, MAILBOX_ID); private static final ImmutableList<ConflictingEntry> CONFLICTING_ENTRIES = ImmutableList.of(CONFLICTING_ENTRY); - private static final SolveMailboxInconsistenciesTask.Details DETAILS = new SolveMailboxInconsistenciesTask.Details(TIMESTAMP, 0, 1, ImmutableList.of(MAILBOX_ID_AS_STRING), CONFLICTING_ENTRIES, 3); + private static final SolveMailboxInconsistenciesTask.Details DETAILS = new SolveMailboxInconsistenciesTask.Details(TIMESTAMP, 0, 1, ImmutableList.of(MAILBOX_ID_AS_STRING), CONFLICTING_ENTRIES, 3, RUNNING_OPTIONS); private static final String SERIALIZED_ADDITIONAL_INFORMATION = "{" + " \"type\":\"solve-mailbox-inconsistencies\"," + " \"processedMailboxEntries\":0," + @@ -65,7 +67,8 @@ class SolveMailboxInconsistenciesTaskSerializationTest { " }" + " }]," + " \"errors\":3," + - " \"timestamp\":\"2018-11-13T12:00:55Z\"" + + " \"timestamp\":\"2018-11-13T12:00:55Z\"," + + " \"runningOptions\":{\"maxIterations\":3}" + "}"; @Test diff --git a/server/protocols/webadmin/webadmin-cassandra/src/main/java/org/apache/james/webadmin/routes/SolveMailboxInconsistenciesRequestToTask.java b/server/protocols/webadmin/webadmin-cassandra/src/main/java/org/apache/james/webadmin/routes/SolveMailboxInconsistenciesRequestToTask.java index 0df2783b14..6ec5928b9c 100644 --- a/server/protocols/webadmin/webadmin-cassandra/src/main/java/org/apache/james/webadmin/routes/SolveMailboxInconsistenciesRequestToTask.java +++ b/server/protocols/webadmin/webadmin-cassandra/src/main/java/org/apache/james/webadmin/routes/SolveMailboxInconsistenciesRequestToTask.java @@ -22,6 +22,7 @@ package org.apache.james.webadmin.routes; import jakarta.inject.Inject; import org.apache.james.mailbox.cassandra.mail.task.SolveMailboxInconsistenciesService; +import org.apache.james.mailbox.cassandra.mail.task.SolveMailboxInconsistenciesService.RunningOptions; import org.apache.james.mailbox.cassandra.mail.task.SolveMailboxInconsistenciesTask; import org.apache.james.webadmin.tasks.TaskFromRequestRegistry; import org.apache.james.webadmin.tasks.TaskRegistrationKey; @@ -42,7 +43,11 @@ public class SolveMailboxInconsistenciesRequestToTask extends TaskFromRequestReg "`ALL-SERVICES-ARE-OFFLINE` in order to prevent accidental calls. " + "Check the documentation for details."); - return new SolveMailboxInconsistenciesTask(service); + RunningOptions runningOptions = RunningOptionsParser.intQueryParameter(request, "maxIterations") + .map(RunningOptions::new) + .orElse(RunningOptions.DEFAULT); + + return new SolveMailboxInconsistenciesTask(service, runningOptions); }); } } diff --git a/server/protocols/webadmin/webadmin-cassandra/src/test/java/org/apache/james/webadmin/routes/SolveMailboxInconsistenciesRequestToTaskTest.java b/server/protocols/webadmin/webadmin-cassandra/src/test/java/org/apache/james/webadmin/routes/SolveMailboxInconsistenciesRequestToTaskTest.java index b0a3cc7b89..5e34d34fe3 100644 --- a/server/protocols/webadmin/webadmin-cassandra/src/test/java/org/apache/james/webadmin/routes/SolveMailboxInconsistenciesRequestToTaskTest.java +++ b/server/protocols/webadmin/webadmin-cassandra/src/test/java/org/apache/james/webadmin/routes/SolveMailboxInconsistenciesRequestToTaskTest.java @@ -86,7 +86,7 @@ class SolveMailboxInconsistenciesRequestToTaskTest { taskManager = new MemoryTaskManager(new Hostname("foo")); service = mock(SolveMailboxInconsistenciesService.class); - Mockito.when(service.fixMailboxInconsistencies(any())).thenReturn(Mono.just(Task.Result.COMPLETED)); + Mockito.when(service.fixMailboxInconsistencies(any(), any())).thenReturn(Mono.just(Task.Result.COMPLETED)); webAdminServer = WebAdminUtils.createWebAdminServer( new TasksRoutes(taskManager, jsonTransformer, DTOConverter.of(SolveMessageInconsistenciesTaskAdditionalInformationDTO.module())), --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
