-
Notifications
You must be signed in to change notification settings - Fork 2
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: Revamp matching service #39
Merged
Merged
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
e89f5e5
Update instructions on running matching-service with redis
bensohh 57c4fd9
Update env example and log statements in main
bensohh 6becc5c
Revamp matching service and update README
bensohh 021eade
Add port into logging
bensohh 58f9b3b
Update context so cleaning up after cancellation works
bensohh 7228473
Fix error with matchmaking service
bensohh ad3397d
fix: json for timeout
tituschewxj File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,7 +24,9 @@ var ( | |
activeConnections = make(map[string]*websocket.Conn) | ||
// A map to hold user's match ctx cancel function | ||
matchContexts = make(map[string]context.CancelFunc) | ||
mu sync.Mutex // Mutex for thread-safe access to activeConnections | ||
// A map to hold user's match channels | ||
matchFoundChannels = make(map[string]chan models.MatchFound) | ||
mu sync.Mutex // Mutex for thread-safe access to activeConnections | ||
) | ||
|
||
// handleConnections manages WebSocket connections and matching logic. | ||
|
@@ -67,6 +69,9 @@ func HandleWebSocketConnections(w http.ResponseWriter, r *http.Request) { | |
activeConnections[matchRequest.Username] = ws | ||
matchCtx, matchCancel := context.WithCancel(context.Background()) | ||
matchContexts[matchRequest.Username] = matchCancel | ||
|
||
matchFoundChan := make(chan models.MatchFound) | ||
matchFoundChannels[matchRequest.Username] = matchFoundChan | ||
mu.Unlock() | ||
|
||
// Create a context for cancellation | ||
|
@@ -84,11 +89,9 @@ func HandleWebSocketConnections(w http.ResponseWriter, r *http.Request) { | |
} | ||
defer timeoutCancel() | ||
|
||
matchFoundChan := make(chan models.MatchFound) | ||
|
||
// Start goroutines for handling messages and performing matching. | ||
go processes.ReadMessages(ws, ctx, cancel) | ||
go processes.PerformMatching(matchRequest, ctx, matchFoundChan) // Perform matching | ||
go processes.PerformMatching(matchRequest, context.Background(), matchFoundChannels) // Perform matching | ||
|
||
// Wait for a match, timeout, or cancellation. | ||
waitForResult(ws, ctx, timeoutCtx, matchCtx, matchFoundChan, matchRequest.Username) | ||
|
@@ -127,29 +130,47 @@ func waitForResult(ws *websocket.Conn, ctx, timeoutCtx, matchCtx context.Context | |
log.Println("Matching cancelled") | ||
// Cleanup Redis | ||
processes.CleanUpUser(processes.GetRedisClient(), username, context.Background()) | ||
// 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) | ||
} | ||
|
||
return | ||
case <-timeoutCtx.Done(): | ||
log.Println("Connection timed out") | ||
// Cleanup Redis | ||
processes.CleanUpUser(processes.GetRedisClient(), username, context.Background()) | ||
// 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) | ||
} | ||
|
||
sendTimeoutResponse(ws) | ||
return | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We probably do not need the case for |
||
if !ok { | ||
// Channel closed without a match, possibly due to context cancellation | ||
log.Println("Match channel closed without finding a match") | ||
return | ||
} | ||
log.Println("Match found for user: " + result.User) | ||
log.Println("Match found for user: " + username) | ||
// Notify the users about the match | ||
notifyMatch(result.User, result.MatchedUser, result) | ||
|
||
// if err := ws.WriteJSON(result); err != nil { | ||
// log.Printf("write error: %v", err) | ||
// } | ||
return | ||
} | ||
} | ||
|
@@ -185,13 +206,22 @@ func notifyMatch(username, matchedUsername string, result models.MatchFound) { | |
} | ||
|
||
// Remove the match context for both users and cancel for matched user | ||
if _, exists := matchContexts[username]; exists { | ||
if cancelFunc, exists := matchContexts[username]; exists { | ||
cancelFunc() | ||
delete(matchContexts, username) | ||
} | ||
|
||
if cancelFunc, exists := matchContexts[matchedUsername]; exists { | ||
if cancelFunc2, exists := matchContexts[matchedUsername]; exists { | ||
cancelFunc2() | ||
delete(matchContexts, matchedUsername) | ||
defer cancelFunc() // TODO: CancelFunction here is not causing the matchCtx to be done | ||
} | ||
|
||
// Remove the match channels | ||
if _, exists := matchFoundChannels[username]; exists { | ||
delete(matchFoundChannels, username) | ||
} | ||
if _, exists := matchFoundChannels[matchedUsername]; exists { | ||
delete(matchFoundChannels, matchedUsername) | ||
} | ||
|
||
// Remove users from the activeConnections map | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.