Skip to content
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

RUMM-709 Upload delay increases in case of network error #249

Merged
merged 2 commits into from
Sep 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Datadog/Datadog.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
61133C5B2423990D00786299 /* DirectoryTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61133C2D2423990D00786299 /* DirectoryTests.swift */; };
61133C5C2423990D00786299 /* DataUploadWorkerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61133C2F2423990D00786299 /* DataUploadWorkerTests.swift */; };
61133C5D2423990D00786299 /* DataUploadConditionsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61133C302423990D00786299 /* DataUploadConditionsTests.swift */; };
61133C5E2423990D00786299 /* LogsUploadDelayTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61133C312423990D00786299 /* LogsUploadDelayTests.swift */; };
61133C5E2423990D00786299 /* DataUploadDelayTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61133C312423990D00786299 /* DataUploadDelayTests.swift */; };
61133C5F2423990D00786299 /* DataUploaderTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61133C322423990D00786299 /* DataUploaderTests.swift */; };
61133C602423990D00786299 /* HTTPHeadersTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61133C332423990D00786299 /* HTTPHeadersTests.swift */; };
61133C612423990D00786299 /* HTTPClientTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61133C342423990D00786299 /* HTTPClientTests.swift */; };
Expand Down Expand Up @@ -384,7 +384,7 @@
61133C2D2423990D00786299 /* DirectoryTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DirectoryTests.swift; sourceTree = "<group>"; };
61133C2F2423990D00786299 /* DataUploadWorkerTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DataUploadWorkerTests.swift; sourceTree = "<group>"; };
61133C302423990D00786299 /* DataUploadConditionsTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DataUploadConditionsTests.swift; sourceTree = "<group>"; };
61133C312423990D00786299 /* LogsUploadDelayTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = LogsUploadDelayTests.swift; sourceTree = "<group>"; };
61133C312423990D00786299 /* DataUploadDelayTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DataUploadDelayTests.swift; sourceTree = "<group>"; };
61133C322423990D00786299 /* DataUploaderTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DataUploaderTests.swift; sourceTree = "<group>"; };
61133C332423990D00786299 /* HTTPHeadersTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = HTTPHeadersTests.swift; sourceTree = "<group>"; };
61133C342423990D00786299 /* HTTPClientTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = HTTPClientTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -993,7 +993,7 @@
children = (
61133C2F2423990D00786299 /* DataUploadWorkerTests.swift */,
61133C302423990D00786299 /* DataUploadConditionsTests.swift */,
61133C312423990D00786299 /* LogsUploadDelayTests.swift */,
61133C312423990D00786299 /* DataUploadDelayTests.swift */,
61133C322423990D00786299 /* DataUploaderTests.swift */,
61133C332423990D00786299 /* HTTPHeadersTests.swift */,
61133C342423990D00786299 /* HTTPClientTests.swift */,
Expand Down Expand Up @@ -2068,7 +2068,7 @@
61BBD19724ED50040023E65F /* FeaturesConfigurationTests.swift in Sources */,
61133C612423990D00786299 /* HTTPClientTests.swift in Sources */,
61133C6A2423990D00786299 /* DatadogTests.swift in Sources */,
61133C5E2423990D00786299 /* LogsUploadDelayTests.swift in Sources */,
61133C5E2423990D00786299 /* DataUploadDelayTests.swift in Sources */,
61133C5C2423990D00786299 /* DataUploadWorkerTests.swift in Sources */,
61E909F624A32D1C005EA2DE /* GlobalTests.swift in Sources */,
9E58E8E324615EDA008E5063 /* JSONEncoderTests.swift in Sources */,
Expand Down
12 changes: 6 additions & 6 deletions Sources/Datadog/Core/PerformancePreset.swift
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ internal protocol UploadPerformancePreset {
var minUploadDelay: TimeInterval { get }
/// Maximum interval of data upload (in seconds).
var maxUploadDelay: TimeInterval { get }
/// If upload succeeds, current interval is multiplied by this factor.
/// Should be less or equal `1.0`.
var uploadDelayDecreaseFactor: Double { get }
/// If upload succeeds or fails, current interval is changed by this rate. Should be less or equal `1.0`.
/// E.g: if rate is `0.1` then `delay` can be increased or decreased by `delay * 0.1`.
var uploadDelayChangeRate: Double { get }
}

internal struct PerformancePreset: Equatable, StoragePerformancePreset, UploadPerformancePreset {
Expand All @@ -67,7 +67,7 @@ internal struct PerformancePreset: Equatable, StoragePerformancePreset, UploadPe
let defaultUploadDelay: TimeInterval
let minUploadDelay: TimeInterval
let maxUploadDelay: TimeInterval
let uploadDelayDecreaseFactor: Double
let uploadDelayChangeRate: Double

// MARK: - Predefined presets

Expand All @@ -91,7 +91,7 @@ internal struct PerformancePreset: Equatable, StoragePerformancePreset, UploadPe
defaultUploadDelay: 5,
minUploadDelay: 1,
maxUploadDelay: 20,
uploadDelayDecreaseFactor: 0.9
uploadDelayChangeRate: 0.1
)

/// Performance preset optimized for instant data delivery.
Expand All @@ -111,7 +111,7 @@ internal struct PerformancePreset: Equatable, StoragePerformancePreset, UploadPe
defaultUploadDelay: 3,
minUploadDelay: 1,
maxUploadDelay: 5,
uploadDelayDecreaseFactor: 0.5 // reduce significantly for more uploads in short-lived app extensions
uploadDelayChangeRate: 0.5 // reduce significantly for more uploads in short-lived app extensions
)

static func best(for bundleType: BundleType) -> PerformancePreset {
Expand Down
27 changes: 13 additions & 14 deletions Sources/Datadog/Core/Upload/DataUploadDelay.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,37 +6,36 @@

import Foundation

internal protocol Delay {
var current: TimeInterval { get }
mutating func decrease()
mutating func increase()
}

/// Mutable interval used for periodic data uploads.
internal struct DataUploadDelay {
internal struct DataUploadDelay: Delay {
private let defaultDelay: TimeInterval
private let minDelay: TimeInterval
private let maxDelay: TimeInterval
private let decreaseFactor: Double
private let changeRate: Double

private var delay: TimeInterval

init(performance: UploadPerformancePreset) {
self.defaultDelay = performance.defaultUploadDelay
self.minDelay = performance.minUploadDelay
self.maxDelay = performance.maxUploadDelay
self.decreaseFactor = performance.uploadDelayDecreaseFactor
self.changeRate = performance.uploadDelayChangeRate
self.delay = performance.initialUploadDelay
}

mutating func nextUploadDelay() -> TimeInterval {
defer {
if delay == maxDelay {
delay = defaultDelay
}
}
return delay
}
var current: TimeInterval { delay }

mutating func decrease() {
delay = max(minDelay, delay * decreaseFactor)
delay = max(minDelay, delay * (1.0 - changeRate))
}

mutating func increaseOnce() {
delay = maxDelay
mutating func increase() {
delay = min(delay * (1.0 + changeRate), maxDelay)
}
}
16 changes: 9 additions & 7 deletions Sources/Datadog/Core/Upload/DataUploadWorker.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@ internal class DataUploadWorker: DataUploadWorkerType {
private let featureName: String

/// Delay used to schedule consecutive uploads.
private var delay: DataUploadDelay
private var delay: Delay

init(
queue: DispatchQueue,
fileReader: FileReaderType,
dataUploader: DataUploader,
uploadConditions: DataUploadConditions,
delay: DataUploadDelay,
delay: Delay,
featureName: String
) {
self.queue = queue
Expand All @@ -44,7 +44,7 @@ internal class DataUploadWorker: DataUploadWorkerType {
self.delay = delay
self.featureName = featureName

scheduleNextUpload(after: self.delay.nextUploadDelay())
scheduleNextUpload(after: self.delay.current)
}

private func scheduleNextUpload(after delay: TimeInterval) {
Expand All @@ -67,24 +67,26 @@ internal class DataUploadWorker: DataUploadWorkerType {

if shouldBeAccepted {
self.fileReader.markBatchAsRead(batch)
self.delay.decrease()

developerLogger?.info(" → (\(self.featureName)) accepted, won't be retransmitted: \(uploadStatus)")
userLogger.debug(" → (\(self.featureName)) accepted, won't be retransmitted: \(uploadStatus)")
} else {
self.delay.increase()

developerLogger?.info(" → (\(self.featureName)) not delivered, will be retransmitted: \(uploadStatus)")
userLogger.debug(" → (\(self.featureName)) not delivered, will be retransmitted: \(uploadStatus)")
}

self.delay.decrease()
} else {
let batchLabel = nextBatch != nil ? "YES" : (isSystemReady ? "NO" : "NOT CHECKED")
let systemLabel = isSystemReady ? "✅" : "❌"
developerLogger?.info("💡 (\(self.featureName)) No upload. Batch to upload: \(batchLabel), System conditions: \(systemLabel)")
userLogger.debug("💡 (\(self.featureName)) No upload. Batch to upload: \(batchLabel), System conditions: \(systemLabel)")

self.delay.increaseOnce()
self.delay.increase()
buranmert marked this conversation as resolved.
Show resolved Hide resolved
}

self.scheduleNextUpload(after: self.delay.nextUploadDelay())
self.scheduleNextUpload(after: self.delay.current)
}
}
}
4 changes: 4 additions & 0 deletions Sources/Datadog/Core/Upload/DataUploader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ internal enum DataUploadStatus: Equatable, Hashable {
case success
/// Corresponds to HTTP 3xx response status codes.
case redirection
/// Corresponds to HTTP 403 response status codes,
/// which means client token is invalid
case clientTokenError
/// Corresponds to HTTP 4xx response status codes.
case clientError
/// Corresponds to HTTP 5xx response status codes.
Expand All @@ -121,6 +124,7 @@ internal enum DataUploadStatus: Equatable, Hashable {
switch httpResponse.statusCode {
case 200...299: self = .success
case 300...399: self = .redirection
case 403: self = .clientTokenError
case 400...499: self = .clientError
case 500...599: self = .serverError
default: self = .unknown
Expand Down
2 changes: 1 addition & 1 deletion Sources/Datadog/Core/Upload/HTTPClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ internal final class HTTPClient {
/// The code execution in `URLSessionTransport` should never reach its initialization.
internal struct URLSessionTransportInconsistencyException: Error {}

/// As `URLSession` returns 3-values-touple for request execution, this function applies consistency constraints and turns
/// As `URLSession` returns 3-values-tuple for request execution, this function applies consistency constraints and turns
/// it into only two possible states of `HTTPTransportResult`.
private func httpClientResult(for urlSessionTaskCompletion: (Data?, URLResponse?, Error?)) -> Result<HTTPURLResponse, Error> {
let (_, response, error) = urlSessionTaskCompletion
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,14 @@ class PerformancePresetTests: XCTestCase {
"Upload delay boundaries must be consistent."
)
XCTAssertLessThanOrEqual(
preset.uploadDelayDecreaseFactor,
preset.uploadDelayChangeRate,
1,
"Upload delay should not be increased towards infinity."
"Upload delay should not change by more than %100 at once."
)
XCTAssertGreaterThan(
preset.uploadDelayDecreaseFactor,
preset.uploadDelayChangeRate,
0,
"Upload delay must never result with 0."
"Upload delay must change at non-zero rate."
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,26 +13,26 @@ class DataUploadDelayTests: XCTestCase {
defaultUploadDelay: 5,
minUploadDelay: 1,
maxUploadDelay: 20,
uploadDelayDecreaseFactor: 0.9
uploadDelayChangeRate: 0.1
)

func testWhenNotModified_itReturnsInitialDelay() {
var delay = DataUploadDelay(performance: mockPerformance)
XCTAssertEqual(delay.nextUploadDelay(), mockPerformance.initialUploadDelay)
XCTAssertEqual(delay.nextUploadDelay(), mockPerformance.initialUploadDelay)
let delay = DataUploadDelay(performance: mockPerformance)
XCTAssertEqual(delay.current, mockPerformance.initialUploadDelay)
XCTAssertEqual(delay.current, mockPerformance.initialUploadDelay)
}

func testWhenDecreasing_itGoesDownToMinimumDelay() {
var delay = DataUploadDelay(performance: mockPerformance)
var previousValue: TimeInterval = delay.nextUploadDelay()
var previousValue: TimeInterval = delay.current

while previousValue != mockPerformance.minUploadDelay {
while previousValue > mockPerformance.minUploadDelay {
delay.decrease()

let nextValue = delay.nextUploadDelay()
let nextValue = delay.current
XCTAssertEqual(
nextValue / previousValue,
mockPerformance.uploadDelayDecreaseFactor,
1.0 - mockPerformance.uploadDelayChangeRate,
accuracy: 0.1
)
XCTAssertLessThanOrEqual(nextValue, max(previousValue, mockPerformance.minUploadDelay))
Expand All @@ -41,13 +41,21 @@ class DataUploadDelayTests: XCTestCase {
}
}

func testWhenIncreasedOnce_itReturnsMaximumDelayOnceThenGoesBackToDefaultDelay() {
func testWhenIncreasing_itClampsToMaximumDelay() {
var delay = DataUploadDelay(performance: mockPerformance)
delay.decrease()
delay.increaseOnce()
var previousValue: TimeInterval = delay.current

while previousValue < mockPerformance.maxUploadDelay {
delay.increase()

XCTAssertEqual(delay.nextUploadDelay(), mockPerformance.maxUploadDelay)
XCTAssertEqual(delay.nextUploadDelay(), mockPerformance.defaultUploadDelay)
XCTAssertEqual(delay.nextUploadDelay(), mockPerformance.defaultUploadDelay)
let nextValue = delay.current
XCTAssertEqual(
nextValue / previousValue,
1.0 + mockPerformance.uploadDelayChangeRate,
accuracy: 0.1
)
XCTAssertGreaterThanOrEqual(nextValue, min(previousValue, mockPerformance.maxUploadDelay))
previousValue = nextValue
}
}
}
Loading