Verified Commit 939e41b8 authored by Nathan Harris's avatar Nathan Harris
Browse files

Fix data race with `RedisMetrics.activeConnectionCount`

Motivation:

After enabling the thread sanitizer during testing, a data race with the `activeConnectionCount` in `RedisMetrics` was caught due to changing a primitive `Int` across threads.

Modifications:

Add a specialized class `ActiveConnectionGauge` to wrap the semantics of the `activeConnectionCount` with an `Atomic<Int>` property.

Result:

No data races should occur with the `RedisMetrics` subsystem.
parent dfca5704
Pipeline #83962610 passed with stage
in 2 minutes and 51 seconds
......@@ -148,7 +148,7 @@ public final class RedisConnection: RedisClient {
self.logger[metadataKey: loggingKeyID] = "\(UUID())"
self._state = .open
self.logger.debug("Connection created.")
RedisMetrics.activeConnectionCount += 1
RedisMetrics.activeConnectionCount.increment()
RedisMetrics.totalConnectionCount.increment()
// attach a callback to the channel to capture situations where the channel might be closed out from under
......@@ -160,7 +160,7 @@ public final class RedisConnection: RedisClient {
self.state = .closed
self.logger.warning("Channel was closed unexpectedly.")
RedisMetrics.activeConnectionCount -= 1
RedisMetrics.activeConnectionCount.decrement()
}
}
......@@ -243,7 +243,7 @@ extension RedisConnection {
notification.whenSuccess {
self.state = .closed
self.logger.debug("Connection closed.")
RedisMetrics.activeConnectionCount -= 1
RedisMetrics.activeConnectionCount.decrement()
}
return notification
......
......@@ -13,6 +13,7 @@
//===----------------------------------------------------------------------===//
import Metrics
import NIOConcurrencyHelpers
/// The system funnel for all `Metrics` interactions from the Redis library.
///
......@@ -38,14 +39,8 @@ public struct RedisMetrics {
}
}
private static let activeConnectionCountGauge = Gauge(label: .activeConnectionCount)
/// The current number of connections this library has active.
/// - Note: Changing this number will update the `Metrics.Gauge` stored for recording the new value.
public static var activeConnectionCount: Int = 0 {
didSet {
activeConnectionCountGauge.record(activeConnectionCount)
}
}
/// The wrapped `Metrics.Gauge` maintaining the current number of connections this library has active.
public static var activeConnectionCount = ActiveConnectionGauge()
/// The `Metrics.Counter` that retains the number of connections made since application startup.
public static let totalConnectionCount = Counter(label: .totalConnectionCount)
/// The `Metrics.Counter` that retains the number of commands that successfully returned from Redis
......@@ -61,6 +56,31 @@ public struct RedisMetrics {
private init() { }
}
/// A specialized wrapper class for working with `Metrics.Gauge` objects for the purpose of an incrementing or decrementing count of active Redis connections.
public class ActiveConnectionGauge {
private let gauge = Gauge(label: .activeConnectionCount)
private let count = Atomic<Int>(value: 0)
/// The number of the connections that are currently reported as active.
var currentCount: Int { return count.load() }
internal init() { }
/// Increments the current count by the amount specified.
/// - Parameter amount: The number to increase the current count by. Default is `1`.
public func increment(by amount: Int = 1) {
_ = self.count.add(amount)
self.gauge.record(self.count.load())
}
/// Decrements the current count by the amount specified.
/// - Parameter amount: The number to decrease the current count by. Default is `1`.
public func decrement(by amount: Int = 1) {
_ = self.count.sub(amount)
self.gauge.record(self.count.load())
}
}
extension Metrics.Counter {
@inline(__always)
convenience init(label: RedisMetrics.Label) {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment