Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions packages/@webex/plugin-meetings/src/hashTree/hashTreeParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ export type HashTreeParserCallbacks = {
locusInfoUpdateCallback: LocusInfoUpdateCallback;
syncLatencyTracker?: SyncLatencyTracker;
generateTrackingId?: GenerateTrackingId;
isLlmConnected?: () => boolean;
};

const SYNC_METRICS_DATA_SETS = [
Expand Down Expand Up @@ -1837,6 +1838,19 @@ class HashTreeParser {
continue;
}

if (
dataSet.name === DataSetNames.MAIN &&

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we sure it's only MAIN dataset? what about others datasets that are sent over LLM like atd-unmuted, atd-active?
We already have LLM_DATASET_NAMES defined that contains them, maybe it should be used here?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Cover all LLM-backed watchdogs

When LLM is disconnected in a webinar/panelist meeting where atd-active or atd-unmuted is visible, this DataSetNames.MAIN guard leaves their watchdogs armed even though those datasets are also LLM-delivered (LLM_DATASET_NAMES includes them). Their timers will still expire and re-enqueue /sync repeatedly while the channel is down, so the no-LLM suppression only fixes main; use the LLM dataset set here instead of the single dataset check.

Useful? React with 👍 / 👎.

this.callbacks.isLlmConnected &&
!this.callbacks.isLlmConnected()
) {
Comment on lines +1841 to +1845

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Suppress already-armed LLM watchdogs

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 /sync before the guard is evaluated again. In that path performSync() 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 👍 / 👎.

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;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The 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;

Expand Down
26 changes: 26 additions & 0 deletions packages/@webex/plugin-meetings/src/locus-info/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
_SPACE_SHARE_,
LOCUSINFO,
LOCUS,
LLM_DEFAULT_SESSION,
_LEFT_,
MEETING_REMOVED_REASON,
CALL_REMOVED_REASON,
Expand Down Expand Up @@ -568,6 +569,31 @@ export default class LocusInfo extends EventsScope {
callbacks: {
locusInfoUpdateCallback: this.updateFromHashTree.bind(this, locusUrl),
syncLatencyTracker: this.callbacks.syncLatencyTracker,
isLlmConnected: () => {
const llm = this.webex?.internal?.llm;

if (!llm?.isConnected?.()) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as discussed 1-1 on webex, we'll need to change this to check if we expect LLM to be connected instead of checking if it is actually connected

return false;
}

const ownership = llm.resolveSessionOwnership?.(this.meetingId, LLM_DEFAULT_SESSION);
Comment thread
marcin-bazyl marked this conversation as resolved.
Outdated

if (
ownership?.currentOwner &&
ownership.currentOwner !== this.meetingId &&

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you sure we need this check to have all these conditions? isn't checking isOwner enough? the checks here look the same like what is already done inside resolveSessionOwnership()

!ownership.isOwner
) {
return false;
}

const connectedLocusUrl = llm.getLocusUrl?.();

if (connectedLocusUrl && connectedLocusUrl !== locusUrl) {
return false;
Comment thread
marcin-bazyl marked this conversation as resolved.
Outdated
}

return true;
},
// 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,8 @@ describe('HashTreeParser', () => {
excludedDataSets?: string[],
syncLatencyTracker?: any,
syncLatencyMeetingId = 'meeting-1',
generateTrackingId?: any
generateTrackingId?: any,
isLlmConnected?: () => boolean
) {
return new HashTreeParser({
initialLocus,
Expand All @@ -190,6 +191,7 @@ describe('HashTreeParser', () => {
locusInfoUpdateCallback: callback,
syncLatencyTracker,
generateTrackingId,
isLlmConnected,
},
debugId: 'test',
excludedDataSets,
Expand Down Expand Up @@ -3403,6 +3405,39 @@ describe('HashTreeParser', () => {
expect(parser.dataSets.main.heartbeatWatchdogTimer).to.be.undefined;
});

it('skips watchdog timer for main only when current meeting LLM is disconnected', async () => {
const parser = createHashTreeParser(
undefined,
undefined,
undefined,
undefined,
'meeting-1',
undefined,
() => false
);

const heartbeatMessage = {
dataSets: [
{
...createDataSet('main', 16, 1100),
root: parser.dataSets.main.hashTree.getRootHash(),
},
{
...createDataSet('self', 1, 2100),
url: parser.dataSets.self.url,
root: parser.dataSets.self.hashTree.getRootHash(),
},
],
visibleDataSetsUrl,
locusUrl,
};

parser.handleMessage(heartbeatMessage, 'heartbeat with meeting-scoped llm disconnected');

expect(parser.dataSets.main.heartbeatWatchdogTimer).to.be.undefined;
expect(parser.dataSets.self.heartbeatWatchdogTimer).to.not.be.undefined;
});

it('stops all watchdog timers when meeting ends via sentinel message', async () => {
const parser = createHashTreeParser();

Expand Down
Loading