-
Notifications
You must be signed in to change notification settings - Fork 0
fix: resolve 9 UI and backend bugs #11
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
Changes from 4 commits
4234455
f8a4907
9f943d5
011cec4
8826d2e
11d3085
f21661f
df7e039
fa909e4
8bba3ef
205c8ad
c25a5fd
a1afd42
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -44,10 +44,13 @@ func (d *DB) GetEnabledLogSources(haproxyID int64) ([]LogSourceSetting, error) { | |||||
| } | ||||||
|
|
||||||
| // GetEnabledLogSourcesByServerID returns enabled log sources for a server by its server_id. | ||||||
| // Returns empty slice (not error) if the server has no HAProxy configuration or no log sources. | ||||||
| func (d *DB) GetEnabledLogSourcesByServerID(serverID string) ([]LogSourceSetting, error) { | ||||||
| d.mu.RLock() | ||||||
| defer d.mu.RUnlock() | ||||||
|
|
||||||
| // Use LEFT JOIN so servers without HAProxy entries return empty results instead of errors. | ||||||
|
||||||
| // Use LEFT JOIN so servers without HAProxy entries return empty results instead of errors. | |
| // Use a JOIN so only servers with HAProxy entries return log sources; others return empty results. |
Copilot
AI
Feb 2, 2026
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.
The comment on line 52 states "Use LEFT JOIN so servers without HAProxy entries return empty results" but the actual SQL query on line 57 uses an INNER JOIN. An INNER JOIN will only return rows where both tables have matching records, which means servers without HAProxy entries will indeed return empty results, but not for the reason stated. If the goal is to gracefully handle servers without HAProxy configuration, the comment should either:
- Be updated to match the implementation: "Use INNER JOIN which naturally returns empty results for servers without HAProxy entries", OR
- The query should be changed to use LEFT JOIN if there's a need to return additional information about servers without HAProxy configuration.
The current implementation appears correct for the stated goal (returning empty results), but the comment is misleading.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1207,18 +1207,23 @@ func (d *DB) SetUserSessionToken(userID, sessionToken, ip, userAgent string) err | |
| return err | ||
| } | ||
|
|
||
| // sessionActivityDebounce is the minimum interval between session_last_activity DB writes per user. | ||
| // This prevents write lock contention on the main mutex during rapid page refreshes. | ||
| const sessionActivityDebounce = 5 * time.Minute | ||
|
|
||
| // ValidateSessionToken checks if the provided session token matches the user's stored token. | ||
| // Returns true if valid, false otherwise. Also updates last activity time. | ||
| // Returns true if valid, false otherwise. Updates last activity time in a debounced manner | ||
| // to avoid write lock contention that causes backend lockups during rapid requests. | ||
| func (d *DB) ValidateSessionToken(userID, sessionToken string) (bool, error) { | ||
| d.mu.Lock() | ||
| defer d.mu.Unlock() | ||
|
|
||
| // Use read lock for the validation check (most common path) | ||
| d.mu.RLock() | ||
| var storedToken sql.NullString | ||
| err := d.db.QueryRow(` | ||
| SELECT session_token | ||
| FROM users | ||
| SELECT session_token | ||
| FROM users | ||
| WHERE id = ?`, userID).Scan(&storedToken) | ||
|
|
||
| d.mu.RUnlock() | ||
|
|
||
| if err != nil { | ||
| return false, err | ||
| } | ||
|
|
@@ -1233,14 +1238,41 @@ func (d *DB) ValidateSessionToken(userID, sessionToken string) (bool, error) { | |
| return false, nil | ||
| } | ||
|
|
||
| // Token matches - update last activity | ||
| // Debounce session_last_activity updates to avoid write lock contention. | ||
| // Go's sync.RWMutex is write-preferring: a pending Lock() blocks new RLock() callers. | ||
| // Without debouncing, every auth check spawns a goroutine that calls Lock(), which | ||
| // blocks all subsequent RLock() calls (including new auth checks), causing cascading lockups. | ||
| d.updateSessionActivityDebounced(userID) | ||
|
|
||
| return true, nil | ||
| } | ||
|
|
||
| // updateSessionActivityDebounced updates session_last_activity at most once per sessionActivityDebounce | ||
| // interval per user. This uses a separate lightweight mutex (not the main DB mutex) to check | ||
| // timing, and only acquires the main write lock when an actual DB write is needed. | ||
| func (d *DB) updateSessionActivityDebounced(userID string) { | ||
| now := time.Now() | ||
| _, err = d.db.Exec(` | ||
| UPDATE users | ||
| SET session_last_activity = ? | ||
| WHERE id = ?`, now, userID) | ||
|
|
||
| return true, err | ||
|
|
||
| d.sessionActivityMu.Lock() | ||
| lastUpdate, exists := d.sessionActivityTimes[userID] | ||
| if exists && now.Sub(lastUpdate) < sessionActivityDebounce { | ||
| d.sessionActivityMu.Unlock() | ||
| return // Skip — updated recently | ||
| } | ||
| d.sessionActivityTimes[userID] = now | ||
| d.sessionActivityMu.Unlock() | ||
|
Comment on lines
+1256
to
+1263
|
||
|
|
||
| // Perform the actual DB write in a goroutine so it doesn't block the auth response. | ||
| // This is safe because the debounce check above ensures at most one write per user | ||
| // per interval, preventing goroutine pileup. | ||
| go func() { | ||
| d.mu.Lock() | ||
| defer d.mu.Unlock() | ||
| _, _ = d.db.Exec(` | ||
| UPDATE users | ||
| SET session_last_activity = ? | ||
| WHERE id = ?`, now, userID) | ||
| }() | ||
|
sarg3nt marked this conversation as resolved.
|
||
| } | ||
|
|
||
| // ClearUserSessionToken invalidates a user's session token. | ||
|
|
||
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.
The sessionActivityTimes map grows indefinitely and is never cleaned up. Each user ID that ever logs in will have an entry in this map that persists for the lifetime of the application. Over time, this will cause a memory leak, especially in systems with many users or where user IDs change frequently.
Consider implementing periodic cleanup to remove entries that haven't been accessed in a long time (e.g., entries older than sessionActivityDebounce * 2), or remove entries when users log out via ClearUserSessionToken.