Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ISSUE-1482 Bundle compute JMAP preview into OpenSearch indexing #1508

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

ISSUE-1482 Bundle compute JMAP preview into OpenSearch indexing #1508

wants to merge 7 commits into from
+990 −2

Conversation

quantranhong1999
Copy link
Member

@quantranhong1999 quantranhong1999 commented Feb 4, 2025 •
edited
Loading

No description provided.

@quantranhong1999 quantranhong1999 self-assigned this Feb 4, 2025
@quantranhong1999 quantranhong1999 mentioned this pull request Feb 4, 2025
Open
@chibenwa
Copy link
Member

chibenwa commented Feb 4, 2025

just bind the CombinedOpenSearchIndexingAndComputeMessageFastViewProjectionListener when OpenSearch is chosen.

+1

@chibenwa
Copy link
Member

chibenwa commented Feb 4, 2025

Need to find a way to unbind the two listeners on James: either find Guice trick or filter out registration of the related Group/ Listeners in TMail event bus.

Redefine the module structure

@chibenwa
Copy link
Member

chibenwa commented Feb 4, 2025

  • Performance tests

chibenwa
chibenwa reviewed Feb 4, 2025
View reviewed changes
Copy link
Member

@chibenwa chibenwa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too Long Did't Read
Too much code duplication

@quantranhong1999
[DO NOT MERGE] Update James SHA1
939c1b4 
to the pending James changes
@quantranhong1999
ISSUE-1482 Implement ComputePreviewMessageIndexer
f102470 
ComputePreviewMessageIndexer: handle indexing JMAP preview as part of OpenSearch indexing process
ExpungeMessageFastViewProjectionListener: handle removing JMAP preview upon Expunged
@quantranhong1999
ISSUE-1482 Guice binding for ComputePreviewMessageIndexer
cb606f5
@quantranhong1999
ISSUE-1482 Integration tests for bundle compute JMAP preview and Open…
32da49c 
…Search indexing
@quantranhong1999 quantranhong1999 force-pushed the combine-opensearch-index-and-compute-preview-listener branch from f73ae67 to 32da49c Compare February 7, 2025 08:54
@quantranhong1999 quantranhong1999 marked this pull request as ready for review February 7, 2025 08:56
@quantranhong1999 quantranhong1999 changed the title ISSUE-1482 Introduce CombinedOpenSearchIndexingAndComputeMessageFastViewProjectionListener ISSUE-1482 Bundle compute JMAP preview into OpenSearch indexing Feb 7, 2025
@quantranhong1999
Copy link
Member Author

I have reworked this following apache/james-project#2631

@chibenwa
Copy link
Member

chibenwa commented Feb 7, 2025

Can we have a test with Cassandra instrumentation that enforced that when we happen once a message into the mailbox we get only a single INSERT INTO message_fast_view_projection ?

Otherwise yes indeed: Good job!

@chibenwa
Copy link
Member

chibenwa commented Feb 7, 2025 •
edited
Loading

Please test also when we are in header indexing only mode: we still need to have the preview...

Add a test for distributed server.

@chibenwa
Copy link
Member

chibenwa commented Feb 7, 2025

And do not forget to performance test this

Arsnael
Arsnael reviewed Feb 7, 2025
View reviewed changes
Copy link
Member

@Arsnael Arsnael left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read it nothing to add

@Arsnael
Copy link
Member

Arsnael commented Feb 10, 2025

Good for rebase!

@quantranhong1999
Copy link
Member Author

And do not forget to performance test this

Before

Overall performance test

image

Metrics

mailbox_listener_ComputeMessageFastViewProjectionListener{quantile="0.5",} 0.039059455
mailbox_listener_ComputeMessageFastViewProjectionListener{quantile="0.75",} 0.061865983000000006
mailbox_listener_ComputeMessageFastViewProjectionListener{quantile="0.95",} 0.12373196700000001
mailbox_listener_ComputeMessageFastViewProjectionListener{quantile="0.98",} 0.199229439
mailbox_listener_ComputeMessageFastViewProjectionListener{quantile="0.99",} 0.30828134300000004
mailbox_listener_ComputeMessageFastViewProjectionListener{quantile="0.999",} 1.065353215
mailbox_listener_ComputeMessageFastViewProjectionListener_count 31916.0

mailbox_listener_OpenSearchListeningMessageSearchIndex{quantile="0.5",} 0.040370175
mailbox_listener_OpenSearchListeningMessageSearchIndex{quantile="0.75",} 0.065273855
mailbox_listener_OpenSearchListeningMessageSearchIndex{quantile="0.95",} 0.130023423
mailbox_listener_OpenSearchListeningMessageSearchIndex{quantile="0.98",} 0.19084083100000002
mailbox_listener_OpenSearchListeningMessageSearchIndex{quantile="0.99",} 0.272629759
mailbox_listener_OpenSearchListeningMessageSearchIndex{quantile="0.999",} 1.0821304310000002
mailbox_listener_OpenSearchListeningMessageSearchIndex_count 42127.0

=> p99 ComputeMessageFastViewProjectionListener + p99 OpenSearchListeningMessageSearchIndex = 570ms

After

Overall performance test

image

A bit better performance overall than before (p99 1057ms vs 1206ms).

Metrics

mailbox_listener_ExpungeMessageFastViewProjectionListener{quantile="0.5",} 0.007372799
mailbox_listener_ExpungeMessageFastViewProjectionListener{quantile="0.75",} 0.011665407000000001
mailbox_listener_ExpungeMessageFastViewProjectionListener{quantile="0.95",} 0.024379391
mailbox_listener_ExpungeMessageFastViewProjectionListener{quantile="0.98",} 0.036962303
mailbox_listener_ExpungeMessageFastViewProjectionListener{quantile="0.99",} 0.061341695
mailbox_listener_ExpungeMessageFastViewProjectionListener{quantile="0.999",} 0.364904447
mailbox_listener_ExpungeMessageFastViewProjectionListener_count 12222.0

mailbox_listener_OpenSearchListeningMessageSearchIndex{quantile="0.5",} 0.044040191
mailbox_listener_OpenSearchListeningMessageSearchIndex{quantile="0.75",} 0.068681727
mailbox_listener_OpenSearchListeningMessageSearchIndex{quantile="0.95",} 0.137363455
mailbox_listener_OpenSearchListeningMessageSearchIndex{quantile="0.98",} 0.22124953500000002
mailbox_listener_OpenSearchListeningMessageSearchIndex{quantile="0.99",} 0.316669951
mailbox_listener_OpenSearchListeningMessageSearchIndex{quantile="0.999",} 1.1324620790000002
mailbox_listener_OpenSearchListeningMessageSearchIndex_count 56956.0

=> p99 ExpungeMessageFastViewProjectionListener + p99 OpenSearchListeningMessageSearchIndex = 380ms

Summary

  • A bit better performance overall than before (p99 1057ms vs 1206ms) -> could just sandbox platform is unstable
  • less ~200ms on listener execution time (380ms vs 570ms) -> quite significant IMO
  • reduce S3 calls (of course)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chibenwa chibenwa chibenwa left review comments

@Arsnael Arsnael Arsnael left review comments

Assignees

@quantranhong1999 quantranhong1999

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@quantranhong1999 @chibenwa @Arsnael