Skip to content

Update cached company metrics on track regardless of connection state - #202

Open
ryanechternacht wants to merge 1 commit into
mainfrom
ryan/replicator-track-offline
Open

ryanechternacht wants to merge 1 commit into
mainfrom
ryan/replicator-track-offline

Conversation

@ryanechternacht

Copy link
Copy Markdown
Member

Problem

Track optimistically bumps the cached company's metrics so usage limits reflect an event before the server pushes an updated company. In replicator mode, when a customer's Schematic account is closed or Schematic is unreachable, the replicator stays up with its Redis cache but reports ready: false, and flag checks keep evaluating from that cache. Usage should keep counting locally in that state.

Findings

The update in Schematic.Track is gated on _datastreamConnected. That field is only maintained by StartConnectionMonitoring, which runs only in websocket mode. In replicator mode the constructor sets it to false and nothing ever changes it. So in replicator mode the optimistic update has never run, whether the replicator reports ready or not. The track event itself is still enqueued and sent to the API.

History: the gate came in with the optimistic update itself in #81 (company != null && _datastreamClient != null && _datastreamConnected) and was carried into #91 (replicator mode), which added the _datastreamConnected = false replicator branch. Neither PR has a description or a review comment about the gate. The #81 review discussion was about deep copying the company and locking so concurrent updates are not lost, not about connection state. In the Go SDK the equivalent gate (c.datastreamClient.IsConnected(), which maps to replicator readiness in replicator mode) came from a refactor (schematic-go #82) of an earlier datastreamConnected check, also without a stated reason.

Correctness checks for removing it:

  • Double counting: company updates from the server (full, or partial via Merge.UpsertMetrics) carry absolute metric values and replace the cached company. The local bump is superseded by the next server value, which already includes the event if the server counted it. Nothing adds a server delta on top of the local bump.
  • Missing cache: UpdateCompanyMetrics returns without writing when the company is not cached, so no partial or empty entries are created.
  • Closed account: the event is rejected server-side, so the local bump is the only record until the replicator next writes that company, which is the intended behavior.
  • Concurrency: the read-modify-write is locked per company within a process but not across processes sharing Redis, so concurrent tracks from different processes can lose an increment. That exists today in websocket mode with Redis and does not depend on connection state.

No correctness reason to keep the gate was found.

Change

  • Remove the connection gate. The update runs whenever a datastream client exists and the event names a non-empty company.
  • Websocket mode also drops the gate. Flag checks there keep evaluating from the cache while the socket is down, so the same reasoning applies, and server updates replace the cached company on reconnect.
  • UpdateCompanyMetrics returns false for routine cases (company not cached, event with no matching metric) and logs its own real failures (lock timeout, cache write failure). The caller now logs false at debug instead of error. Without this, enabling the update in replicator mode would log an error for every track event that has no matching metric.
  • Remove the now unused private UseDatastream() helper.

Note: in replicator mode Track now does Redis reads and a write per call that names a company, the same work websocket mode with Redis already does.

Test

New TrackOfflineMetricsTests:

  • Replicator mode, not ready: two Track calls bump the cached metric from 10 to 15 (failed before this change).
  • Replicator mode, ready: same result (also failed before this change, showing the update never ran in replicator mode).
  • Replicator mode, company not cached: nothing is written to the cache.
  • Websocket mode, disconnected: Track bumps the cached metric (failed before this change).

dotnet build (Release, all target frameworks) and dotnet test: 442 passed, 0 failed.

🤖 Generated with Claude Code

Track's optimistic metric update was gated on _datastreamConnected. That
field is only maintained by websocket connection monitoring. In replicator
mode it is set to false at construction and never updated, so the update
never ran in replicator mode, whether or not the replicator reported ready.
In websocket mode it stopped while the connection was down, even though
flag checks keep evaluating from the cache in that state.

Remove the gate so the update runs whenever a datastream client exists and
the event names a company. Server updates carry absolute metric values and
replace the cached company, so the local bump is superseded rather than
double counted, and an uncached company is left alone. The event is still
sent to the API as before.

UpdateCompanyMetrics returns false for routine cases (company not cached,
no matching metric) and logs its own real failures, so the caller now logs
false at debug instead of error. Otherwise enabling the update in replicator
mode would log an error for every track event without a matching metric.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@ryanechternacht
ryanechternacht force-pushed the ryan/replicator-track-offline branch from 13ee966 to 509a91e Compare September 30, 2026 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant