Repository navigation
Conversation
…BTechLab#17) TaskInitializer.init() loads the model and experiment configurations and schedules the feature registry setup and the periodic model-result and bloom-filter loading. The first model load stays on the periodic schedule, after a random delay of up to one refresh period (300 s by default), as before: blocking init() on it would add up to a minute of model load time to every deployment batch on large fleets, stop bidding for that long on an all-at-once restart, and send the fleet's first S3 reads in one burst. Callers had no way to tell when the model data was in place, and the README suggested sleeping for 10 seconds. Add BidRequestEvaluatorFactory.isReady(): - true once init() has completed, the extractor and transformer registries hold every ServiceLoader provider, and every model-result and bloom-filter loading task of that TaskInitializer has finished its first periodic run, whatever the outcome (a missing file, an S3 error or a loader exception still counts, so model data never blocks readiness forever); once true it stays true - the base class method throws UnsupportedOperationException, so existing subclasses keep compiling; DefaultBidRequestEvaluatorFactory overrides it Supporting changes, all additive: - PeriodicTaskWithRandomizedStart.hasFinishedFirstRun(), a volatile flag set in a finally block of the scheduled run - a TaskInitializer constructor taking a callback that init() runs once both stages have completed; init() now also throws when a stage ends with tasks still pending, instead of returning normally - getTask() on InitializerTaskOnPeriodicTask and ModelResultPeriodicLoadingInitializerTask Fix the init() and loading task javadoc to say what happens, and replace the README's Thread.sleep(10000) advice with isReady(), stating that this can take up to one refresh period plus the load time and that until a model loads it returns its no-match value: LowValue models forward (fail open), HighValue models score the request low value. Fixes IABTechLab#17
mpc00620
force-pushed
the
fix-17-init-loads-models
branch
from
October 20, 2026 22:40
2bbe4e3 to
562f751
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #17.
TaskInitializer.init()loads the model and experiment configurations and schedules the feature registry setup and the periodic model-result and bloom-filter loading. Before this change, callers had no way to tell when the model data was in place, and the README told them toThread.sleep(10000), which does not cover the first load: that load runs after a random delay of up to one refresh period (300 s by default).This PR keeps that first load lazy, exactly as on main, and adds a readiness signal instead.
Changes:
BidRequestEvaluatorFactory.isReady(). It is true onceinit()has completed, the extractor and transformer registries hold every ServiceLoader provider, and every model-result and bloom-filter loading task of thatTaskInitializerhas finished its first periodic run. The outcome of that run does not matter: a missing file, an S3 error or a loader exception still counts, so model data can never block readiness forever. Once true it stays true. The base class method throwsUnsupportedOperationException, so existing subclasses keep compiling.DefaultBidRequestEvaluatorFactoryoverrides it.PeriodicTaskWithRandomizedStart.hasFinishedFirstRun(). This is a volatile flag set in afinallyblock of the scheduled run, so an exception or anErrorstill marks the run as finished.TaskInitializerconstructor that takes a callback.init()runs the callback once both stages have completed.init()now also throws when a stage ends with tasks still pending, instead of returning normally. The three-argument constructor is unchanged.getTask()onInitializerTaskOnPeriodicTaskandModelResultPeriodicLoadingInitializerTask.init(),isReady()and the loading tasks now says what actually happens. The README replaces the sleep withisReady()and says it can take up to one refresh period plus the load time.LowValuemodel and 0.0 (low value) for aHighValuemodel. So evaluations fail open only forLowValuemodels. This is unchanged from main and is only documented here.Tests:
DefaultBidRequestEvaluatorFactoryReadinessTestuses a fake S3 and an executor that captures periodic tasks so each test decides when they run. It covers:isReady()is false afterinit()and true only after both model tasks have run.init()itself does not load model data.isReady()true.isReady()stays true after a later refresh fails.isReady()waits for the registry setup.isReady()stays false wheninit()fails.PeriodicTaskWithRandomizedStartTest: the flag before and after the first run, after an Exception and after an Error, and the first delay staying within one period.TaskInitializerTest: the callback runs after both stages, is not run on failure, and a null callback is rejected.LC_ALL=C.UTF-8 ./gradlew clean buildpasses: 556 tests, checkstyle and spotbugs green.