-
Notifications
You must be signed in to change notification settings - Fork 404
fix: skip heartbeat check for main dataset if no LLM channel #5139
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: next
Are you sure you want to change the base?
Changes from all commits
f0ac910
8904a43
0798996
c2184fe
2c74f5a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -110,6 +110,7 @@ export type HashTreeParserCallbacks = { | |
| locusInfoUpdateCallback: LocusInfoUpdateCallback; | ||
| syncLatencyTracker?: SyncLatencyTracker; | ||
| generateTrackingId?: GenerateTrackingId; | ||
| isLlmExpected?: () => boolean; | ||
| }; | ||
|
|
||
| const SYNC_METRICS_DATA_SETS = [ | ||
|
|
@@ -1837,14 +1838,27 @@ class HashTreeParser { | |
| continue; | ||
| } | ||
|
|
||
| if ( | ||
| LLM_DATASET_NAMES.includes(dataSet.name) && | ||
| this.callbacks.isLlmExpected && | ||
| !this.callbacks.isLlmExpected() | ||
| ) { | ||
| LoggerProxy.logger.info( | ||
| `HashTreeParser#resetHeartbeatWatchdogs --> ${this.debugId} skipping heartbeat watchdog timer for data set "${dataSet.name}" because LLM is disconnected` | ||
| ); | ||
|
|
||
| // eslint-disable-next-line no-continue | ||
| continue; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this code here handles the case when we call resetHeartbeatWatchdogs() for main and LLM is not connected, but what about the case when LLM is not connected yet, but it is connecting and later when it becomes connected, we will end up with no heartbeat watchdog. I think we need to check if we need to restart any heartbeat watchdogs when LLM changes from "not connected" to "connected". |
||
| } | ||
|
|
||
| const backoffTime = this.getWeightedBackoffTime(dataSet.backoff); | ||
| const delay = heartbeatIntervalMs + backoffTime; | ||
|
|
||
| dataSet.heartbeatWatchdogTimer = setTimeout(() => { | ||
| dataSet.heartbeatWatchdogTimer = undefined; | ||
|
|
||
| LoggerProxy.logger.warn( | ||
| `HashTreeParser#resetHeartbeatWatchdogs --> ${this.debugId} Heartbeat watchdog fired for data set "${dataSet.name}" - no heartbeat received within expected interval, initiating sync` | ||
| `HashTreeParser#resetHeartbeatWatchdogs --> ${this.debugId} Heartbeat watchdog fired for data set "${dataSet.name}" - no heartbeat received within expected interval(heartbeatIntervalMs=${heartbeatIntervalMs}, backoffTime=${backoffTime}), initiating sync` | ||
| ); | ||
|
|
||
| Metrics.sendBehavioralMetric(BEHAVIORAL_METRICS.HASH_TREE_HEARTBEAT_WATCHDOG_EXPIRED, { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -568,6 +568,7 @@ export default class LocusInfo extends EventsScope { | |
| callbacks: { | ||
| locusInfoUpdateCallback: this.updateFromHashTree.bind(this, locusUrl), | ||
| syncLatencyTracker: this.callbacks.syncLatencyTracker, | ||
| isLlmExpected: () => this.parsedLocus.self?.joinedWith?.state === 'JOINED', | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When the LLM websocket drops while the meeting is still joined (for example, connect failed or the socket is reconnecting), Useful? React with 👍 / 👎. |
||
| // Reuse webex-core's tracking-id interceptor sequence (exposed publicly via | ||
| // webexTrackingIdSequenceNumbers) so Locus requests share the client's unified | ||
| // ${sessionId}_${sequence} tracking id space instead of minting an unrelated id. Fall | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When LLM becomes not expected after an LLM dataset watchdog has already been scheduled, no further LLM dataset message is guaranteed to arrive and re-enter this guard, so the existing timeout still fires and enqueues
/syncbefore the guard is evaluated again. In that pathperformSync()also restarts the root-hash sync timer, so the no-LLM case can continue issuing syncs; the expectation check needs to run in the timeout/enqueue path or LLM timers need to be cleared when the meeting stops expecting LLM.Useful? React with 👍 / 👎.