Make WindTrackMasterData.windTrack transient and transmit each wind track's
fixes as individual top-level writeObject(...) calls terminated by a null
sentinel (mirroring the bug6227 GPS-fix streaming), resetting the
ObjectOutputStream handle table every FIXES_PER_HANDLE_TABLE_RESET fixes.
This restores reader/writer interleaving so the importer keeps draining the
HTTP socket during large Mongo inserts, avoiding the Jetty connector
idle-timeout EofException / Premature EOF on multi-day wind tracks.
The deduping against already-present fixes is preserved on the import side.
Assisted-By: Claude Opus 4.8
Record in the adapter's class Javadoc that activecerts (and thus search())
only ever returns active certificates, that the sole wired route to an
inactive certificate is DownBoatRMS_byrefno by a known RefNo, and the
two-hop country-fleet → dxtName → DownBoatRMS path that would let one
discover inactive certificates by boat identity (not wired today; would
also need a dxtName-keyed handle representation).
Also fix a stale {@link #search(..., boolean)} reference to the current
6-arg signature.
Assisted-By: Claude Opus 4.8 (claude-opus-4-8)
The key-based sas.orc.org API routes all active-certificate search through the
activecerts endpoint, which returns active certificates only. The includeInvalid
flag on ORCPublicCertificateDatabase.search(...) could therefore never change the
result and was dead. Remove it from the interface and impl, the default
getCertificateHandle, the two fuzzy-search call sites, SailingServiceImpl, and all
ORC test call sites. The sole route to an inactive certificate in this adapter is to
download it by reference number via DownBoatRMS_byrefno (getCertificate /
getCertificateHandle); the search Javadoc now states this explicitly.
Also export ORC_API_KEY from java/target/env-default-rules.sh so the Java VM can read
it via System.getenv(...). configuration/secrets assigns it as a plain (non-exported)
shell variable that the java child process would not otherwise inherit; it is passed
as an environment variable rather than a -D system property so it is not exposed on
the process command line.
Assisted-By: Claude Opus 4.8 (claude-opus-5-5)
Replace the decommissioned data.orc.org/public/WPub.dll XML endpoints with the
new key-based JSON API at https://sas.orc.org/api/{key}/<endpoint> (docs:
https://sas.orc.org/api-docs.php), restoring the pre-bug6318 semantics that the
bug6318 stopgap had regressed:
- country-less and year-based search via the 'activecerts' endpoint
(all criteria optional and AND-combined server-side; no criteria -> every
active certificate), removing the forced-country / current-year-only branches;
- real per-country/family overview counts and VPP year via the 'RMS' endpoint,
replacing the stubbed certCount=0 / vppYear=null;
- full-certificate download via 'DownBoatRMS_byrefno' (top-level rms[], consumed
by the unchanged ORCCertificatesJsonImporter);
- TestORCPerformanceCurve bulk online fetch via 'DownRMS'.
The API key is read from the ORC_API_KEY environment variable and placed into the
request path; it is never logged. When the key is absent the adapter throws, so
the live tests FAIL loudly rather than skip. The two pure-offline date-parse tests
run independently of the key. release.yml passes ORC_API_KEY to the test runtime,
gated so compile-only (skip_tests) runs never see the real secret.
Assisted-By: Claude Opus 4.8 (claude-opus-4-8)
The runWhenDoneLoading rewrite in 15dbbc03a1 (companion bug6241
listener-leak fix) made runWhenDoneLoading register a race-removal
listener on the race's regatta via TrackedRegatta.addRaceListener. When
a replica performs its initial load from a master
(RacingEventServiceImpl.initiallyFillFromInternal), it registers a
synchronous RaceAdditionListener on each deserialized regatta. Those
regattas already contain their races, so addRaceListener's catch-up loop
fires raceAdded for every existing race -- and it did so from INSIDE the
ConcurrentHashMap.computeIfAbsent mapping function, because the
SynchronousRunnableExecutor runs the dispatch inline. raceAdded then
called runWhenDoneLoading, which re-entered addRaceListener and nested a
second computeIfAbsent on the same raceListeners map while the outer one
still held the key's reservation node. ConcurrentHashMap forbids the
mapping function from modifying the map, so the nested update spun/blocked
forever on the reservation node -- a single-threaded self-deadlock, not a
lock-ordering deadlock.
This affected production: a replica would hang indefinitely during
initial load from a non-empty master (both automatic replica startup via
the replication Activator and operator-initiated replication). It also
wedged the CI build, which runs the replication initial-load tests;
TrackedRaceWithSensorFixStoreContentsReplicationTest hung until the 6h
GitHub Actions cap cancelled the job. The normal getOrCreateTrackedRegatta
path was unaffected because the listener there is registered on a
freshly-created empty regatta (zero-race catch-up, no re-entry).
Fix: TrackedRegattaImpl.addRaceListener now captures the catch-up work in
a holder inside the mapping function and runs it AFTER computeIfAbsent
returns, with the entry fully installed and no reservation node held. The
holder is populated only when a new entry was actually created, preserving
the duplicate-registration collapse, and the catch-up dispatch still runs
under the trackedRacesLock read lock, so ordering relative to enqueEvent
is unchanged. A nested addRaceListener for a different listener key now
proceeds normally.
TrackedRaceWithSensorFixStoreContentsReplicationTest now passes in ~7s.
Assisted-By: Claude (Anthropic claude-4.8-opus)
On ARCHIVE (replay-heavy) startup the restore counters could drift:
numberofracesrestoreddoneloading short and numberofracesstillloading
negative (observed -1). The negative count traced to a single race whose
stillLoading-- callback fired twice against one matching raceAdded +1.
Every restored race arms runWhenDoneLoading twice, from two independent
sites with two distinct runnables (restore path -> doneLoading++, and
RaceAdditionListener.raceAdded -> stillLoading--), each meant to fire
once. The old runWhenDoneLoading removed its listener inside
statusChanged with no one-shot guard. Because notifyListeners snapshots
the listener set under lock and then iterates the snapshot outside the
lock, a removeListener happening during dispatch does not shorten an
already-in-flight iteration. When a race crossed past-LOADING twice
back-to-back under concurrent status pushes (loader finish plus the
stopTracking teardown), a second dispatch snapshotted the listener array
before the first dispatch's removeListener landed, so the callback fired
on both transitions -> two decrements vs one increment -> net -1 (and a
symmetric +1 over-count on doneLoading).
Rewrite runWhenDoneLoading to mirror runWhenPastLoading: a settled
AtomicBoolean CAS makes firing strictly one-shot per registration no
matter how many times or along how many concurrent paths the transition
is delivered, plus a cooperating regatta raceRemoved listener that tears
down and settles-without-firing if the race is removed while still
PREPARED/LOADING/ERROR (closing the companion bug6241 listener-leak
class).
Add TrackedRaceRunWhenDoneLoadingTest as contract tests for the
single-fire / already-done / removed-before-done / fire-then-remove
behaviours. Note these do not reproduce the production concurrency
interleaving: setStatus dispatches statusChanged synchronously and
sequentially, so single-threaded JUnit cannot deterministically recreate
the snapshot/remove race; the fix rests on the settled CAS.
Assisted-By: Claude (Anthropic claude-4.8-opus)
the old code subtracted the millisecond representation of the time
points from one another and cast the result to an int which may have
silently led to an overflow in case the difference between the time
points was greater than 49 days. For races with open-ended tracking time
ranges and sporadic fixes along that full time range may have caused
problems here.
Introduces two convenience methods on TimePoint that step by one resolution
unit rather than a hard-coded millisecond:
TimePoint plusResolution() == plus(getResolution())
TimePoint minusResolution() == minus(getResolution())
They are implemented once in AbstractTimePoint and saturate at EndOfTime /
BeginningOfTime just like plus/minus do. Expressing "the next/previous
representable instant" through getResolution() keeps these sites correct
should a finer-resolution TimePoint ever be introduced, instead of assuming
one-millisecond granularity.
All plus(1)/minus(1) call sites whose receiver is a TimePoint and whose intent
is a single resolution tick were converted to plusResolution()/minusResolution()
across production and tests. Deliberately left unchanged:
- plus(1)/minus(1) on Duration receivers (not TimePoints)
- TimePointTest overflow/saturation tests that exercise plus(long)/minus(long)
with the literal 1 on purpose
Assisted-By: Claude Opus 4.8 (claude-opus-5)
Introduce TimePoint.getResolution() returning the smallest positive Duration by which two
distinct instants of an implementation can differ, implemented in AbstractTimePoint as
Duration.ONE_MILLISECOND (all current impls are millisecond-backed). TopLevelMasterData's
inclusive-to-exclusive conversion for device mapping ranges now uses
toInclusive.plus(toInclusive.getResolution()) instead of a hard-coded +1ms, so a
single-instant PING mapping stays a non-empty range and the code remains correct should a
finer-resolution TimePoint ever be introduced.
Assisted-By: Claude Opus 4.8 (claude-opus-5)
RegattaLogDeviceMappingEvent.getToInclusive() is an inclusive end, but
TimeRange.to() is exclusive. TopLevelMasterData.addRangeIfMappingEvent
built TimeRange.create(from, toInclusive); for a single-instant PING
mapping (from == toInclusive, e.g. a pinged mark) this is an empty range,
which MultiTimeRange silently discards. The device is then never streamed
and its single fix never reaches the importing instance.
Convert the inclusive end to an exclusive one by adding one TimePoint
resolution unit (currently 1ms), so [t, t] becomes the non-empty range
[t, t+1ms), and load the corresponding fixes with toIsInclusive == false
in MasterDataResource.writeRaceLogTrackingFixes. Update the related
Javadoc on both sides accordingly.
Adds MasterDataImportTest.testMasterDataImportPingedMarkSingleInstantFix,
which reproduces the drop (single-instant inclusive mapping with one
stored fix) and now passes.
Assisted-By: Claude Opus 4.8 (claude-opus-5)
Follow-up to fd6739d9a6 (stream GPS fixes as top-level objects), addressing
four defects on the master data import path:
- Merge DeviceMappingDescriptor objects per DeviceIdentifier into a single
MultiTimeRange so a device mapped in several regattas (or via several
open-ended mapping events) has its fixes streamed once over coalesced,
non-overlapping sub-ranges rather than once per mapping. Obsoletes and
removes the nested DeviceMappingDescriptor class; the field becomes
Map<DeviceIdentifier, MultiTimeRange> raceLogTrackingDeviceRanges. The
inclusive mapping end is carried through and streamed with
toIsInclusive=true, at worst streaming a harmless superset (store dedups).
- Apply the top-level-streaming + per-object reset() design to
WindTrackMasterData: strip the wind tracks out of the serialized graph,
stream one WindTrackMasterData at a time on export (held only transiently),
and read+store them incrementally on import (master only). Bounds retained
memory to a single wind track at a time instead of the whole season.
- Remove the premature "Done importing master data into ..." log and the
overall 1.0 from ImportMasterDataOperation.internalApplyTo; that fired
before the sensor-fix import ran. The single terminal "Done" and 1.0 now
live in MasterDataImporter.importFromStream, the true end of the import.
- Make progress rise monotonically and reserve the two phases that usually
dominate the import: the object graph now ends at 0.6, the wind-track
import occupies 0.6 -> 0.8 (20%), and the sensor-fix import occupies
0.8 -> 1.0 (20%), instead of reaching 1.0 in the operation and then
dropping back in the importer.
Assisted-By: Claude Opus 4.8 (claude-opus-5)
Master data import/export (MDI) previously materialized every mapped
device's GPS fixes in memory before serialization: the fixes lived inside
the single TopLevelMasterData object graph written with one writeObject, so
ObjectOutputStream/ObjectInputStream had to retain a handle for every fix to
resolve potential back-references. For events with many fixes this dominated
heap on both the exporting and importing server.
Change the wire format so fixes are streamed as top-level stream objects
after the (now fix-free) TopLevelMasterData:
- TopLevelMasterData no longer carries the fixes themselves. It records
lightweight, fully Serializable DeviceFixMappingDescriptor entries
(device, from, toInclusive) plus a transient SensorFixStore used only on
the export side. Default serialization now does the right thing; the
stripped replica copy carries no descriptors.
- MasterDataResource.writeRaceLogTrackingFixes streams each device section
straight from the SensorFixStore: device header, each fix as a top-level
object, a null fix terminator, then ObjectOutputStream.reset() once per
device so no fix handle is retained across devices while the fix class
descriptor stays shared within a device. A trailing null device ends the
section. Framing uses null sentinels, never fix counts (a concurrent write
could invalidate a count).
- MasterDataImporter.importRaceLogTrackingGPSFixes reads that section
incrementally and stores fixes in batches of BATCH_SIZE_FOR_IMPORTING_FIXES
directly into the master's SensorFixStore, holding at most one batch at a
time. This stream-based import only ever runs on the primary/master:
such requests must always be routed there, and a replica receives only the
stripped ImportMasterDataOperation (no fixes), obtaining tracking data by
the master loading tracked races whose transactions are replicated. A
non-null master descriptor here is therefore a broken routing state and is
reported by throwing IllegalStateException rather than silently draining.
IMPORT_SENSOR_FIXES sub-progress is advanced once per device section,
driven by the descriptor count, restoring the per-device granularity that
existed before.
- ImportMasterDataOperation no longer imports fixes; that logic moved to
MasterDataImporter, which has the target store in scope.
Exporter and importer are upgraded together; mixed-version transfer is not
required.
Verified: MasterDataImportTest green (full class 13/13 repeatedly;
testMasterDataImportWithoutHttpStack repeatedly), covering the 4999/5000/5001
batch-boundary fix counts and the DoubleVectorFix round trip.
Assisted-By: Claude Opus 4.8 (claude-opus-5)
Two related fixes for the permanent hang where a request thread held the
raceTrackersByRegatta write lock while parked in Object.wait(), timing out all
subsequent requests to the service.
TrackedRaceImpl.waitForAllRaceLogsAttached(): the wait guard re-evaluates the
live attached-race-log count inside the synchronized(latchForRaceLogs) block
instead of a stale local captured before entering the monitor. The stale local
could never change, so a notifyAll() that fired between the initial read and
entering wait() was lost, parking the thread forever (classic lost-wakeup).
RacingEventServiceImpl.addRace(...): split the register-or-notify handshake so
that only *claiming* (removing) the callbacks for the new tracker happens under
the raceTrackersByRegatta write lock, atomically with publishing the tracker;
the callbacks are then *invoked* after the lock is released. A callback may
block during tracker startup (waiting for race logs to attach), and previously
that wait ran while holding the global write lock, turning a single stalled
tracker into a service-wide outage. The read-locked find-or-enqueue in
getRaceTrackerByRegattaAndRaceIdentifier(...) still races atomically against the
claim, so no callback can be lost.
Assisted-By: Claude Opus 4.8
Follow-up to holding PolarDataMiner's race-keyed collections weakly. The weak
key on callbacksWaitingForFixIngestion was defeated by its own values: a parked
callback (the scheduleWindEstimationInstallation closure) strongly captures the
same TrackedRace that keys its entry, so the entry -- and the race with all its
tracks -- would never be collected while parked. That is a brittle contract to
place on callers (they must ensure their Runnable holds no strong reference to
the race), and easy to get wrong.
Instead:
- callbacksWaitingForFixIngestion goes back to a strong HashMap (weak keys were
misleading there) and is pruned explicitly.
- racesWithIngestedFixes stays a weak set (it has no values, so weak keys are
effective) and is additionally pruned eagerly as belt-and-suspenders.
- New PolarDataService.raceRemoved(TrackedRace) (delegating to
PolarDataMiner.raceRemoved) forgets both collections for a race.
- RacingEventServiceImpl.RaceAdditionListener.raceRemoved now calls
polarDataService.raceRemoved(trackedRace) (null-guarded, since the service
arrives via an OSGi ServiceTracker).
The runWhenPolarLoadingFinishedFor Javadoc gains an explicit LEAK CONTRACT
paragraph documenting that callers must arrange for raceRemoved to be called on
race removal, and why the map cannot rely on weak keys. Dropping a removed
instance's parked install callback loses nothing: a re-added race is a new
TrackedRace instance that gets a fresh raceAdded -> scheduleWindEstimationInstallation
-> runWhenPolarLoadingFinishedFor registration. Post-startup manual loads still
wait for the loading-pipeline drain (Gate A on racesWithIngestedFixes ensures the
race's own fixes are ingested before the callback is eligible; the drain then
covers it and any concurrently-loading races). Live TRACKING fixes flow through
the separate preFilteringProcessor chain and never perturb the loading drain.
Also documents the bounded, self-clearing lifecycle of
callbacksWaitingForLoadingOfAllRacesToRestoreToStart.
The Javadoc on PolarDataService.runWhenPolarLoadingFinishedFor (interface),
PolarDataMiner (impl) and the RacingEventServiceImpl.scheduleWindEstimationInstallation
caller all read as if the callback fired once the fixes belonging to the given
race had drained. In reality the loading pipeline (preFilteringProcessorForLoadedFixes)
is shared across all races and has no per-race completion tracking, so the
callback fires only on a global drain: when the fixes of every race ingested so
far have been processed.
The race parameter does not scope the wait; it only gates WHEN the callback may
attach to the shared drain, via two conditions that must both hold: (1) the
race's own fixes have been ingested (so the callback can't fire on a pipeline
that is merely momentarily idle before this race was queued), and (2)
markLoadingOfAllRacesToRestoreStarted() has been signalled (so an idle window
between two startup races' ingestion bursts isn't mistaken for a real drain).
This global-wait behavior is deliberate: we want the polar model to reflect all
loaded data before any maneuver-based wind estimation is installed, and we accept
the resulting startup sequentiality (an early race's estimator install waits
behind the whole loaded-fix backlog). Documentation only; no behavior change.
The method name is kept as-is.
The delegate's canBeUpdated() was returning true unconditionally, hiding the
distinction between the DB-load state (cacheToUse = ManeuversFromDatabase,
inner canBeUpdated=false) and the compute state (cacheToUse =
ManeuversFromSmartFutureCache, inner canBeUpdated=true). That broke the
gate on TrackedRaceImpl.feedAlreadyKnownManeuversToWindEstimation which
uses maneuverCache.canBeUpdated() to decide whether the current JVM's
cache is DB-backed and therefore needs alreadyClassifiedManeuversAvailable
to feed the estimator.
With the pre-fix behavior, the gate never opened even for Path A races:
feed was always skipped, the estimator never received the loaded
maneuvers, PreClassifiedUpdate never ran, and the MBE wind track stayed
empty. Observable as hasWindData=false on all fingerprint-matched races
of "The Final Push 49erFX" (W1..W9, W11, W12) on archive-candidate even
28+ hours after the wind-estimation install callback had fired, while
production (running older code) had hasWindData=true.
Change canBeUpdated() to delegate to cacheToUse.canBeUpdated(). Now it
truthfully reflects which inner cache is in use. All other callers of
cacheToUse.canBeUpdated() within the delegate (ensureFilled, computeAndStore,
triggerUpdate) already read the inner cache directly and are unaffected.
The only external caller was TrackedRaceImpl.feedAlreadyKnownManeuversToWindEstimation
which now correctly opens the gate for Path A and keeps it closed for
Path B.
Javadoc on the method explains the semantics and how it toggles across
resume() and triggerUpdate() calls.
The existing isAllEmpty() self-heal in ManeuverCacheDelegate.resume() only
caught the case where every competitor's persisted maneuver list was null
or literally empty. A separate poisoned state slipped through: lists are
non-empty but every stored maneuver is typed as UNKNOWN (or PENALTY_CIRCLE),
which happens when the detector ran while getWind(pos, at) still returned
null everywhere and createManeuverFromManeuverCurveAndWind hit its 'wind ==
null' fall-through in ManeuverDetectorImpl.
Feeding such records to the estimator via alreadyClassifiedManeuversAvailable
produces zero wind fixes because
IncrementalMstHmmWindEstimationForTrackedRace.mapManeuverType maps UNKNOWN
and PENALTY_CIRCLE to null and the whole batch gets filtered out. Observable
as hasWindData=false on the affected races even after the wind estimation
callback has fired. Confirmed on archive-candidate for W10 of "The Final
Push 49erFX" (982 stored maneuvers, all UNKNOWN) and for Abeam Training
2016 Race 1 (453 stored, all UNKNOWN); production shows the same event with
hasWindData=true because it ran with older code paths that populated the
MBE track through a different route.
The fix adds a companion check, hasNoClassifiableManeuver, invoked after
isAllEmpty in resume(). It returns true iff no stored maneuver anywhere in
the loaded map is typed as TACK/JIBE/HEAD_UP/BEAR_AWAY (the four types
mapManeuverType turns into a non-null ManeuverTypeForClassification). When
it fires, resume() logs the reason and falls through to the compute path,
which now correctly sequences its storage after the wind estimator has
settled (see computeAndStore / retypeAndStoreAfterWindEstimationSettled) so
the recompute produces properly-typed maneuvers and overwrites the poison.
isClassifiableType() is a tiny helper listing the four classifiable types;
a comment reminds future maintainers to keep it in sync with mapManeuverType.
Restores the compute-path guard on feedAlreadyKnownManeuversToWindEstimation.
Without the gate, setWindEstimation submits per-competitor feed tasks that
call maneuverCache.get(waitForLatest=true) concurrently with the ordinary
detector-driven feed via newManeuverSpotsDetected. Depending on ordering the
feed task either sees the recalculation already in progress (blocks, then
double-feeds via alreadyClassifiedManeuversAvailable, whose per-competitor
PreClassifiedUpdate reconciliation clobbers the graph path's whole-race
track) or completes before the trigger fires (get returns null, feed skips).
The double-feed shows up as flakiness of
IncrementalMstHmmWindEstimationForTrackedRaceTest -- observed locally with a
ratio-of-matching-fixes of 0.658 vs the 0.75 threshold, and also seen on
GitHub Actions run 30262443673.
The rationale for removing the gate previously (that after the bug6241
polar-data hold-back the estimator may be installed after detection ran
with a null WindEstimationInteraction) is now handled explicitly by the
retype-and-store choreography in ManeuverCacheDelegate.computeAndStore:
its runWhenWindEstimationInstalled callback re-drives the detector once
the estimator is in place, and freshly built detectors (whose per-competitor
cache was cleared by updateManeuversAndWindWithNewWindEstimation at install
time) emit spots through the graph path with cross-competitor aggregation
intact -- no per-competitor slice ever needed.
Adds a substantial comment on the method spelling out why the gate must
stay in place, so future edits don't remove it again.
The previous single-round retypeAndStoreAfterWindEstimationSettled was
incorrect: it called waitUntilDone on the estimator before any spots
had been re-emitted (the compute-path's initial detection ran with a
null WindEstimationInteraction because the estimator hadn't been
installed yet, so no spots were pushed). waitUntilDone then returned
immediately on an empty queue, and the subsequent recalculate emitted
spots but returned before the estimator could process them and produce
wind fixes. Net effect: stored maneuvers were still typed with null
wind (mostly UNKNOWN), reproducing the poisoned-DB pattern the fix was
supposed to prevent.
Split into two rounds:
1. First recalculate + get per competitor. Since setWindEstimation
cleared maneuverDetectorPerCompetitorCache, this builds a fresh
detector with a non-null WindEstimationInteraction, runs a full
scan, and *does* emit spots to the estimator during this pass.
2. waitUntilDone -- now the estimator's queue has the spots from
round 1 and this actually waits for the graph inference to
produce wind fixes and flow them into the tracked race's
MANEUVER_BASED_ESTIMATION track.
3. Second recalculate + get per competitor. The detector's
lastManeuverDetectionResult is now populated and no new raw fixes
have arrived, so it takes the re-type branch of detectManeuverSpots
-- which routes through determineManeuversFromManeuverCurve and
types the spots using trackedRace.getWind(...), which now sees the
estimator's fixes (bug6274 removed the MANEUVER_BASED_ESTIMATION
exclusion). The isManeuverSpotWindNearlySame check correctly
detects that the previous typing had null wind and the current
has non-null, so it doesn't short-circuit to the UNKNOWN spot.
4. Snapshot the classified maneuvers and store.
For W1 in 49erFX (5 competitors), pre-fix Mongo had 1 competitor with
362 typed maneuvers (whose detection happened to run after
setWindEstimation) and 1 with 628 all-UNKNOWN (whose detection ran
before setWindEstimation). Post-fix, both should be typed.
The Runnable passed to runWhenWindEstimationInstalled(...) fires
synchronously on whichever thread first invokes setWindEstimation with
a non-null argument. That thread is typically a shared
background-executor thread (the same pool used by SmartFutureCache and
the wind estimator's GraphRecalculationTask). Running waitUntilDone,
recalculate(...), and get(waitForLatest=true) on that thread makes the
callback block waiting for tasks that need the same pool to progress,
which starves the pool -- fatal with pool size 1, and observed as
timeout retries on saturated pools in local runs.
Detach the callback body onto a dedicated Thread so the setWindEstimation
caller returns immediately and the shared pool stays free to service the
tasks the retype-and-store step is waiting on.
The previous ManeuverCacheDelegate.resume() Path B stored the initial
detector output before the wind estimator had a chance to produce any
fixes. For races without another wind source (typically no-race-start
practice sessions with only GPS tracks), that first pass typed every
spot as UNKNOWN or produced no maneuvers at all, and the empty result
was persisted. On subsequent server starts the fingerprint matched, an
empty maneuver list got loaded, feedAlreadyKnownManeuversToWindEstimation
had nothing to hand to the estimator, and no wind fixes materialised.
Two fixes:
* New primitive TrackedRace.runWhenWindEstimationInstalled(Runnable),
default fires synchronously if getWindEstimation() is already
non-null else no-op (suitable for mocks). TrackedRaceImpl overrides
with the full callback-registry pattern used by runWhenPastLoading
(two-cooperating-listeners, CAS-guarded single fire, race-removal
cancellation via regatta listener). Two new transient fields on
TrackedRaceImpl for the callback list and its monitor, both
reinitialized in readObject for replicas.
* ManeuverCacheDelegate.resume() refactored into three methods --
resume(), computeAndStore(), retypeAndStoreAfterWindEstimationSettled().
The compute path now runs the detector, then via
runWhenWindEstimationInstalled(...) waits until the estimator is
installed, awaits its inference via waitUntilDone(), recalculates
each competitor so maneuvers are re-typed using the estimator's
MANEUVER_BASED_ESTIMATION fixes (visible to typing thanks to bug6274),
then snapshots and persists. The DB now captures the converged
classification, not the pre-estimator empty state.
resume() also self-heals against DB records poisoned by earlier server
runs: on fingerprint match with all-empty stored maneuvers, treats as
a stale cache miss and falls through to compute. Races that legitimately
have no maneuvers (short races with no motion) will recompute on every
startup, which is acceptable because the cost is proportional to zero.
Previously the polars bundle registered its PolarDataService in the OSGi
registry immediately at bundle start, then unregistered/re-registered
around a remote regression-data fetch in a background thread. The
unregister transiently nulled RacingEventServiceImpl.polarDataService,
opening a window in which PolarFixCacheUpdater.statusChanged would skip
its raceFinishedLoading call for any race transitioning past LOADING
during that window, and in which restoreTrackedRaces()'s call to
markLoadingOfAllRacesToRestoreStarted() would silently drop.
Reversed the dependency: the polars Activator now opens a ServiceTracker
for RacingEventService, obtains its DomainFactory, constructs the
PolarDataServiceImpl, populates it (registerDomainFactory, then remote
fetch if configured), and only then registers the service in the OSGi
registry -- exactly once. The OSGi contract now holds: if the service
is registered, it is ready.
Also dropped the !maneuverCache.canBeUpdated() gate on
TrackedRaceImpl.feedAlreadyKnownManeuversToWindEstimation, so that
compute-path races whose maneuver detector captured a null
WindEstimationInteraction at detection time (because the estimator
hadn't been installed yet) still get their detected maneuvers fed to
the estimator when it arrives.
Manifest change: com.sap.sailing.polars now Require-Bundles
com.sap.sailing.server.interface directly, in order to reference the
RacingEventService type from the Activator's ServiceTracker.
The dedicated raceListenersLock was causing a read-then-write
self-deadlock in the raceAdded dispatch chain:
1. addTrackedRace holds trackedRacesLock for write, calls enqueEvent.
2. enqueEvent holds raceListenersLock for read while iterating
raceListeners.forEach and dispatching to each listener's
RunnableExecutor.
3. For synchronous listeners (SynchronousRunnableExecutor), the
callback runs inline on the enqueEvent thread. In particular,
RaceAdditionListener.raceAdded reaches
RacingEventServiceImpl.scheduleWindEstimationInstallation ->
TrackedRaceImpl.runWhenPastLoading ->
TrackedRegatta.addRaceListener, which tries to acquire
raceListenersLock for write.
4. ReentrantReadWriteLock does not allow upgrading read to write;
the write acquisition blocks the same thread against itself.
Observed as a client-thread readLock timeout on the tracked-races
lock (qtp1682955428-52) with the current-writer trace showing the
RaceCourseReceiver thread stuck inside
ReentrantReadWriteLock.tryLock via addRaceListener.
The raceListenersLock was redundant. Its serialization role is
already provided by trackedRacesLock:
* raceListeners is a ConcurrentHashMap; computeIfAbsent, remove,
and forEach are individually atomic.
* enqueEvent is only called from addTrackedRace / removeTrackedRace,
both of which hold trackedRacesLock for write throughout. During
that write region, addRaceListener and removeRaceListener cannot
proceed because each takes trackedRacesLock for read at entry
(blocked by write). So the listener set is stable for the whole
dispatch.
* The add/remove listener paths keep their trackedRacesLock read
acquisition, which serializes them against addTrackedRace /
removeTrackedRace (avoiding a race that could enqueue an event
on a listener being torn down).
The raceListeners field's Javadoc now documents this concurrency
contract explicitly, so a future reader knows why there is no
dedicated raceListeners lock.
Reverts the three call-site changes from issue #6184 (commit
1368179ac0) that passed getWindSources(MANEUVER_BASED_ESTIMATION) as
the exclusion set to trackedRace.getWind(pos, at, exclude) inside the
maneuver-typing paths:
* IncrementalManeuverDetectorImpl.getManeuverSpotWithTypedManeuversFromExistingManeuverSpotConsideringPossibleWindChange
(line 261) -- re-typing an existing maneuver spot when the wind at
its position/time has changed.
* ManeuverDetectorImpl.determineManeuversFromManeuverCurve (line 335)
-- initial typing of a fresh spot.
* ManeuverDetectorWithEstimationDataSupportDecoratorImpl (line 78) --
same, in the estimation-data-support decorator.
Each now calls the two-arg getWind(pos, at), letting the tracked race's
confidence-weighted averaging pick the best wind source. When
non-estimation sources (EXPEDITION, TRACK_BASED_ESTIMATION, COURSE_BASED,
etc.) provide a wind fix at the requested position and time, their
higher base confidence dominates and MANEUVER_BASED_ESTIMATION does
not affect the result meaningfully -- so #6184's original concern
(the estimation track feeding back on itself with equal weight) does
not materialize.
When no other wind source has a fix, the estimation track's wind (with
its lower base confidence) is used. That is the intended bootstrap of
the wind-estimation-from-maneuvers design:
1. Maneuver *spots* detected on the competitor tracks (geometry only).
2. Estimator's NN + MST/HMM pre-classifies the spots, producing wind
fixes on the MANEUVER_BASED_ESTIMATION track.
3. The wind track update triggers a maneuver-cache recalc; on
re-typing, isManeuverSpotWindNearlySame reads all wind sources
including the estimation track (line 437, unchanged by #6184).
4. determineManeuversFromManeuverCurve types the spot using whatever
wind sources are available.
With #6184's exclusion in place, step 4 refused to use the estimation
track's wind, so races without any other wind source could never
progress from pre-classified spots to fully-typed ManeuverImpl
objects, breaking the bootstrap.
Removing the exclusion restores the bootstrap. Confidence-weighted
averaging inherent to TrackedRaceImpl.getWindWithConfidenceUncached
handles the 'prefer non-estimation when available' concern that
motivated #6184.
The hand-off from setWindEstimation into the newly-installed estimator
via alreadyClassifiedManeuversAvailable was firing unconditionally
whenever the wind estimation transitioned from null to non-null. That
is only useful in the DB-load path (ManeuversFromDatabase, where the
maneuver detector never invokes newManeuverSpotsDetected because
maneuvers came from the persistent cache, not from detection). In the
compute path (ManeuversFromSmartFutureCache), detection runs after
setWindEstimation triggers triggerManeuverCacheRecalculationForAllCompetitors,
freshly built detectors capture the newly installed WindEstimationInteraction,
and newManeuverSpotsDetected drives the MST/HMM graph path -- which is
the correct way to feed the estimator because it aggregates ALL
competitors' spots into the same graph and benefits from their spatial
and temporal proximity.
Feeding the same maneuvers a second time via the per-competitor hand-off
would produce per-competitor slices of wind fixes without that
cross-competitor MST aggregation. applyManeuverClassificationsToWindTrack
reconciles the estimated wind track down to whatever the current input
batch produces -- a per-competitor call would then clobber the correct
whole-race track computed by the graph path. Observed as a drop of the
ratio-of-matching-fixes in IncrementalMstHmmWindEstimationForTrackedRaceTest
line 250.
Gating on !maneuverCache.canBeUpdated() correctly selects the DB-load
path (canBeUpdated == false for ManeuversFromDatabase) and skips the
hand-off for the compute path. The gate is safe because ManeuverCacheDelegate.resume,
which switches cacheToUse between the two variants, runs synchronously
inside TrackedRaceImpl.setStatus's LOADING-to-post-LOADING transition
handler, whereas setWindEstimation is invoked afterwards (either by
scheduleWindEstimationInstallation waiting on runWhenPastLoading, or by
tests manually sequencing after wind sources are populated).
Fixes a thread-pool starvation deadlock observed in CI (log excerpts
from build 29344193740, timing out at 6h). setWindEstimation schedules
one per-competitor task onto ThreadPoolUtil.INSTANCE.getDefaultBackgroundTaskThreadPoolExecutor()
which then blocks in maneuverCache.get(competitor, /* waitForLatest */ true).
That get(_, true) is itself waiting on a SmartFutureCache FutureTask
scheduled on the SAME shared executor. In IncrementalMstHmmWindEstimationForTrackedRaceTest
the test flow is
setWindEstimation(...) --> queues N feed-waiter tasks
triggerManeuverCacheRecalculationForAllCompetitors() --> queues M detection tasks
waitForManeuverDetectionToFinish() --> main also blocks in get(_, true)
Once all pool threads are running feed-waiter tasks (or main is
holding one of them), the detection tasks queued behind them can never
start, and the waiters can never complete. Classic pool-starvation
deadlock. The log shows all 3 threads active, 76 tasks queued,
retrying forever.
Fix: give the feed waiters their own small dedicated daemon
ScheduledExecutorService via ThreadPoolUtil.createBackgroundTaskThreadPoolExecutor.
Its pool size defaults to max(2, getReasonableThreadPoolSize()/4) so
we don't hog resources on machines with modest core counts. Because
the waiters and the detection tasks now run on separate pools, the
waiters can block indefinitely without preventing the detection tasks
they wait for from making progress.
Introduces a single per-race primitive in RacingEventServiceImpl that
composes TrackedRace.runWhenPastLoading with
PolarDataService.runWhenPolarLoadingFinishedFor to install the
maneuver-based wind estimation on a race only once BOTH of these are
true: the race has reached a status strictly beyond LOADING (so
whatever loading was to happen has finished), AND the polar-data
mining pipeline has fully drained this race's loaded fixes.
The primitive is called from two places, replacing two separate
installation code paths with one:
1. RaceAdditionListener.raceAdded, for races added while
windEstimationFactoryService is already registered.
2. setWindEstimationFactoryService (via
setWindEstimationOnAllTrackedRaces), for races that were already
present when the wind-factory service arrives later.
Under either arrival order the same guard applies: the install only
runs if the wind-factory reference is still the one we were scheduled
with, the race hasn't been removed or errored, and no other estimator
has been installed on it in the meantime. This removes the ordering
sensitivity between OSGi service arrival and race creation that
comment #17 on issue 6241 identified as a root cause.
PolarFixCacheUpdater simplifies dramatically: its only remaining job is
to trigger ingestion of the race's fixes into the polar-mining loading
pipeline on the first transition to a status past LOADING. It no longer
holds an installation callback, and it now also fires ingestion for
races that go directly PREPARED->TRACKING without ever entering
LOADING (previously that fallback branch fired only the install
callback, not the ingestion). This ensures the polar-loading gate in
PolarDataMiner always opens for such races, so any pending
runWhenPolarLoadingFinishedFor callbacks can proceed.
Also adds TrackedRace.getWindEstimation() so the installation
primitive can guard against duplicate installation (it was previously
write-only).
Axel Uhl
Masha Kashirina