Skip to content

Commit

Permalink
[bug] Decrease repeatInterval by 1% to avoid race condition if timeou…
Browse files Browse the repository at this point in the history
…t=repeatInterval; Lock repeatInterval in redis for cluster; For bug 70419
  • Loading branch information
konovalovsergey committed Sep 25, 2024
1 parent 34e5c51 commit b46e0ab
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 31 deletions.
35 changes: 20 additions & 15 deletions Common/sources/notificationService.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,11 @@ const mailService = require('./mailService');
const cfgMailServer = config.get('email.smtpServerConfiguration');
const cfgMailMessageDefaults = config.get('email.contactDefaults');
const cfgNotificationEnable = config.get('notification.enable');
const cfgEditorDataStorage = config.get('services.CoAuthoring.server.editorDataStorage');
const cfgEditorStatStorage = config.get('services.CoAuthoring.server.editorStatStorage');
const editorStatStorage = require('./../../DocService/sources/' + (cfgEditorStatStorage || cfgEditorDataStorage));

const infiniteRepeatInterval = Infinity;
const repeatIntervalsExpired = new Map();
const editorStat = editorStatStorage.EditorStat ? new editorStatStorage.EditorStat() : new editorStatStorage();
const notificationTypes = {
LICENSE_EXPIRATION_WARNING: 'licenseExpirationWarning',
LICENSE_LIMIT: 'licenseLimit'
Expand Down Expand Up @@ -111,24 +113,27 @@ async function notify(ctx, notificationType, messageParams) {
ctx.logger.debug('Notification service: notify "%s"', notificationType);

const tenRule = ctx.getCfg(`notification.rules.${notificationType}`, config.get(`notification.rules.${notificationType}`));
if (tenRule && checkRulePolicies(ctx, notificationType, tenRule)) {
await notifyRule(ctx, tenRule, messageParams);
if (tenRule) {
let checkRes = await checkRulePolicies(ctx, notificationType, tenRule);
if (checkRes) {
await notifyRule(ctx, tenRule, messageParams);
}
}
}

function checkRulePolicies(ctx, notificationType, tenRule) {
async function checkRulePolicies(ctx, notificationType, tenRule) {
const { repeatInterval } = tenRule.policies;
const intervalMilliseconds = repeatInterval ? ms(repeatInterval) : infiniteRepeatInterval;
const cacheKey = `${notificationType}_${ctx.tenant}`;
const expired = repeatIntervalsExpired.get(cacheKey);

if (!expired || expired <= Date.now()) {
repeatIntervalsExpired.set(cacheKey, Date.now() + intervalMilliseconds);
return true;
//decrease repeatInterval by 1% to avoid race condition if timeout=repeatInterval
let ttl = Math.floor(ms(repeatInterval) * 0.99 / 1000);
let isLock = false;
//todo for compatibility remove if after 8.2
if (editorStat?.lockNotification) {
isLock = await editorStat.lockNotification(ctx, notificationType, ttl);
}

ctx.logger.debug(`Notification service: skip rule "%s" due to repeat interval = %s`, notificationType, repeatInterval ?? "infinite");
return false;
if (!isLock) {
ctx.logger.debug(`Notification service: skip rule "%s" due to repeat interval = %s`, notificationType, repeatInterval);
}
return isLock;
}

async function notifyRule(ctx, tenRule, messageParams) {
Expand Down
4 changes: 2 additions & 2 deletions DocService/sources/DocsCoServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -3988,7 +3988,7 @@ exports.install = function(server, callbackFunction) {
exports.setLicenseInfo = async function(globalCtx, data, original) {
tenantManager.setDefLicense(data, original);

utilsDocService.notifyLicenseExpiration(globalCtx, data.endDate);
await utilsDocService.notifyLicenseExpiration(globalCtx, data.endDate);

const tenantsList = await tenantManager.getAllTenants(globalCtx);
for (const tenant of tenantsList) {
Expand All @@ -3997,7 +3997,7 @@ exports.setLicenseInfo = async function(globalCtx, data, original) {
await ctx.initTenantCache();

const [licenseInfo] = await tenantManager.getTenantLicense(ctx);
utilsDocService.notifyLicenseExpiration(ctx, licenseInfo.endDate);
await utilsDocService.notifyLicenseExpiration(ctx, licenseInfo.endDate);
}
};
exports.healthCheck = function(req, res) {
Expand Down
26 changes: 15 additions & 11 deletions DocService/sources/editorDataMemory.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const tenantManager = require('./../../Common/sources/tenantManager');
const cfgExpMonthUniqueUsers = ms(config.get('services.CoAuthoring.expire.monthUniqueUsers'));

function EditorCommon() {
this.data = {};
}
EditorCommon.prototype.connect = async function () {};
EditorCommon.prototype.isConnected = function() {
Expand All @@ -54,15 +55,7 @@ EditorCommon.prototype.healthCheck = async function() {
}
return false;
};

function EditorData() {
EditorCommon.call(this);
this.data = {};
this.forceSaveTimer = {};
}
EditorData.prototype = Object.create(EditorCommon.prototype);
EditorData.prototype.constructor = EditorData;
EditorData.prototype._getDocumentData = function(ctx, docId) {
EditorCommon.prototype._getDocumentData = function(ctx, docId) {
let tenantData = this.data[ctx.tenant];
if (!tenantData) {
this.data[ctx.tenant] = tenantData = {};
Expand All @@ -73,7 +66,7 @@ EditorData.prototype._getDocumentData = function(ctx, docId) {
}
return options;
};
EditorData.prototype._checkAndLock = function(ctx, name, docId, fencingToken, ttl) {
EditorCommon.prototype._checkAndLock = function(ctx, name, docId, fencingToken, ttl) {
let data = this._getDocumentData(ctx, docId);
const now = Date.now();
let res = true;
Expand All @@ -85,7 +78,7 @@ EditorData.prototype._checkAndLock = function(ctx, name, docId, fencingToken, tt
}
return res;
};
EditorData.prototype._checkAndUnlock = function(ctx, name, docId, fencingToken) {
EditorCommon.prototype._checkAndUnlock = function(ctx, name, docId, fencingToken) {
let data = this._getDocumentData(ctx, docId);
const now = Date.now();
let res;
Expand All @@ -103,6 +96,13 @@ EditorData.prototype._checkAndUnlock = function(ctx, name, docId, fencingToken)
return res;
};

function EditorData() {
EditorCommon.call(this);
this.forceSaveTimer = {};
}
EditorData.prototype = Object.create(EditorCommon.prototype);
EditorData.prototype.constructor = EditorData;

EditorData.prototype.addPresence = async function(ctx, docId, userId, userInfo) {};
EditorData.prototype.updatePresence = async function(ctx, docId, userId) {};
EditorData.prototype.removePresence = async function(ctx, docId, userId) {};
Expand Down Expand Up @@ -497,6 +497,10 @@ EditorStat.prototype.getLicense = async function(key) {
EditorStat.prototype.removeLicense = async function(key) {
delete this.license[key];
};
EditorStat.prototype.lockNotification = async function(ctx, notificationType, ttl) {
//true NaN !== NaN
return this._checkAndLock(ctx, notificationType, notificationType, NaN, ttl);
};

module.exports = {
EditorData,
Expand Down
6 changes: 3 additions & 3 deletions DocService/sources/utilsDocService.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,11 @@ function humanFriendlyExpirationTime(endTime) {

/**
* Notify server user about license expiration via configured notification transports.
* @param {string} ctx Context.
* @param {Context} ctx Context.
* @param {Date} endDate Date of expiration.
* @returns {undefined}
*/
function notifyLicenseExpiration(ctx, endDate) {
async function notifyLicenseExpiration(ctx, endDate) {
if (!endDate) {
ctx.logger.warn('notifyLicenseExpiration(): expiration date is not defined');
return;
Expand All @@ -127,7 +127,7 @@ function notifyLicenseExpiration(ctx, endDate) {

const state = endDate < currentDate ? 'expired' : 'expires';
ctx.logger.warn('%s license %s on %s!!!', tenant, state, formattedExpirationTime);
notificationService.notify(ctx, notificationTypes.LICENSE_EXPIRATION_WARNING, [tenant, state, formattedExpirationTime]);
await notificationService.notify(ctx, notificationTypes.LICENSE_EXPIRATION_WARNING, [tenant, state, formattedExpirationTime]);
}
}

Expand Down

0 comments on commit b46e0ab

Please sign in to comment.