feat: Revamp matching service #39
Conversation
|
Newest Update on PR
Detailed Explanation of Matchmaking
I also attempted to utilise mutex to ensure that each time there is only one process editing/accessing the Redis Data Structures. Since there can be multiple connections, I created several global maps for matchContext, matchChannel, websocket connections. General Flow
Matching Algorithm
|
| case <-matchCtx.Done(): | ||
| log.Println("Match found for user HEREEE: " + username) | ||
| log.Println("Match found for user: " + username) | ||
| return | ||
| case result, ok := <-matchFoundChan: |
There was a problem hiding this comment.
We probably do not need the case for <-matchCtx.Done if we have the <-matchFoundChan that checks if matching is done
| // Remove the match context and active | ||
| if _, exists := matchContexts[username]; exists { | ||
| delete(matchContexts, username) | ||
| } | ||
| if _, exists := activeConnections[username]; exists { | ||
| delete(activeConnections, username) | ||
| } | ||
| if _, exists := matchFoundChannels[username]; exists { | ||
| delete(matchFoundChannels, username) | ||
| } |
There was a problem hiding this comment.
We should abstract out the match context and active connections, and the redis cleanup, to a function. We don't have to call it here, but we can call the function after waitForResult, since it is blocking, which should clean up the code.
| // SetRedisClient sets the Redis client to a global variable | ||
| func SetRedisClient(client *redis.Client) { | ||
| redisClient = client | ||
| } | ||
|
|
||
| // Get redisclient | ||
| func GetRedisClient() *redis.Client { | ||
| return redisClient | ||
| } | ||
|
|
There was a problem hiding this comment.
Maybe we can move this to a new file under the utils folder
| func getPortNumber(addr string) (int64, error) { | ||
| // Split the string by the colon | ||
| parts := strings.Split(addr, ":") | ||
| if len(parts) < 2 { | ||
| return 0, fmt.Errorf("no port number found") | ||
| } | ||
|
|
||
| matchFoundChan <- result | ||
| // Convert the port string to an integer | ||
| port, err := strconv.ParseInt(parts[len(parts)-1], 10, 64) | ||
| if err != nil { | ||
| return 0, err // Return an error if conversion fails | ||
| } | ||
|
|
||
| return port, nil | ||
| } |
There was a problem hiding this comment.
Probably can either remove this function as it is not called, or move to the utils folder if it has a use in future?
| // Acquire mutex | ||
| mu.Lock() | ||
| // Defer unlocking the mutex | ||
| defer mu.Unlock() |
There was a problem hiding this comment.
The mechanism for ensuring mutually exclusive access to the matchmaking process might not work when there are multiple instances of the matching-service, which happens when we scale the number of matching services.
This works for now, but we probably want a database level mutex, using something like https://github.com/amyangfei/redlock-go.
| var mutex sync.Mutex // Mutex for concurrency safety | ||
|
|
||
| // To simulate generating a random matchID for collaboration service (TODO: Future) | ||
| func GenerateMatchID() (string, error) { |
| for _, topic := range user.Topics { | ||
| users, err := client.SMembers(ctx, strings.ToLower(topic)).Result() | ||
| if err != nil { | ||
| return "", "", "", err | ||
| } | ||
|
|
||
| for _, potentialMatch := range users { | ||
| if potentialMatch != username { | ||
| matchedUser, err := GetUserDetails(client, potentialMatch, ctx) | ||
| if err != nil { | ||
| return "", "", "", err | ||
| } | ||
|
|
||
| commonDifficulty := GetCommonDifficulty(user.Difficulties, matchedUser.Difficulties) | ||
| return potentialMatch, topic, commonDifficulty, nil | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
There could be a problem when both users have the topics Algorithms and Arrays, but it is always that Algorithms topic gets chosen, because it comes first in the user.Topics, so they would never match on Arrays?
Also, I realize that two users can still be matched with different topics, through a question that contains both topics from users, since a question can have more than one topic. We probably should update the matching algorithm based on this.
| } | ||
|
|
||
| commonDifficulty := GetCommonDifficulty(user.Difficulties, matchedUser.Difficulties) | ||
| return potentialMatch, topic, commonDifficulty, nil |
There was a problem hiding this comment.
Do we want to also check whether there is such a question with the difficulty and topic?
If two users match, but the matching topic and difficult pair doesn't exist, how should we handle it?
We probably want to return the matched question in this step also.
| } | ||
|
|
||
| // Get the highest common difficulty between two users, if no common difficulty found, choose the min of the 2 arrays | ||
| func GetCommonDifficulty(userArr []string, matchedUserArr []string) string { |
There was a problem hiding this comment.
Probably we can find a matching question before the matching the difficulty?
Change the logic of matching service:
Added new type match_rejected when a user within an existing queue tries to matchmake again @chiaryan @solomonng2001 can take note of this for frontend. Refer to README.md for more details.
Current Situation:
@tituschewxj need your help to checkout my branch and
apps/matching-service/handlers/websocket.goAfter resolving the problem, we should be able to merge 👍