Skip to content
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

Request batch when idle #7526

Merged
merged 35 commits into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
13a074f
WIP
akhenry Feb 10, 2024
b8c2a4c
WIP
akhenry Feb 23, 2024
e618338
Added comment
akhenry Feb 27, 2024
25e5754
Restored dynamic column sizing
akhenry Feb 28, 2024
8336efd
Added aria labels
akhenry Feb 28, 2024
eebbf29
Cleaned up debug logging
akhenry Feb 28, 2024
c3b8801
Restore table column sizing
akhenry Feb 28, 2024
b0e38a0
Remove resize polling
akhenry Feb 28, 2024
5aa7dc4
Throttle on leading edge rather than trailing edge to align with tele…
akhenry Feb 29, 2024
83e6da3
Restore initial call to updateVisibleRows
akhenry Feb 29, 2024
3b8df58
Make sure rescale happens at least once on initialization
akhenry Feb 29, 2024
df30c3d
Update comment to more effectively sass Safari
akhenry Feb 29, 2024
5b8e138
Remove redundant code
akhenry Feb 29, 2024
20316d9
Remove debugging code
akhenry Feb 29, 2024
03367e4
Remove more unused code
akhenry Feb 29, 2024
e5d61fd
Use resize observer for time axis, not polling
akhenry Feb 29, 2024
851feae
Debounce resize observer for tables
akhenry Feb 29, 2024
b356dca
Remove hard-coded batch wait and make it configurable
akhenry Feb 29, 2024
b24ee7c
Fixing linting errors
akhenry Feb 29, 2024
d55b163
Merge branch 'master' into request-batch-when-idle
akhenry Feb 29, 2024
7a921a4
Remove debugging code
akhenry Mar 1, 2024
385cbea
Fix prettier problem
akhenry Mar 4, 2024
d648046
Remove commented code
akhenry Mar 4, 2024
34b0bf9
Remove debugging code
akhenry Mar 4, 2024
8cdd161
Remove dead code
akhenry Mar 4, 2024
73584b4
Added websocket reconnect event
akhenry Mar 5, 2024
092f496
Merge branch 'master' into request-batch-when-idle
akhenry Mar 5, 2024
5f6b8e3
Fixed linting errors in master
akhenry Mar 5, 2024
b4e7c13
Wait for first batch to be returned before enforcing batch sizes
akhenry Mar 8, 2024
e91d005
Remove debugging code
akhenry Mar 8, 2024
a71d122
Merge branch 'master' into request-batch-when-idle
akhenry Mar 8, 2024
c40e2bc
Merged conflict
akhenry Mar 12, 2024
7128b02
Updated test for aria label change
akhenry Mar 12, 2024
445ca17
Fixed failing test
akhenry Mar 12, 2024
a006dce
Fixed failing test
akhenry Mar 12, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ test.describe('Display Layout', () => {
expect(trimmedDisplayValue).toBe(formattedTelemetryValue);

// ensure we can right click on the alpha-numeric widget and view historical data
await page.getByLabel('Sine', { exact: true }).click({
await page.getByLabel(/Alpha-numeric telemetry value of.*/).click({
button: 'right'
});
await page.getByLabel('View Historical Data').click();
Expand Down
4 changes: 3 additions & 1 deletion e2e/tests/functional/plugins/plot/previews.e2e.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ test.describe('Plots work in Previews', () => {
await page.getByRole('listitem', { name: 'Save and Finish Editing' }).click();

// right click on the plot and select view large
await page.getByLabel('Sine', { exact: true }).click({ button: 'right' });
await page.getByLabel(/Alpha-numeric telemetry value of.*/).click({
button: 'right'
});
await page.getByLabel('View Historical Data').click();
await expect(page.getByLabel('Preview Container').getByLabel('Plot Canvas')).toBeVisible();
await page.getByRole('button', { name: 'Close' }).click();
Expand Down
2 changes: 1 addition & 1 deletion e2e/tests/functional/tooltips.e2e.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ test.describe('Verify tooltips', () => {

async function getToolTip(object) {
await page.locator('.c-create-button').hover();
await page.getByRole('cell', { name: object.name }).hover();
await page.getByLabel('lad name').getByText(object.name).hover();
let tooltipText = await page.locator('.c-tooltip').textContent();
return tooltipText.replace('\n', '').trim();
}
Expand Down
98 changes: 82 additions & 16 deletions src/api/telemetry/BatchingWebSocket.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
* at runtime from the About dialog for additional information.
*****************************************************************************/
import installWorker from './WebSocketWorker.js';
const DEFAULT_RATE_MS = 1000;
/**
* Describes the strategy to be used when batching WebSocket messages
*
Expand Down Expand Up @@ -51,11 +50,21 @@
*
* @memberof module:openmct.telemetry
*/
// Shim for Internet Explorer, I mean Safari. It doesn't support requestIdleCallback, but it's in a tech preview, so it will be dropping soon.
const requestIdleCallback =
// eslint-disable-next-line compat/compat
window.requestIdleCallback ?? ((fn, { timeout }) => setTimeout(fn, timeout));
const ONE_SECOND = 1000;
const FIVE_SECONDS = 5 * ONE_SECOND;

class BatchingWebSocket extends EventTarget {
#worker;
#openmct;
#showingRateLimitNotification;
#rate;
#maxBatchSize;
#applicationIsInitializing;
#maxBatchWait;
#firstBatchReceived;

constructor(openmct) {
super();
Expand All @@ -66,7 +75,10 @@
this.#worker = new Worker(workerUrl);
this.#openmct = openmct;
this.#showingRateLimitNotification = false;
this.#rate = DEFAULT_RATE_MS;
this.#maxBatchSize = Number.POSITIVE_INFINITY;
this.#maxBatchWait = ONE_SECOND;
this.#applicationIsInitializing = true;
this.#firstBatchReceived = false;

Check warning on line 81 in src/api/telemetry/BatchingWebSocket.js

View check run for this annotation

Codecov / codecov/patch

src/api/telemetry/BatchingWebSocket.js#L78-L81

Added lines #L78 - L81 were not covered by tests

const routeMessageToHandler = this.#routeMessageToHandler.bind(this);
this.#worker.addEventListener('message', routeMessageToHandler);
Expand All @@ -78,6 +90,20 @@
},
{ once: true }
);

openmct.once('start', () => {

Check warning on line 94 in src/api/telemetry/BatchingWebSocket.js

View check run for this annotation

Codecov / codecov/patch

src/api/telemetry/BatchingWebSocket.js#L94

Added line #L94 was not covered by tests
// An idle callback is a pretty good indication that a complex display is done loading. At that point set the batch size more conservatively.
// Force it after 5 seconds if it hasn't happened yet.
requestIdleCallback(

Check warning on line 97 in src/api/telemetry/BatchingWebSocket.js

View check run for this annotation

Codecov / codecov/patch

src/api/telemetry/BatchingWebSocket.js#L97

Added line #L97 was not covered by tests
() => {
this.#applicationIsInitializing = false;
this.setMaxBatchSize(this.#maxBatchSize);

Check warning on line 100 in src/api/telemetry/BatchingWebSocket.js

View check run for this annotation

Codecov / codecov/patch

src/api/telemetry/BatchingWebSocket.js#L99-L100

Added lines #L99 - L100 were not covered by tests
},
{
timeout: FIVE_SECONDS
}
);
});
}

/**
Expand Down Expand Up @@ -129,14 +155,6 @@
});
}

/**
* When using batching, sets the rate at which batches of messages are released.
* @param {Number} rate the amount of time to wait, in ms, between batches.
*/
setRate(rate) {
this.#rate = rate;
}

/**
* @param {Number} maxBatchSize the maximum length of a batch of messages. For example,
* the maximum number of telemetry values to batch before dropping them
Expand All @@ -151,12 +169,29 @@
* 15 would probably be a better batch size.
*/
setMaxBatchSize(maxBatchSize) {
this.#maxBatchSize = maxBatchSize;
if (!this.#applicationIsInitializing) {
this.#sendMaxBatchSizeToWorker(this.#maxBatchSize);

Check warning on line 174 in src/api/telemetry/BatchingWebSocket.js

View check run for this annotation

Codecov / codecov/patch

src/api/telemetry/BatchingWebSocket.js#L172-L174

Added lines #L172 - L174 were not covered by tests
}
}
setMaxBatchWait(wait) {
this.#maxBatchWait = wait;
this.#sendBatchWaitToWorker(this.#maxBatchWait);

Check warning on line 179 in src/api/telemetry/BatchingWebSocket.js

View check run for this annotation

Codecov / codecov/patch

src/api/telemetry/BatchingWebSocket.js#L178-L179

Added lines #L178 - L179 were not covered by tests
}
#sendMaxBatchSizeToWorker(maxBatchSize) {
this.#worker.postMessage({
type: 'setMaxBatchSize',
maxBatchSize
});
}

#sendBatchWaitToWorker(maxBatchWait) {
this.#worker.postMessage({

Check warning on line 189 in src/api/telemetry/BatchingWebSocket.js

View check run for this annotation

Codecov / codecov/patch

src/api/telemetry/BatchingWebSocket.js#L189

Added line #L189 was not covered by tests
type: 'setMaxBatchWait',
maxBatchWait
});
}

/**
* Disconnect the associated WebSocket. Generally speaking there is no need to call
* this manually.
Expand All @@ -169,7 +204,9 @@

#routeMessageToHandler(message) {
if (message.data.type === 'batch') {
if (message.data.batch.dropped === true && !this.#showingRateLimitNotification) {
this.start = Date.now();
const batch = message.data.batch;
if (batch.dropped === true && !this.#showingRateLimitNotification) {

Check warning on line 209 in src/api/telemetry/BatchingWebSocket.js

View check run for this annotation

Codecov / codecov/patch

src/api/telemetry/BatchingWebSocket.js#L207-L209

Added lines #L207 - L209 were not covered by tests
const notification = this.#openmct.notifications.alert(
'Telemetry dropped due to client rate limiting.',
{ hint: 'Refresh individual telemetry views to retrieve dropped telemetry if needed.' }
Expand All @@ -179,16 +216,45 @@
this.#showingRateLimitNotification = false;
});
}
this.dispatchEvent(new CustomEvent('batch', { detail: message.data.batch }));
setTimeout(() => {
this.#readyForNextBatch();
}, this.#rate);

this.dispatchEvent(new CustomEvent('batch', { detail: batch }));
this.#waitUntilIdleAndRequestNextBatch(batch);

Check warning on line 221 in src/api/telemetry/BatchingWebSocket.js

View check run for this annotation

Codecov / codecov/patch

src/api/telemetry/BatchingWebSocket.js#L220-L221

Added lines #L220 - L221 were not covered by tests
} else if (message.data.type === 'message') {
this.dispatchEvent(new CustomEvent('message', { detail: message.data.message }));
} else if (message.data.type === 'reconnected') {
this.dispatchEvent(new CustomEvent('reconnected'));

Check warning on line 225 in src/api/telemetry/BatchingWebSocket.js

View check run for this annotation

Codecov / codecov/patch

src/api/telemetry/BatchingWebSocket.js#L224-L225

Added lines #L224 - L225 were not covered by tests
} else {
throw new Error(`Unknown message type: ${message.data.type}`);
}
}

#waitUntilIdleAndRequestNextBatch(batch) {
requestIdleCallback(

Check warning on line 232 in src/api/telemetry/BatchingWebSocket.js

View check run for this annotation

Codecov / codecov/patch

src/api/telemetry/BatchingWebSocket.js#L232

Added line #L232 was not covered by tests
(state) => {
if (this.#firstBatchReceived === false) {
this.#firstBatchReceived = true;

Check warning on line 235 in src/api/telemetry/BatchingWebSocket.js

View check run for this annotation

Codecov / codecov/patch

src/api/telemetry/BatchingWebSocket.js#L234-L235

Added lines #L234 - L235 were not covered by tests
}
const now = Date.now();
const waitedFor = now - this.start;
if (state.didTimeout === true) {
if (document.visibilityState === 'visible') {
console.warn(`Event loop is too busy to process batch.`);
this.#waitUntilIdleAndRequestNextBatch(batch);

Check warning on line 242 in src/api/telemetry/BatchingWebSocket.js

View check run for this annotation

Codecov / codecov/patch

src/api/telemetry/BatchingWebSocket.js#L237-L242

Added lines #L237 - L242 were not covered by tests
} else {
// After ingesting a telemetry batch, wait until the event loop is idle again before
// informing the worker we are ready for another batch.
this.#readyForNextBatch();

Check warning on line 246 in src/api/telemetry/BatchingWebSocket.js

View check run for this annotation

Codecov / codecov/patch

src/api/telemetry/BatchingWebSocket.js#L246

Added line #L246 was not covered by tests
}
} else {
if (waitedFor > ONE_SECOND) {
console.warn(`Warning, batch processing took ${waitedFor}ms`);

Check warning on line 250 in src/api/telemetry/BatchingWebSocket.js

View check run for this annotation

Codecov / codecov/patch

src/api/telemetry/BatchingWebSocket.js#L249-L250

Added lines #L249 - L250 were not covered by tests
}
this.#readyForNextBatch();

Check warning on line 252 in src/api/telemetry/BatchingWebSocket.js

View check run for this annotation

Codecov / codecov/patch

src/api/telemetry/BatchingWebSocket.js#L252

Added line #L252 was not covered by tests
}
},
{ timeout: ONE_SECOND }
);
}
}

export default BatchingWebSocket;
7 changes: 6 additions & 1 deletion src/api/telemetry/TelemetryAPI.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ const SUBSCRIBE_STRATEGY = {
export default class TelemetryAPI {
#isGreedyLAD;
#subscribeCache;
#hasReturnedFirstData;

get SUBSCRIBE_STRATEGY() {
return SUBSCRIBE_STRATEGY;
Expand All @@ -108,6 +109,7 @@ export default class TelemetryAPI {
this.#isGreedyLAD = true;
this.BatchingWebSocket = BatchingWebSocket;
this.#subscribeCache = {};
this.#hasReturnedFirstData = false;
}

abortAllRequests() {
Expand Down Expand Up @@ -383,7 +385,10 @@ export default class TelemetryAPI {
arguments[1] = await this.applyRequestInterceptors(domainObject, arguments[1]);
try {
const telemetry = await provider.request(...arguments);

if (!this.#hasReturnedFirstData) {
this.#hasReturnedFirstData = true;
performance.mark('firstHistoricalDataReturned');
}
return telemetry;
} catch (error) {
if (error.name !== 'AbortError') {
Expand Down
Loading
Loading