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.

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

Communication: Speedup messaging #7404

Merged
merged 8 commits into from
Oct 17, 2023
Merged

Communication: Speedup messaging #7404

merged 8 commits into from
Oct 17, 2023

Conversation

lennart-keller
Copy link
Contributor

@lennart-keller lennart-keller commented Oct 17, 2023

Checklist

General

Server

  • Important: I implemented the changes with a very good performance and prevented too many (unnecessary) database calls.
  • I followed the coding and design guidelines.
  • I documented the Java code using JavaDoc style.

Motivation and Context

The query for retrieving recipients for WebSocket messages takes too much time on production systems.

Description

The query does not return the whole user anymore. Additionally, the join with the course table has been removed.

Steps for Testing

Prerequisites:

  • 2 Students
  • 1 Course with messaging enabled
  1. Write messages in the on the messaging page, a lecture details page, and an exercise details page as student 1 in channels, group chats and direct chats that both students are a member of
  2. Student 1 should see the message without much delay
  3. Student 2 should see a notification (if not on the same page as student 1) or the message without much delay if they have opened the conversation.
  4. Student 2 does not receive duplicate notifications if mentioned
  5. If student 1 writes in a channel or group chat that student 2 isn't a member of, student 2 does not receive notifications

Review Progress

Performance Review

  • I (as a reviewer) confirm that the server changes (in particular related to database calls) are implemented with a very good performance

Code Review

  • Code Review 1
  • Code Review 2

Manual Tests

  • Test 1
  • Test 2

Test Coverage

Screenshots

No UI Changes

@lennart-keller lennart-keller requested a review from a team as a code owner October 17, 2023 11:56
@github-actions github-actions bot added the server Pull requests that update Java code. (Added Automatically!) label Oct 17, 2023
Copy link
Collaborator

@MaximilianAnzinger MaximilianAnzinger left a comment

Choose a reason for hiding this comment

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

Code lgtm
Manually tested on TS2

Copy link
Contributor

@basak-akan basak-akan left a comment

Choose a reason for hiding this comment

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

Tested in testing session on TS2 worked as described, message delay reduced.

Copy link
Contributor

@rstief rstief left a comment

Choose a reason for hiding this comment

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

Tested in testing session (ts2)

@krusche krusche added this to the 6.6.1 milestone Oct 17, 2023
@krusche krusche merged commit 6f244cc into develop Oct 17, 2023
18 of 22 checks passed
@krusche krusche deleted the performance/messages branch October 17, 2023 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
server Pull requests that update Java code. (Added Automatically!)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants