mirror of
https://github.com/ether/etherpad-lite.git
synced 2026-07-18 17:13:58 +00:00
perf: don't log settings.loadTest warning per-message (#7756)
CPU profile of develop (and of the open #7775 branch) at the 100-400 author dive sweep attributed ~4% of total process CPU to log4js inside SecurityManager.checkAccess. Tracing the actual log call: line 79-80 emits `console.warn('bypassing socket.io authentication...')` on every checkAccess invocation when settings.loadTest is true — once per inbound message. With log4js's replaceConsole + cluster-mode dispatch enabled, that warning allocated, formatted, and dispatched a LogEvent through sendToListeners -> sendLogEventToAppender for every CLIENT_READY, COMMIT_CHANGESET, USERINFO_UPDATE, etc. settings.loadTest is a configuration choice, not a per-request condition. The warning belongs at startup. Move it to Settings.ts init alongside the other "you set X, beware" warnings, and drop the per-message branch (the loadTest short-circuit still applies). Test plan: - tests/backend/specs/api/sessionsAndGroups.ts: 32 passing - tests/backend/specs/socketio.ts: 39 passing (handleMessage paths) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
79f525b0a7
commit
620c8b20cd
2 changed files with 13 additions and 4 deletions
|
|
@ -75,10 +75,13 @@ exports.checkAccess = async (padID:string, sessionCookie:string, token:string, u
|
|||
}
|
||||
|
||||
// Authentication and authorization checks.
|
||||
if (settings.loadTest) {
|
||||
console.warn(
|
||||
'bypassing socket.io authentication and authorization checks due to settings.loadTest');
|
||||
} else if (settings.requireAuthentication) {
|
||||
// settings.loadTest just short-circuits authn/authz; the user-facing
|
||||
// warning about this configuration choice fires once at startup, not
|
||||
// per request (see Settings.ts). Re-logging it here was costing
|
||||
// ~4% of process CPU in the 100-400 author dive sweep (#7756): the
|
||||
// routed-console-warn went through log4js's clustering dispatch on
|
||||
// every message.
|
||||
if (!settings.loadTest && settings.requireAuthentication) {
|
||||
if (userSettings == null) {
|
||||
authLogger.debug('access denied: authentication is required');
|
||||
return DENY;
|
||||
|
|
|
|||
|
|
@ -1193,6 +1193,12 @@ export const reloadSettings = () => {
|
|||
logger.warn("logLayoutType: " + settings.logLayoutType);
|
||||
initLogging(settings.logconfig);
|
||||
|
||||
if (settings.loadTest) {
|
||||
logger.warn(
|
||||
'settings.loadTest is true: socket.io authentication and authorization checks ' +
|
||||
'will be bypassed for every connection. Do NOT enable this in production.');
|
||||
}
|
||||
|
||||
if (!settings.skinName) {
|
||||
logger.warn('No "skinName" parameter found. Please check out settings.json.template and ' +
|
||||
'update your settings.json. Falling back to the default "colibris".');
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue