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

feat: question matching #50

Merged
merged 38 commits into from
Oct 26, 2024
Merged

feat: question matching #50

merged 38 commits into from
Oct 26, 2024

Conversation

tituschewxj
Copy link

@tituschewxj tituschewxj commented Oct 24, 2024

Summary

This PR aims to implement a question service API that returns a question given matching parameters, and linking this API to the matching service.

Changes:

  • Updated the matching algorithm for more robust matching, which prioritizes user at the front of the queue.
  • Implemented gRPC server
  • Implemented gRPC client request
  • Implemented the question finding algorithm
  • Integrated with current frontend
  • Implemented tests
  • Updated docker: the repo variables and environment file has been updated with QUESTION_SERVICE_GRPC_URL

Note: JWT verification is not implemented yet.

Implementation

Given that the match result should be returned as soon as possible, a synchronous call makes more sense in this case, although this results in higher coupling between the question service and matching service. gRPC was chosen as it has better performance compared to REST.

Matching process (done in matching service)

  1. User A and User B matches on Easy and Algorithms. Both users are removed from the matching queue.
  2. Then this information is sent to the question service.
  3. The question service returns a question ID through the Question matching algorithm below. Note: The frontend should only show the options for which there are questions for., otherwise two users may match on a different topic due to the fallback in the question matching algorithm.
  4. The matching service gets the result back and returns the response to both A and B.

Question matching algorithm (done in question service)

  1. If there are questions with any of the specified topics and difficulty found, return one of those questions randomly.
  2. If no questions from any of the specified topics and difficulty is found, find a question of any of the specified topics. If found, return one at random.
  3. If no questions from any of the specified topics is found, find a question of any of the specified difficulties. If found, return one at random.
  4. If no question is still found, then return a random question.

Warning

If there are no questions in the database, then the question matching service would fail.

API Structure

Request to question-service from matching-service:

{
  "matched_topics": ["Arrays", "Algorithms", "Strings"],
  "matched_difficulties": ["Easy", "Medium"],
}

Response from question-service on question found:

{
  "question_doc_ref_id": "123",
  "question_name": "abcde",
  "question_difficulty": "Easy",
  "question_topics": ["Arrays", "Algorithms", "Greedy"]
}

Response from matching-service to user on match found:

{
  "type": "match_question_found",
  "match_id": "12345",
  "user": "A",
  "matched_user": "B",
  "matched_topics": ["Arrays", "Algorithms"],
  "question_doc_ref_id": "123",
  "question_name": "abced",
  "question_difficulty": "Easy",
  "question_topics": ["Arrays", "Algorithms", "Greedy"]
}
  • matched_topics is still returned to the frontend when question_topics is present. If we are able to distinguish which topics users match on and display it to the users, it would be informative. Possible improvement for frontend.

Testing

We can use docker-compose up --build. Ensure that the environment variables are correct.

websocket-test.html provides an interface for easier testing for testing the matching queue. It can be used once the docker compose is ready or manually with go run main.go in both the question and matching service. (Try not to use too many users or it may exceed the firebase free limit.)

image

Known Issues

  • If multiple users request at the same time, it is possible that the context deadline exceeds, causing the service to fail, we should look into whether we can implement jittered backoff strategy for obtaining the redis lock.
matching-service_1  | 2024/10/26 06:07:10 context deadline exceeded
apps_matching-service_1 exited with code 1

@tituschewxj tituschewxj self-assigned this Oct 24, 2024
@tituschewxj tituschewxj marked this pull request as draft October 24, 2024 03:58
@tituschewxj tituschewxj changed the title Titus/question matching feat/question matching Oct 24, 2024
@tituschewxj tituschewxj added the enhancement New feature or request label Oct 24, 2024
@tituschewxj tituschewxj changed the title feat/question matching feat: question matching Oct 24, 2024
@tituschewxj tituschewxj marked this pull request as ready for review October 25, 2024 17:15
@solomonng2001
Copy link

solomonng2001 commented Oct 26, 2024

@tituschewxj looks great! I noticed the following 2 behaviours of the matching service

  1. When selecting "easy" and "arrays" for admin and "hard" and "arrays" for notadmin, matching service will timeout without a match found, but I was expecting that the algorithm will fall back to case 2 with a random "arrays" question.
Screenshot 2024-10-26 at 5 40 07 PM
  1. When selecting "medium" and "algorithms" on admin and "medium" and "radix sort" on notadmin, there is a timeout without a match found, but I was expecting that the algo will fall back to case 3 with a random medium question.
Screenshot 2024-10-26 at 5 47 03 PM
  1. When selecting "easy" and "arrays" for admin and "hard" and "radix sort" on not admin, there is a timeout without a match found, but I was expecting that the algo will fall back to case 4 with a random question of arbitrary topic and difficulty.

The rest LGTM. Thanks for your diligent backend design!

@tituschewxj
Copy link
Author

tituschewxj commented Oct 26, 2024

  1. When selecting "easy" and "arrays" for admin and "hard" and "arrays" for notadmin, matching service will timeout without a match found, but I was expecting that the algorithm will fall back to case 4 of the matching service algorithm, such that a random question is still generated.
  1. When selecting "medium" and "algorithms" on admin and "medium" and "radix sort" on notadmin, there is a timeout without a match found, but I was expecting that the also will fall back to case 3 with a random medium question.

I think these are intended behaviour as there are two stages to the matching. Both stages are independent of each other.

Stage 1: Two users have to match first before proceeding to the question matching stage in our current algorithm.

Stage 2: The question finding algorithm tries to find a question that meets the criteria from the question bank.

Maybe we can revisit how the matching should be done later if we have time, if anyone has any suggestions how we can improve on this design.

@solomonng2001
Copy link

  1. When selecting "easy" and "arrays" for admin and "hard" and "arrays" for notadmin, matching service will timeout without a match found, but I was expecting that the algorithm will fall back to case 4 of the matching service algorithm, such that a random question is still generated.
  1. When selecting "medium" and "algorithms" on admin and "medium" and "radix sort" on notadmin, there is a timeout without a match found, but I was expecting that the also will fall back to case 3 with a random medium question.

I think these are intended behaviour as there are two stages to the matching. Both stages are independent of each other.

Stage 1: Two users have to match first before proceeding to the question matching stage in our current algorithm.

Stage 2: The question finding algorithm tries to find a question that meets the criteria from the question bank.

Maybe we can revisit how the matching should be done later if we have time, if anyone has any suggestions how we can improve on this design.

Noted that the behaviour in question finding algorithm is contingent on the users being matched, perhaps we could make this clearer as a note in our report.

Copy link

@solomonng2001 solomonng2001 left a comment

Choose a reason for hiding this comment

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

LGTM

@solomonng2001 solomonng2001 merged commit 0f16864 into staging Oct 26, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants