-
Notifications
You must be signed in to change notification settings - Fork 85
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: Implement shedding identity queue #343
Changes from all commits
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 |
---|---|---|
|
@@ -278,15 +278,53 @@ public class LDClient { | |
- parameter context: The LDContext set with the desired context. | ||
- parameter completion: Closure called when the embedded `setOnlineIdentify` call completes, subject to throttling delays. (Optional) | ||
*/ | ||
@available(*, deprecated, message: "Use LDClient.identify(context: completion:) with non-optional completion parameter") | ||
public func identify(context: LDContext, completion: (() -> Void)? = nil) { | ||
let dispatch = DispatchGroup() | ||
LDClient.instances?.forEach { _, instance in | ||
dispatch.enter() | ||
instance.internalIdentify(newContext: context, completion: dispatch.leave) | ||
_identify(context: context, sheddable: false) { _ in | ||
if let completion = completion { | ||
completion() | ||
} | ||
} | ||
if let completion = completion { | ||
dispatch.notify(queue: DispatchQueue.global(), execute: completion) | ||
} | ||
|
||
/** | ||
The LDContext set into the LDClient may affect the set of feature flags returned by the LaunchDarkly server, and ties event tracking to the context. See `LDContext` for details about what information can be retained. | ||
Normally, the client app should create and set the LDContext and pass that into `start(config: context: completion:)`. | ||
The client app can change the active `context` by calling identify with a new or updated LDContext. Client apps should follow [Apple's Privacy Policy](apple.com/legal/privacy) when collecting user information. | ||
When a new context is set, the LDClient goes offline and sets the new context. If the client was online when the new context was set, it goes online again, subject to a throttling delay if in force (see `setOnline(_: completion:)` for details). A completion may be passed to the identify method to allow a client app to know when fresh flag values for the new context are ready. | ||
While only a single identify request can be active at a time, consumers of this SDK can call this method multiple times. To prevent unnecessary network traffic, these requests are placed | ||
into a sheddable queue. Identify requests will be shed if 1) an existing identify request is in flight, and 2) a third identify has been requested which can be replace the one being shed. | ||
- parameter context: The LDContext set with the desired context. | ||
- parameter completion: Closure called when the embedded `setOnlineIdentify` call completes, subject to throttling delays. | ||
*/ | ||
public func identify(context: LDContext, completion: @escaping (_ result: IdentifyResult) -> Void) { | ||
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. The other identify method has an optional completion method. This one isn't optional. If I continue with the completion method being optional, then the method isn't fully disambiguated. Customers could have code like: Given that these identify requests might never actually complete, it seems kind of reasonable to force customers listen for those results, just as a measure of good practice for how to use this SDK. What do you think? If we decide to make it optional, how do we deal with the backwards compatibility naming convention? Keep in mind that if we name it something like We could consider making it non-nullable in this. version and loosen that restriction in the next. Or we could wait to see what customer feedback is and add the nullability back at that time. 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. I am interested to see what customer feedback is. It doesn't seem unreasonable at this point to require it. 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. They can always pass a no-op and ignore it. |
||
_identify(context: context, sheddable: true, completion: completion) | ||
} | ||
|
||
// Temporary helper method to allow code sharing between the sheddable and unsheddable identify methods. In the next major release, we will remove the deprecated identify method and inline | ||
// this implementation in the other one. | ||
private func _identify(context: LDContext, sheddable: Bool, completion: @escaping (_ result: IdentifyResult) -> Void) { | ||
let work: TaskHandler = { taskCompletion in | ||
let dispatch = DispatchGroup() | ||
|
||
LDClient.instances?.forEach { _, instance in | ||
dispatch.enter() | ||
instance.internalIdentify(newContext: context, completion: dispatch.leave) | ||
} | ||
|
||
dispatch.notify(queue: DispatchQueue.global(), execute: taskCompletion) | ||
} | ||
|
||
let identifyTask = Task(work: work, sheddable: sheddable) { [self] result in | ||
os_log("%s identity completion with result %s", log: config.logger, type: .debug, typeName(and: #function), String(describing: result)) | ||
completion(IdentifyResult(from: result)) | ||
} | ||
identifyQueue.enqueue(request: identifyTask) | ||
} | ||
|
||
func internalIdentify(newContext: LDContext, completion: (() -> Void)? = nil) { | ||
|
@@ -711,6 +749,7 @@ public class LDClient { | |
} | ||
private var _initialized = false | ||
private var initializedQueue = DispatchQueue(label: "com.launchdarkly.LDClient.initializedQueue") | ||
private var identifyQueue = SheddingQueue() | ||
|
||
private init(serviceFactory: ClientServiceCreating, configuration: LDConfig, startContext: LDContext?, completion: (() -> Void)? = nil) { | ||
self.serviceFactory = serviceFactory | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
import Foundation | ||
|
||
/** | ||
Denotes the result of an identify request made through the `LDClient.identify(context: completion:)` method. | ||
*/ | ||
public enum IdentifyResult { | ||
/** | ||
The identify request has completed successfully. | ||
*/ | ||
case complete | ||
/** | ||
The identify request has received an unrecoverable failure. | ||
*/ | ||
case error | ||
/** | ||
The identify request has been replaced with a subsequent request. See `LDClient.identify(context: completion:)` for more details. | ||
*/ | ||
case shed | ||
|
||
init(from: TaskResult) { | ||
switch from { | ||
case .complete: | ||
self = .complete | ||
case .error: | ||
self = .error | ||
case .shed: | ||
self = .shed | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
import Foundation | ||
|
||
enum TaskResult { | ||
case complete | ||
case error | ||
case shed | ||
} | ||
|
||
typealias TaskHandlerCompletion = () -> Void | ||
typealias TaskHandler = (_ completion: @escaping TaskHandlerCompletion) -> Void | ||
typealias TaskCompletion = (_ result: TaskResult) -> Void | ||
|
||
struct Task { | ||
let work: TaskHandler | ||
let sheddable: Bool | ||
let completion: TaskCompletion | ||
} | ||
|
||
class SheddingQueue { | ||
private let stateQueue: DispatchQueue = DispatchQueue(label: "StateQueue") | ||
private let identifyQueue: DispatchQueue = DispatchQueue(label: "IdentifyQueue") | ||
|
||
private var inFlight: Task? | ||
private var queue: [Task] = [] | ||
|
||
func enqueue(request: Task) { | ||
stateQueue.async { [self] in | ||
guard inFlight != nil else { | ||
inFlight = request | ||
identifyQueue.async { self.execute() } | ||
return | ||
} | ||
|
||
if let lastTask = queue.last, lastTask.sheddable { | ||
queue.removeLast() | ||
lastTask.completion(.shed) | ||
} | ||
|
||
queue.append(request) | ||
} | ||
} | ||
|
||
private func execute() { | ||
var nextTask: Task? | ||
|
||
stateQueue.sync { | ||
nextTask = inFlight | ||
} | ||
|
||
if nextTask == nil { | ||
return | ||
} | ||
|
||
guard let request = nextTask else { return } | ||
|
||
request.work() { [self] in | ||
request.completion(.complete) | ||
|
||
stateQueue.sync { | ||
inFlight = queue.first | ||
if inFlight != nil { | ||
queue.remove(at: 0) | ||
identifyQueue.async { self.execute() } | ||
} | ||
} | ||
} | ||
} | ||
} |
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.
Deprecating this method so that customers will start moving over to the callback that informs them of success or failure.