Conversation
|
Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details? |
📝 WalkthroughWalkthroughThe changes expand chatbot status matching, make Redis connectivity checks null-safe, alter Redis reconnection behavior, report service-bus health correctly, and bypass session middleware for the current-health endpoint. ChangesChatbot intent matching
Connectivity and health handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Providers/Resgrid.Providers.Cache/AzureRedisCacheProvider.cs (1)
291-323: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRace condition during static connection initialization.
_connectionis a static field, butEstablishRedisConnection()initializes it without thread synchronization. If multiple instances ofAzureRedisCacheProviderare constructed concurrently while_connectionis null, they will all run this method simultaneously and create newConnectionMultiplexerinstances. The later instances will overwrite_connection, leaking the previously created multiplexers (and their background threads/sockets).Please synchronize the initialization using a static lock. Note that to use the fix below, you will also need to add
private static readonly object _connectionLock = new object();at the class level.🔒️ Proposed fix with lock
- private void EstablishRedisConnection() - { - int retry = 0; - - while (_connection == null && retry <= 3) - { - retry++; - - try - { - var options = ConfigurationOptions.Parse(Config.CacheConfig.RedisConnectionString); - options.AbortOnConnectFail = false; - options.ConnectRetry = 1; - options.ConnectTimeout = Math.Min(options.ConnectTimeout, 1000); - options.SyncTimeout = 1000; - options.AsyncTimeout = 1000; - - _connection = ConnectionMultiplexer.Connect(options); - } - catch (Exception ex) - { - _connection = null; - Logging.LogException(ex); - Thread.Sleep(150); - } - } - } + private void EstablishRedisConnection() + { + if (_connection != null) + return; + + lock (_connectionLock) + { + int retry = 0; + + while (_connection == null && retry <= 3) + { + retry++; + + try + { + var options = ConfigurationOptions.Parse(Config.CacheConfig.RedisConnectionString); + options.AbortOnConnectFail = false; + options.ConnectRetry = 1; + options.ConnectTimeout = Math.Min(options.ConnectTimeout, 1000); + options.SyncTimeout = 1000; + options.AsyncTimeout = 1000; + + _connection = ConnectionMultiplexer.Connect(options); + } + catch (Exception ex) + { + _connection = null; + Logging.LogException(ex); + Thread.Sleep(150); + } + } + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Providers/Resgrid.Providers.Cache/AzureRedisCacheProvider.cs` around lines 291 - 323, Synchronize static connection initialization in EstablishRedisConnection by adding a class-level private static readonly _connectionLock object and locking the entire null-check/retry initialization flow. Recheck _connection inside the lock before calling ConnectionMultiplexer.Connect so concurrent AzureRedisCacheProvider constructions reuse the single established connection and do not overwrite it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Providers/Resgrid.Providers.Cache/AzureRedisCacheProvider.cs`:
- Around line 291-323: Synchronize static connection initialization in
EstablishRedisConnection by adding a class-level private static readonly
_connectionLock object and locking the entire null-check/retry initialization
flow. Recheck _connection inside the lock before calling
ConnectionMultiplexer.Connect so concurrent AzureRedisCacheProvider
constructions reuse the single established connection and do not overwrite it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 66708b5a-ae1f-4157-89d6-d8ddde63c8ed
⛔ Files ignored due to path filters (1)
Tests/Resgrid.Tests/Chatbot/ChatbotAvailabilityIntentClassifierTests.csis excluded by!**/Tests/**
📒 Files selected for processing (4)
Core/Resgrid.Chatbot.NLU/Providers/KeywordIntentClassifier.csProviders/Resgrid.Providers.Cache/AzureRedisCacheProvider.csWeb/Resgrid.Web/Controllers/HealthController.csWeb/Resgrid.Web/Startup.cs
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
1 participant
Summary by CodeRabbit