You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by "vttranlina (via GitHub)" <gi...@apache.org> on 2023/03/11 02:57:47 UTC
[GitHub] [james-project] vttranlina opened a new pull request, #1477: JAMES-3440 Fixbug: Data race issue with JMAP email query view
vttranlina opened a new pull request, #1477:
URL: https://github.com/apache/james-project/pull/1477
We spotted a possible data race issue when populating the query email view, giving potentially false results when fetching emails ids via the email query view with Email/query jmap method
For example, for a user
```json
{
"using": [
"urn:ietf:params:jmap:core",
"urn:ietf:params:jmap:mail"
],
"methodCalls": [
[
"Email/query",
{
"accountId": "59ecb678c68f69e90d50b580d14512b6c2ec520afe5a3c522113b100235df799",
"filter": {
"inMailbox": "bb6b69d0-32cf-11eb-995c-a3ae66e9f96a"
},
"sort": [
{
"isAscending": false,
"property": "receivedAt"
}
],
"limit": 20
},
"c2"
],
[
"Email/get",
{
"accountId": "59ecb678c68f69e90d50b580d14512b6c2ec520afe5a3c522113b100235df799",
"#ids": {
"resultOf": "c2",
"name": "Email/query",
"path": "ids/*"
}
},
"c3"
]
]
}
```
Then the response
```json
{
"sessionState": "2c9f1b12-b35a-43e6-9af2-0106fb53a943",
"methodResponses": [
[
"Email/query",
{
"accountId": "59ecb678c68f69e90d50b580d14512b6c2ec520afe5a3c522113b100235df799",
"queryState": "d36356fd",
"canCalculateChanges": false,
"ids": [
"8a6b5e00-f4f1-11eb-b5be-2347bf0946f3",
"6458a9c0-f4f1-11eb-b5be-2347bf0946f3",
"4f07e100-5151-11ec-b483-3d35a4dab7c4"
],
"position": 0
},
"c2"
],
[
"Email/get",
{
"accountId": "59ecb678c68f69e90d50b580d14512b6c2ec520afe5a3c522113b100235df799",
"notFound": [],
"state": "497c0f10-bf1c-11ed-8da6-c5628a8cf003",
"list": [
{
"id": "6458a9c0-f4f1-11eb-b5be-2347bf0946f3",
"mailboxIds": {
"bb6b69d0-32cf-11eb-995c-a3ae66e9f96a": true,
"bb794c80-32cf-11eb-995c-a3ae66e9f96a": true
}
},
{
"id": "8a6b5e00-f4f1-11eb-b5be-2347bf0946f3",
"mailboxIds": {
"bb794c80-32cf-11eb-995c-a3ae66e9f96a": true
}
},
{
"id": "4f07e100-5151-11ec-b483-3d35a4dab7c4",
"mailboxIds": {
"bb6b69d0-32cf-11eb-995c-a3ae66e9f96a": true
}
}
]
},
"c3"
]
]
}
```
The messageId `4f07e100-5151-11ec-b483-3d35a4dab7c4` is not in `bb6b69d0-32cf-11eb-995c-a3ae66e9f96a`,
but It is still in the response
The mailbox targeted here is Outbox. Some emails returned are part of that mailbox, some others are part of the Sent mailbox, some others are part of both.
When sending email, normal sequence should be:
* users sends the email thus saves it in outbox
* populate email query view is populated for outbox
* system moves the email into sent box
* populate email query view is populated for sent
What maybe happens is:
* users sends the email thus saves it in outbox
* system moves the email into sent box
* populate email query view is populated for sent
* populate email query view is populated for outbox
We need to find a way to handle this case correctly.
## How to fix it?
in the `PopulateEmailQueryViewListener`. re-check the current mailbox-id of a message to ensure it is not outdated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] quantranhong1999 commented on pull request #1477: JAMES-3440 Fixbug: Data race issue with JMAP email query view
Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on PR #1477:
URL: https://github.com/apache/james-project/pull/1477#issuecomment-1465747916
> Network is unreachable
FYI, the new intern is having this issue too build the project locally.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a diff in pull request #1477: JAMES-3440 Fixbug: Data race issue with JMAP email query view
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1477:
URL: https://github.com/apache/james-project/pull/1477#discussion_r1133047113
##########
server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/event/PopulateEmailQueryViewListener.java:
##########
@@ -156,13 +157,22 @@ private Mono<Void> handleAdded(Added added) {
private Mono<Void> handleAdded(Added added, MessageMetaData messageMetaData, MailboxSession session) {
MessageId messageId = messageMetaData.getMessageId();
+ MailboxId mailboxId = added.getMailboxId();
- return Flux.from(messageIdManager.getMessagesReactive(ImmutableList.of(messageId), FetchGroup.HEADERS, session))
- .next()
+ return checkMessageStillInOriginMailbox(messageId, session, mailboxId)
+ .filter(FunctionalUtils.identityPredicate())
+ .flatMap(stillInOriginMailbox -> Flux.from(messageIdManager.getMessagesReactive(ImmutableList.of(messageId), FetchGroup.HEADERS, session))
+ .next())
.filter(message -> !message.getFlags().contains(DELETED))
.flatMap(messageResult -> handleAdded(added.getMailboxId(), messageResult));
}
Review Comment:
Please do this check only if `added.getMailboxPath().getName().isEqualTo(OUTBOX)` to limit its cost....
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] vttranlina commented on pull request #1477: JAMES-3440 Fixbug: Data race issue with JMAP email query view
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on PR #1477:
URL: https://github.com/apache/james-project/pull/1477#issuecomment-1465657675
> ```
> 03:49:22,048 [ERROR] Failed to execute goal on project scaling-pulsar-smtp: Could not resolve dependencies for project org.apache.james:scaling-pulsar-smtp:jar:3.8.0-SNAPSHOT: Failed to collect dependencies at org.apache.james:james-server-guice-es-resporter:jar:3.8.0-SNAPSHOT: Failed to read artifact descriptor for org.apache.james:james-server-guice-es-resporter:jar:3.8.0-SNAPSHOT: Could not transfer artifact org.apache.james:james-server-guice-es-resporter:pom:3.8.0-SNAPSHOT from/to Nexus (https://repository.apache.org/snapshots): transfer failed for https://repository.apache.org/snapshots/org/apache/james/james-server-guice-es-resporter/3.8.0-SNAPSHOT/james-server-guice-es-resporter-3.8.0-SNAPSHOT.pom: Connect to repository.apache.org:443 [repository.apache.org/136.243.146.148, repository.apache.org/2a01:4f8:171:1513:0:0:0:2] failed: Network is unreachable (connect failed) -> [Help 1]
> org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal on project scaling-pulsar-smtp: Could not resolve dependencies for project org.apache.james:scaling-pulsar-smtp:jar:3.8.0-SNAPSHOT: Failed to collect dependencies at org.apache.james:james-server-guice-es-resporter:jar:3.8.0-SNAPSHOT
> ```
Succeed on my local, and this commit is already based on latest master branch.
It look like CI issue?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #1477: JAMES-3440 Fixbug: Data race issue with JMAP email query view
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on PR #1477:
URL: https://github.com/apache/james-project/pull/1477#issuecomment-1465665616
There is never smoke without fire.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a diff in pull request #1477: JAMES-3440 Fixbug: Data race issue with JMAP email query view
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1477:
URL: https://github.com/apache/james-project/pull/1477#discussion_r1133411713
##########
mailbox/opensearch/src/test/java/org/apache/james/mailbox/opensearch/events/OpenSearchListeningMessageSearchIndexTest.java:
##########
@@ -289,6 +297,54 @@ void addShouldPropagateExceptionWhenExceptionOccurs() throws Exception {
openSearch.getDockerOpenSearch().unpause();
}
+ @Test
+ void addAOutdatedMessageInOutBoxShouldNotIndex() throws Exception {
Review Comment:
Cool to see no changes is needed for OpenSearch!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a diff in pull request #1477: JAMES-3440 Fixbug: Data race issue with JMAP email query view
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1477:
URL: https://github.com/apache/james-project/pull/1477#discussion_r1133411410
##########
server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/event/PopulateEmailQueryViewListener.java:
##########
@@ -159,12 +160,16 @@ private Mono<Void> handleAdded(Added added, MessageMetaData messageMetaData, Mai
MessageId messageId = messageMetaData.getMessageId();
MailboxId mailboxId = added.getMailboxId();
- return checkMessageStillInOriginMailbox(messageId, session, mailboxId)
- .filter(FunctionalUtils.identityPredicate())
- .flatMap(stillInOriginMailbox -> Flux.from(messageIdManager.getMessagesReactive(ImmutableList.of(messageId), FetchGroup.HEADERS, session))
- .next())
+ Mono<Void> doHandleAdded = Flux.from(messageIdManager.getMessagesReactive(ImmutableList.of(messageId), FetchGroup.HEADERS, session))
+ .next()
.filter(message -> !message.getFlags().contains(DELETED))
.flatMap(messageResult -> handleAdded(added.getMailboxId(), messageResult));
+ if (DefaultMailboxes.OUTBOX.equalsIgnoreCase(added.getMailboxPath().getName())) {
Review Comment:
`Role.from(added.getMailboxPath().getName()).contains(Role.OUTBOX)` would even be better...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa merged pull request #1477: JAMES-3440 Fixbug: Data race issue with JMAP email query view
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa merged PR #1477:
URL: https://github.com/apache/james-project/pull/1477
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #1477: JAMES-3440 Fixbug: Data race issue with JMAP email query view
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on PR #1477:
URL: https://github.com/apache/james-project/pull/1477#issuecomment-1465472443
```
03:49:22,048 [ERROR] Failed to execute goal on project scaling-pulsar-smtp: Could not resolve dependencies for project org.apache.james:scaling-pulsar-smtp:jar:3.8.0-SNAPSHOT: Failed to collect dependencies at org.apache.james:james-server-guice-es-resporter:jar:3.8.0-SNAPSHOT: Failed to read artifact descriptor for org.apache.james:james-server-guice-es-resporter:jar:3.8.0-SNAPSHOT: Could not transfer artifact org.apache.james:james-server-guice-es-resporter:pom:3.8.0-SNAPSHOT from/to Nexus (https://repository.apache.org/snapshots): transfer failed for https://repository.apache.org/snapshots/org/apache/james/james-server-guice-es-resporter/3.8.0-SNAPSHOT/james-server-guice-es-resporter-3.8.0-SNAPSHOT.pom: Connect to repository.apache.org:443 [repository.apache.org/136.243.146.148, repository.apache.org/2a01:4f8:171:1513:0:0:0:2] failed: Network is unreachable (connect failed) -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal on project scaling-pulsar-smtp: Could not resolve dependencies for project org.apache.james:scaling-pulsar-smtp:jar:3.8.0-SNAPSHOT: Failed to collect dependencies at org.apache.james:james-server-guice-es-resporter:jar:3.8.0-SNAPSHOT
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] vttranlina commented on pull request #1477: JAMES-3440 Fixbug: Data race issue with JMAP email query view
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on PR #1477:
URL: https://github.com/apache/james-project/pull/1477#issuecomment-1465701800
```
Caused by: org.apache.maven.wagon.providers.http.httpclient.conn.HttpHostConnectException: Connect to repository.apache.org:443 [repository.apache.org/136.243.146.148, repository.apache.org/2a01:4f8:171:1513:0:0:0:2] failed: Network is unreachable (connect failed)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] vttranlina commented on pull request #1477: JAMES-3440 Fixbug: Data race issue with JMAP email query view
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on PR #1477:
URL: https://github.com/apache/james-project/pull/1477#issuecomment-1469251281
rebase
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #1477: JAMES-3440 Fixbug: Data race issue with JMAP email query view
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on PR #1477:
URL: https://github.com/apache/james-project/pull/1477#issuecomment-1464851421
Please do a similar for OpenSearch search index too!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] vttranlina commented on pull request #1477: JAMES-3440 Fixbug: Data race issue with JMAP email query view
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on PR #1477:
URL: https://github.com/apache/james-project/pull/1477#issuecomment-1465414287
> Please do a similar for OpenSearch search index too!
The OpenSearch index did that.
Quote `ListeningMessageSearchIndex.java`
```java
private Mono<Void> handleAdded(MailboxSession session, Mailbox mailbox, Added added) {
return Flux.fromIterable(MessageRange.toRanges(added.getUids()))
.concatMap(range -> retrieveMailboxMessages(session, mailbox, range))
.publishOn(Schedulers.parallel())
.concatMap(mailboxMessage -> add(session, mailbox, mailboxMessage))
.then();
}
private Flux<MailboxMessage> retrieveMailboxMessages(MailboxSession session, Mailbox mailbox, MessageRange range) {
return factory.getMessageMapper(session)
.findInMailboxReactive(mailbox, range, FetchType.FULL, UNLIMITED);
}
```
The `MailboxSessionMapperFactory factory` query db for latest MailboxMessage
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org