refactor(flags): use time-based periodic flush instead of cancel+reschedule by typotter · Pull Request #3189 · DataDog/dd-sdk-android

typotter · 2026-02-19T06:57:36Z

What does this PR do?

Replaces the cancel and reschedule pattern for periodic evaluation flushes with a more efficient time-tracking approach. The scheduled task now checks if enough time has elapsed since the last flush before executing.

Motivation

The previous implementation would cancel and reschedule the periodic flush task every time a flush occurred (manual or automatic). This created unnecessary overhead from repeatedly creating and cancelling scheduled tasks. The new approach uses a single scheduled task that checks a lastFlushTimeMs timestamp to determine if a flush is needed.

Additional Notes

Trade-offs:
This configuration guarantees that the age of the in-memory cache will be between flushIntervalMs and 2 * flushIntervalMs in exchange for much simpler handling logic.

Key changes:

Added lastFlushTimeMs volatile field to track when events were last written
periodicFlushTask() checks now - lastFlushTimeMs >= flushIntervalMs before calling flush()
Made startPeriodicFlush() private and idempotent (only starts if not already running)
Removed unnecessary reschedulePeriodicFlush() call from EvaluationsFeature
Removed unnecessary periodicFlushEnabled property

Review checklist (to be filled by reviewers)

Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
Make sure you discussed the feature or bugfix with the maintaining team in an Issue
Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

…hedule Replace the cancel and reschedule pattern for periodic evaluation flushes with a more efficient time-tracking approach. The scheduled task now runs at a fixed rate and checks if enough time has elapsed since the last flush before executing. Changes: - Add lastFlushTimeMs to track when events were last written - Use scheduleAtFixedRate() instead of repeated schedule() calls - periodicFlushTask() checks time elapsed before calling flush() - Remove unnecessary reschedulePeriodicFlush() call from EvaluationsFeature

codecov-commenter · 2026-02-20T17:17:11Z

Codecov Report

❌ Patch coverage is 95.23810% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 71.42%. Comparing base (699dce4) to head (6441b5d).
⚠️ Report is 248 commits behind head on develop.

Files with missing lines	Patch %	Lines
...ndroid/flags/internal/EvaluationEventsProcessor.kt	95.24%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3189      +/-   ##
===========================================
+ Coverage    71.26%   71.42%   +0.17%     
===========================================
  Files          929      929              
  Lines        34442    34447       +5     
  Branches      5813     5810       -3     
===========================================
+ Hits         24542    24603      +61     
+ Misses        8255     8229      -26     
+ Partials      1645     1615      -30

Files with missing lines	Coverage Δ
...tadog/android/flags/internal/EvaluationsFeature.kt	`21.28% <ø> (+0.44%)`	⬆️
...ndroid/flags/internal/EvaluationEventsProcessor.kt	`98.33% <95.24%> (+3.17%)`	⬆️

... and 41 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

0xnm · 2026-02-23T07:45:11Z

-                reschedulePeriodicFlush()
-            }
-        }
+    private var lastFlushTimeMs: Long = 0L


nit: probably it is worth to start not from 0, but from the class creation time, so System.currentTimeMillis

added to init since we're using TimeProvider, we should be using it for the initial timestamp if not 0.

this can be even moved out of init

Suggested change

private var lastFlushTimeMs: Long = 0L

private var lastFlushTimeMs: Long = timeProvider.getDeviceTimestampMillis()

0xnm · 2026-02-23T07:51:28Z

        // Wait for any in-progress flush to complete, then drain any remaining events.
-        @Suppress("UnsafeThirdPartyFunctionCall") // safe - ReentrantLock.lock() does not throw
        val events = flushMutex.withLock {
            scheduledFlushFuture?.cancel(false)


There is no need to cancel it, because the executor is shut down above anyway. This will remove the need to store the reference to scheduledFlushFuture at all (probably). Runnable passed in the scheduleWithFixedDelay can check if it should schedule itself once again after periodicFlushTask is executed and do it if periodicFlushEnabled is enabled.

Great simplification. The shutdown will allow the running task to complete and will also prevent the scheduleExecutor from re-scheduling the task so we don't need to even manage the rescheduling.

0xnm · 2026-02-23T07:54:13Z

+                        flushIntervalMs,
+                        flushIntervalMs,


Should it be the same value? The first one is for how long to wait until the task get executed, the second is how long to wait until the next one should be scheduled for execution. But we always have only single task anyway if I'm not wrong.

Good question. I don't think it makes sense to immediately start a flush since this is is called on startup, so waiting 1 interval makes sense there. The executor waits for delayms after the task executes, so it is automatically rescheduled and as such, there would indeed only ever be one task scheduled at a time.

typotter

Thanks. ptal

0xnm · 2026-02-23T17:37:52Z

-                reschedulePeriodicFlush()
-            }
-        }
+    private var lastFlushTimeMs: Long = 0L


this can be even moved out of init

Suggested change

private var lastFlushTimeMs: Long = 0L

private var lastFlushTimeMs: Long = timeProvider.getDeviceTimestampMillis()

0xnm · 2026-02-23T17:40:27Z

+    private var periodicFlushScheduled: Boolean = false
+


Suggested change

private var periodicFlushScheduled: Boolean = false

This was extra-defensive since the class, as written, will only ever call startPeriodicFlush once from init.

0xnm · 2026-02-23T17:40:50Z

@@ -31,19 +30,18 @@ internal class EvaluationEventsProcessor(
    periodicFlushEnabled: Boolean = true


Suggested change

periodicFlushEnabled: Boolean = true

@Volatile

private var periodicFlushEnabled: Boolean = false

We aren't storing this value - just read it once at construction and start the period flush using a mechanism that reschedules the task after each completion.

0xnm · 2026-02-23T17:49:01Z

-        // Cancel any scheduled before scheduling a new one.
-        @Suppress("UnsafeThirdPartyFunctionCall") // safe - ReentrantLock.lock() does not throw
+    private fun startPeriodicFlush() {
        flushMutex.withLock {


I think flushMutex is not needed here, because what this function does is only scheduling, it doesn't do the actual flush.

right. now that we don't need to protect the scheduledFuture with the lock we can drop it here.

typotter added 7 commits February 18, 2026 23:57

Merge branch 'develop' into typo/no-cancel-flush-schedule

edfd4a9

remove unnecessary annoation

96103bb

clean up periodic flush bool

3fac648

fix flush timestamp order

25b4dc2

use regular schedule instead of fixedRate

3467620

trim fixed rate test

0850e95

typotter marked this pull request as ready for review February 20, 2026 16:33

typotter requested a review from a team as a code owner February 20, 2026 16:33

This comment has been minimized.

Sign in to view

0xnm reviewed Feb 23, 2026

View reviewed changes

no need to track/cancel flush future

04aac97

typotter commented Feb 23, 2026

View reviewed changes

typotter requested a review from 0xnm February 23, 2026 17:40

0xnm reviewed Feb 23, 2026

View reviewed changes

typotter added 2 commits February 23, 2026 11:17

init last flush time and drop lock

a58b1f3

drop unneeded started check

6441b5d

typotter requested a review from 0xnm February 23, 2026 18:21

0xnm approved these changes Feb 23, 2026

View reviewed changes

gh-worker-dd-devflow-36fce6 Bot added mergequeue-status: queued mergequeue-status: in_progress and removed mergequeue-status: queued labels Feb 23, 2026

gh-worker-dd-mergequeue-cf854d Bot merged commit 63dfac8 into develop Feb 23, 2026
27 checks passed

gh-worker-dd-devflow-36fce6 Bot removed the mergequeue-status: in_progress label Feb 23, 2026

gh-worker-dd-mergequeue-cf854d Bot deleted the typo/no-cancel-flush-schedule branch February 23, 2026 21:02

gh-worker-dd-devflow-36fce6 Bot added the mergequeue-status: done label Feb 23, 2026

ncreated restored the typo/no-cancel-flush-schedule branch April 9, 2026 09:51

	private var lastFlushTimeMs: Long = 0L
	private var lastFlushTimeMs: Long = timeProvider.getDeviceTimestampMillis()

		@@ -31,19 +30,18 @@ internal class EvaluationEventsProcessor(
		periodicFlushEnabled: Boolean = true

	periodicFlushEnabled: Boolean = true
	@Volatile
	private var periodicFlushEnabled: Boolean = false

Uh oh!

Conversation

typotter commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Motivation

Additional Notes

Review checklist (to be filled by reviewers)

Uh oh!

This comment has been minimized.

codecov-commenter commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

typotter left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

typotter commented Feb 19, 2026 •

edited

Loading

codecov-commenter commented Feb 20, 2026 •

edited

Loading