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

enhancement/resource-performance-and-memory #615

Open
wants to merge 7 commits into
base: enhancement/websockets
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 2 additions & 0 deletions .env.sample
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ POOL_DESTROY_TIMEOUT = 5000
POOL_IDLE_TIMEOUT = 30000
POOL_CREATE_RETRY_INTERVAL = 200
POOL_REAPER_INTERVAL = 1000
POOL_RESOURCES_INTERVAL = 30000
POOL_BENCHMARKING = false

# LOGGING CONFIG
Expand All @@ -98,6 +99,7 @@ OTHER_LISTEN_TO_PROCESS_EXITS = true
OTHER_NO_LOGO = false
OTHER_HARD_RESET_PAGE = false
OTHER_BROWSER_SHELL_MODE = true
OTHER_CONNECTION_OVER_PIPE = false
OTHER_VALIDATION = true

# DEBUG CONFIG
Expand Down
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ _New Features:_
- Added the `validateOption` function for validating a single option. It is used in the code to validate individual options (`svg`, `instr`, `resources`, `customCode`, `callback`, `globalOptions`, and `themeOptions`) loaded from a file.
- Added the `validateOptions` function for validating the full set of options. It is used in the code to validate options coming from functions that update global options, CLI arguments, configurations loaded via `--loadConfig`, and configurations created using the prompts functionality.
- Introduced redefined `getOptions` and `updateOptions` functions to retrieve and update the original global options or a copy of global options, allowing flexibility in export scenarios.
- Added a toggleable mechanism to ensure the correct number of resources exist at any given time on an interval, creating them if necessary.
- Added the `OTHER_CONNECTION_OVER_PIPE` option for a more stable and optimized connection to the browser using a `Pipe` instead of `WebSockets`.
- Added an optional mechanism to reconnect the browser in case of an unexpected disconnect (only applicable for `WebSocket` connections).
- Added an optional health-checking mechanism for resource pages, with automatic recreation if needed (only applicable for `WebSocket` connections).
- Introduced the ability to enable a customizable `WebSocket` connection between the Export Server instance and any server or service that supports such connections to collect chart options usage data. This is useful for gathering telemetry data.
- Added a simple filtering mechanism (based on the `./lib/schemas/telemetry.json` file) to control the data being sent.
- Added a new option called `uploadLimit` to control the maximum size of a request's payload body.
Expand Down Expand Up @@ -56,6 +60,7 @@ _Enhancements:_
- Created `_checkDataSize` for handling the data size validation.
- Optimized `initExport`, utilizing `updateOptions` for global option updates.
- The `initExport` now have its options parameters defaulted to an empty object, using global option values if none are provided.
- Various optimizations and performance improvements in functions like `createBrowser`, `killPool`, and `shutdownCleanUp` to enhance performance and address memory issues.
- Updated exported API functions for module usage.
- Adjusted imports to get functions from corresponding modules rather than `index.js`.
- Server functions are now directly exported (rather than within a `server` object) as API functions.
Expand All @@ -75,6 +80,7 @@ _Enhancements:_
- Refactored and split the `_updateCache` function into smaller, more manageable parts.
- The `updateHcVersion` function now correctly updates the global options.
- The new `_configureRequest` function is responsible for setting the proxy agent.
- Improved logging with more detailed information about rejected requests at an early stage (in the `validation` middleware).
- Passing `version` as the first argument to `_saveConfigToManifest`.
- Removed the `fetchAndProcessScript` and `fetchScripts` functions, replacing them with a single function, `_fetchScript`, for the same purpose.
- Renamed the `checkAndUpdateCache` function to `checkCache`, the `updateVersion` function to `updateHcVersion`, the `version` function to `getHcVersion`, the `saveConfigToManifest` function to `_saveConfigToManifest`, the `extractModuleName` function to `_extractModuleName`, the `extractVersion` function to `extractHcVersion`, and the `cdnURL` property to `cdnUrl` in the `cache.js` module.
Expand Down
149 changes: 140 additions & 9 deletions lib/browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ import { getCachePath } from './cache.js';
import { getOptions } from './config.js';
import { setupHighcharts } from './highcharts.js';
import { log, logWithStack } from './logger.js';
import { __dirname, getAbsolutePath } from './utils.js';
import { __dirname, expBackoff, getAbsolutePath } from './utils.js';
import { envs } from './validation.js';

import ExportError from './errors/ExportError.js';

Expand All @@ -42,6 +43,9 @@ const pageTemplate = readFileSync(
// To save the browser
let browser = null;

// To save the WebSocket endpoint in case of a sudden disconnect
let wsEndpoint;

/**
* Retrieves the existing Puppeteer browser instance.
*
Expand Down Expand Up @@ -87,6 +91,8 @@ export async function createBrowser(puppeteerArgs) {
headless: other.browserShellMode ? 'shell' : true,
userDataDir: 'tmp',
args: puppeteerArgs || [],
// Must be disabled for debugging to work
pipe: envs.OTHER_CONNECTION_OVER_PIPE,
handleSIGINT: false,
handleSIGTERM: false,
handleSIGHUP: false,
Expand All @@ -108,6 +114,23 @@ export async function createBrowser(puppeteerArgs) {

// Launch the browser
browser = await puppeteer.launch(launchOptions);

// Close the initial pages if any found
const pages = await browser.pages();
if (pages) {
for (const page of pages) {
await page.close();
}
}

// Only for the WebSocket connection
if (!launchOptions.pipe) {
// Save WebSocket endpoint
wsEndpoint = browser.wsEndpoint();

// Attach the disconnected event
browser.on('disconnected', _reconnect);
}
} catch (error) {
logWithStack(
1,
Expand Down Expand Up @@ -203,7 +226,7 @@ export async function newPage(poolResource) {
await _setPageContent(poolResource.page);

// Set page events
_setPageEvents(poolResource.page);
_setPageEvents(poolResource);

// Check if the page is correctly created
if (!poolResource.page || poolResource.page.isClosed()) {
Expand Down Expand Up @@ -431,6 +454,61 @@ export async function clearPageResources(page, injectedResources) {
}
}

/**
* Reconnects to the browser instance when it is disconnected. If the current
* browser connection is lost, it attempts to reconnect using the previous
* WebSocket endpoint. If the reconnection fails, it will try to close
* the browser and relaunch a new instance.
*
* @async
* @function _reconnect
*/
async function _reconnect() {
try {
// Start the reconnecting
log(3, `[browser] Restarting the browser connection.`);

// Try to reconnect the browser
if (browser && !browser.connected) {
browser = await puppeteer.connect({
browserWSEndpoint: wsEndpoint
});
}

// Save a new WebSocket endpoint
wsEndpoint = browser.wsEndpoint();

// Add the reconnect event again
browser.on('disconnected', _reconnect);

// Log the success message
log(3, `[browser] Browser reconnected successfully.`);
} catch (error) {
logWithStack(
1,
error,
'[browser] Could not restore the browser connection, attempting to relaunch.'
);

// Try to close the browser before relaunching
try {
await close();
} catch (error) {
logWithStack(
1,
error,
'[browser] Could not close the browser before relaunching (probably is already closed).'
);
}

// Try to relaunch the browser
await createBrowser(getOptions().puppeteer.args || []);

// Log the success message
log(3, `[browser] Browser relaunched successfully.`);
}
}

/**
* Sets the content for a Puppeteer page using a predefined template
* and additional scripts.
Expand Down Expand Up @@ -458,28 +536,81 @@ async function _setPageContent(page) {
*
* @function _setPageEvents
*
* @param {Object} page - The Puppeteer page object to which the listeners
* are being set.
* @param {Object} poolResource - The pool resource that contains `id`,
* `workCount`, and `page`.
*/
function _setPageEvents(page) {
function _setPageEvents(poolResource) {
// Get `debug` options
const { debug } = getOptions();
const { debug, pool } = getOptions();

// Set the `pageerror` listener
page.on('pageerror', async () => {
poolResource.page.on('pageerror', async () => {
// It would seem like this may fire at the same time or shortly before
// a page is closed.
if (page.isClosed()) {
if (poolResource.page.isClosed()) {
return;
}
});

// Set the `console` listener, if needed
if (debug.enable && debug.listenToConsole) {
page.on('console', (message) => {
poolResource.page.on('console', (message) => {
console.log(`[debug] ${message.text()}`);
});
}

// Add the framedetached event if the connection is over WebSocket
if (envs.OTHER_CONNECTION_OVER_PIPE === false) {
poolResource.page.on('framedetached', async (frame) => {
// Get the main frame
const mainFrame = poolResource.page.mainFrame();

// Check if a page's frame is detached and requires to be recreated
if (
frame === mainFrame &&
mainFrame.detached &&
poolResource.workCount <= pool.workLimit
) {
log(
3,
`[browser] Pool resource [${poolResource.id}] - Page's frame detached.`
);
try {
// Try to connect to a new page using exponential backoff strategy
expBackoff(
async (poolResourceId, poolResource) => {
try {
// Try to close the page with a detached frame
if (!poolResource.page.isClosed()) {
await poolResource.page.close();
}
} catch (error) {
log(
3,
`[browser] Pool resource [${poolResourceId}] - Could not close the page with a detached frame.`
);
}

// Trigger a page creation
await newPage(poolResource);
},
0,
poolResource.id,
poolResource
);
} catch (error) {
logWithStack(
3,
error,
`[browser] Pool resource [${poolResource.id}] - Could not create a new page.`
);

// Set the `workLimit` to exceeded in order to recreate the resource
poolResource.workCount = pool.workLimit + 1;
}
}
});
}
}

export default {
Expand Down
54 changes: 54 additions & 0 deletions lib/pool.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ import { v4 as uuid } from 'uuid';
import { clearPage, createBrowser, closeBrowser, newPage } from './browser.js';
import { puppeteerExport } from './export.js';
import { log, logWithStack } from './logger.js';
import { addTimer } from './timer.js';
import { getNewDateTime, measureTime } from './utils.js';
import { envs } from './validation.js';

import ExportError from './errors/ExportError.js';

Expand Down Expand Up @@ -136,6 +138,12 @@ export async function initPool(poolOptions, puppeteerArgs) {
pool.release(resource);
});

// Init the interval for checking if the minimum number of resources exist
if (envs.POOL_RESOURCES_INTERVAL) {
// Register interval for the later clearing
addTimer(_checkingResourcesInterval(envs.POOL_RESOURCES_INTERVAL));
}

log(
3,
`[pool] The pool is ready${initialResources.length ? ` with ${initialResources.length} initial resources waiting.` : '.'}`
Expand Down Expand Up @@ -168,6 +176,11 @@ export async function killPool() {
pool.release(worker.resource);
}

// Remove all attached event listeners from the pool
pool.removeAllListeners('release');
pool.removeAllListeners('destroySuccess');
pool.removeAllListeners('destroyFail');

// Destroy the pool if it is still available
if (!pool.destroyed) {
await pool.destroy();
Expand Down Expand Up @@ -610,6 +623,47 @@ function _factory(poolOptions) {
};
}

/**
* Periodically checks and ensures the minimum number of resources in the pool.
* If the total number of used, free and about to be created resources falls
* below the minimum set with the `pool.min`, it creates additional resources to
* meet the minimum requirement.
*
* @function _checkingResourcesInterval
*
* @param {number} resourceCheckInterval - The interval, in milliseconds, at
* which the pool resources are checked.
*
* @returns {NodeJS.Timeout} - Returns a timer ID that can be used to clear the
* interval later.
*/
function _checkingResourcesInterval(resourceCheckInterval) {
// Set the interval for checking the number of pool resources
return setInterval(async () => {
try {
// Get the current number of resources
let currentNumber =
pool.numUsed() + pool.numFree() + pool.numPendingCreates();

// Create missing resources
while (currentNumber++ < pool.min) {
try {
// Explicitly creating a resource
await pool._doCreate();
} catch (error) {
logWithStack(2, error, '[pool] Could not create a missing resource.');
}
}
} catch (error) {
logWithStack(
1,
error,
`[pool] Something went wrong when trying to create missing resources.`
);
}
}, resourceCheckInterval);
}

export default {
initPool,
killPool,
Expand Down
4 changes: 4 additions & 0 deletions lib/resourceRelease.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ See LICENSE file in root for details.
* proper cleanup of resources such as browser, pages, servers, and timers.
*/

import { getBrowser } from './browser.js';
import { killPool } from './pool.js';
import { clearAllTimers } from './timer.js';

Expand All @@ -35,6 +36,9 @@ import { terminateClients } from './server/webSocket.js';
* The default value is `0`.
*/
export async function shutdownCleanUp(exitCode = 0) {
// Remove all attached event listeners from the browser
getBrowser().removeAllListeners('disconnected');

// Await freeing all resources
await Promise.allSettled([
// Clear all ongoing intervals
Expand Down
13 changes: 11 additions & 2 deletions lib/server/middlewares/validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,18 @@ function requestBodyMiddleware(request, response, next) {
if (instr === null && !body.svg) {
log(
2,
`[validation] Request [${requestId}] - The request from ${
`[validation] Request [${requestId}] - The request with ID ${requestId} from ${
request.headers['x-forwarded-for'] || request.connection.remoteAddress
} was incorrect. Received payload is missing correct chart data for export: ${JSON.stringify(body)}.`
} was incorrect:
Content-Type: ${request.headers['content-type']}.
Chart constructor: ${body.constr}.
Dimensions: ${body.width}x${body.height} @ ${body.scale} scale.
Type: ${body.type}.
Is SVG set? ${typeof body.svg !== 'undefined'}.
B64? ${typeof body.b64 !== 'undefined'}.
No download? ${typeof body.noDownload !== 'undefined'}.
Received payload is missing correct chart data for export: ${JSON.stringify(body)}.
`
);

throw new ExportError(
Expand Down
Loading
Loading