From 6c214b9723f411097ed7b83e9d6987f7abaf6622 Mon Sep 17 00:00:00 2001 From: Jamie Lynch Date: Wed, 4 Dec 2024 17:22:43 +0000 Subject: [PATCH] simplify delivery loop logic --- .../delivery/debug/DeliveryTraceState.kt | 4 +-- .../internal/delivery/debug/DeliveryTracer.kt | 4 +-- .../scheduling/SchedulingServiceImpl.kt | 29 +++++-------------- .../scheduling/SchedulingServiceImplTest.kt | 2 +- 4 files changed, 13 insertions(+), 26 deletions(-) diff --git a/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/debug/DeliveryTraceState.kt b/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/debug/DeliveryTraceState.kt index 5f379a0eef..241230b5d5 100644 --- a/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/debug/DeliveryTraceState.kt +++ b/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/debug/DeliveryTraceState.kt @@ -97,8 +97,8 @@ internal sealed class DeliveryTraceState { /** * The delivery loop was started */ - internal class StartDeliveryLoop(private val loopAlreadyActive: Boolean) : DeliveryTraceState() { - override fun toString(): String = "StartDeliveryLoop loopAlreadyActive=$loopAlreadyActive" + internal object StartDeliveryLoop : DeliveryTraceState() { + override fun toString(): String = "StartDeliveryLoop" } /** diff --git a/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/debug/DeliveryTracer.kt b/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/debug/DeliveryTracer.kt index 0e1d9c73be..0ea25fe7f9 100644 --- a/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/debug/DeliveryTracer.kt +++ b/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/debug/DeliveryTracer.kt @@ -60,8 +60,8 @@ class DeliveryTracer { }.joinToString("\n") } - fun onStartDeliveryLoop(sendLoopActive: Boolean) { - events.add(DeliveryTraceState.StartDeliveryLoop(sendLoopActive)) + fun onStartDeliveryLoop() { + events.add(DeliveryTraceState.StartDeliveryLoop) } fun onPayloadQueueCreated( diff --git a/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/scheduling/SchedulingServiceImpl.kt b/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/scheduling/SchedulingServiceImpl.kt index d66eaa71d4..b113dc5f8f 100644 --- a/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/scheduling/SchedulingServiceImpl.kt +++ b/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/scheduling/SchedulingServiceImpl.kt @@ -14,7 +14,6 @@ import io.embrace.android.embracesdk.internal.logging.InternalErrorType import io.embrace.android.embracesdk.internal.worker.BackgroundWorker import java.io.InputStream import java.util.Collections -import java.util.LinkedList import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.Future import java.util.concurrent.TimeUnit @@ -32,14 +31,11 @@ class SchedulingServiceImpl( private val blockedEndpoints: MutableMap = ConcurrentHashMap() private val hasNetwork = AtomicBoolean(true) - private val sendLoopActive = AtomicBoolean(false) - private val queryForPayloads = AtomicBoolean(true) private val activeSends: MutableSet = Collections.newSetFromMap(ConcurrentHashMap()) private val deleteInProgress: MutableSet = Collections.newSetFromMap(ConcurrentHashMap()) private val payloadsToRetry: MutableMap = ConcurrentHashMap() override fun onPayloadIntake() { - queryForPayloads.set(true) startDeliveryLoop() } @@ -66,13 +62,9 @@ class SchedulingServiceImpl( private fun startDeliveryLoop() { // When a payload arrives, check to see if there's already an active job try to deliver payloads // If not, schedule job. If so, do nothing. - if (sendLoopActive.compareAndSet(false, true)) { - deliveryTracer?.onStartDeliveryLoop(true) - schedulingWorker.submit { - deliveryLoop() - } - } else { - deliveryTracer?.onStartDeliveryLoop(false) + deliveryTracer?.onStartDeliveryLoop() + schedulingWorker.submit { + deliveryLoop() } } @@ -82,9 +74,8 @@ class SchedulingServiceImpl( private fun deliveryLoop() { val failedPayloads = mutableSetOf() try { - var deliveryQueue = createPayloadQueue() - while (deliveryQueue.isNotEmpty() && readyToSend()) { - val payload = deliveryQueue.poll() + var payload: StoredTelemetryMetadata? = findNextPayload() + while (payload != null && readyToSend()) { runCatching { payload?.run { if (shouldSendPayload() && readyToSend()) { @@ -109,10 +100,7 @@ class SchedulingServiceImpl( throwable = IllegalStateException("Failed to queue payload with file name $fileName", error) ) } - - if (queryForPayloads.compareAndSet(true, false) || deliveryQueue.isEmpty()) { - deliveryQueue = createPayloadQueue(failedPayloads) - } + payload = findNextPayload(failedPayloads) } } catch (t: Throwable) { // This block catches unhandled errors resulting from the recreation of a queue of payloads to be delivered @@ -120,12 +108,11 @@ class SchedulingServiceImpl( // to retry any pending payloads. logger.trackInternalError(InternalErrorType.DELIVERY_SCHEDULING_FAIL, t) } finally { - sendLoopActive.set(false) scheduleDeliveryLoopForNextRetry() } } - private fun createPayloadQueue(exclude: Set = emptySet()): LinkedList { + private fun findNextPayload(exclude: Set = emptySet()): StoredTelemetryMetadata? { val payloadsByPriority = storageService.getPayloadsByPriority() val payloadsToSend = payloadsByPriority .filter { it.shouldSendPayload() && !exclude.contains(it) } @@ -134,7 +121,7 @@ class SchedulingServiceImpl( payloadsByPriority, payloadsToSend, ) - return LinkedList(payloadsToSend) + return payloadsToSend.firstOrNull() } private fun queueDelivery(payload: StoredTelemetryMetadata): Future { diff --git a/embrace-android-delivery/src/test/kotlin/io/embrace/android/embracesdk/internal/delivery/scheduling/SchedulingServiceImplTest.kt b/embrace-android-delivery/src/test/kotlin/io/embrace/android/embracesdk/internal/delivery/scheduling/SchedulingServiceImplTest.kt index 6ad6e3d01c..3733ed6dc0 100644 --- a/embrace-android-delivery/src/test/kotlin/io/embrace/android/embracesdk/internal/delivery/scheduling/SchedulingServiceImplTest.kt +++ b/embrace-android-delivery/src/test/kotlin/io/embrace/android/embracesdk/internal/delivery/scheduling/SchedulingServiceImplTest.kt @@ -73,7 +73,7 @@ internal class SchedulingServiceImplTest { schedulingExecutor.blockingMode = true schedulingService.onPayloadIntake() schedulingService.onPayloadIntake() - assertEquals(1, schedulingExecutor.submitCount) + assertEquals(2, schedulingExecutor.submitCount) } @Test