5154e8d24f
ref https://linear.app/tryghost/issue/ENG-1240/race-condition-when-updating-members-last-seen-at-timestamp When members click a link in an email, Ghost updates the member's `last_seen_at` timestamp, but it should only update the timestamp if the member hasn't yet been seen in the current day (based on the publication's timezone). Currently there is a race condition present where multiple simultaneous requests from the same member (if e.g. an email link checker is following all links in an email) can cause the `last_seen_at` timestamp to be updated multiple times in the same day for the same member. These additional queries add a significant load on Ghost and its database, which can contribute to the exhaustion of the connection pool and eventually requests may time out. The primary motivation for this change is to avoid that race condition by adding a lock to the member row, checking if `last_seen_at` has already been updated in the current day, and only updating it if it hasn't. Another beneficial side-effect of this change is that it avoids locking the `labels` and `newsletters` tables, which are locked when we update the `last_seen_at` timestamp in the `members` table currently. This should improve Ghost's ability to handle a large influx of requests to redirect endpoints (confirmed with load tests), which tend to happen immediately after a publisher sends an email.
163 lines
8.5 KiB
JavaScript
163 lines
8.5 KiB
JavaScript
const {MemberPageViewEvent, MemberCommentEvent, MemberLinkClickEvent} = require('@tryghost/member-events');
|
|
const moment = require('moment-timezone');
|
|
const {IncorrectUsageError} = require('@tryghost/errors');
|
|
const {EmailOpenedEvent} = require('@tryghost/email-events');
|
|
const logging = require('@tryghost/logging');
|
|
|
|
/**
|
|
* Listen for `MemberViewEvent` to update the `member.last_seen_at` timestamp
|
|
*/
|
|
class LastSeenAtUpdater {
|
|
/**
|
|
* Initializes the event subscriber
|
|
* @param {Object} deps dependencies
|
|
* @param {Object} deps.services The list of service dependencies
|
|
* @param {any} deps.services.settingsCache The settings service
|
|
* @param {() => object} deps.getMembersApi - A function which returns an instance of members-api
|
|
* @param {any} deps.db Database connection
|
|
* @param {any} deps.events The event emitter
|
|
*/
|
|
constructor({
|
|
services: {
|
|
settingsCache
|
|
},
|
|
getMembersApi,
|
|
db,
|
|
events
|
|
}) {
|
|
if (!getMembersApi) {
|
|
throw new IncorrectUsageError({message: 'Missing option getMembersApi'});
|
|
}
|
|
|
|
this._getMembersApi = getMembersApi;
|
|
this._settingsCacheService = settingsCache;
|
|
this._db = db;
|
|
this._events = events;
|
|
}
|
|
/**
|
|
* Subscribe to events of this domainEvents service
|
|
* @param {any} domainEvents The DomainEvents service
|
|
*/
|
|
subscribe(domainEvents) {
|
|
domainEvents.subscribe(MemberPageViewEvent, async (event) => {
|
|
try {
|
|
await this.updateLastSeenAt(event.data.memberId, event.data.memberLastSeenAt, event.timestamp);
|
|
} catch (err) {
|
|
logging.error(`Error in LastSeenAtUpdater.MemberPageViewEvent listener for member ${event.data.memberId}`);
|
|
logging.error(err);
|
|
}
|
|
});
|
|
|
|
domainEvents.subscribe(MemberLinkClickEvent, async (event) => {
|
|
try {
|
|
await this.updateLastSeenAt(event.data.memberId, event.data.memberLastSeenAt, event.timestamp);
|
|
} catch (err) {
|
|
logging.error(`Error in LastSeenAtUpdater.MemberLinkClickEvent listener for member ${event.data.memberId}`);
|
|
logging.error(err);
|
|
}
|
|
});
|
|
|
|
domainEvents.subscribe(MemberCommentEvent, async (event) => {
|
|
try {
|
|
await this.updateLastCommentedAt(event.data.memberId, event.timestamp);
|
|
} catch (err) {
|
|
logging.error(`Error in LastSeenAtUpdater.MemberCommentEvent listener for member ${event.data.memberId}`);
|
|
logging.error(err);
|
|
}
|
|
});
|
|
|
|
domainEvents.subscribe(EmailOpenedEvent, async (event) => {
|
|
try {
|
|
await this.updateLastSeenAtWithoutKnownLastSeen(event.memberId, event.timestamp);
|
|
} catch (err) {
|
|
logging.error(`Error in LastSeenAtUpdater.EmailOpenedEvent listener for member ${event.memberId}, emailRecipientId ${event.emailRecipientId}`);
|
|
logging.error(err);
|
|
}
|
|
});
|
|
}
|
|
|
|
/**
|
|
* Updates the member.last_seen_at field if it wasn't updated in the current day yet (in the publication timezone)
|
|
* Example: current time is 2022-02-28 18:00:00
|
|
* - memberLastSeenAt is 2022-02-27 23:00:00, timestamp is current time, then `last_seen_at` is set to the current time
|
|
* - memberLastSeenAt is 2022-02-28 01:00:00, timestamp is current time, then `last_seen_at` isn't changed
|
|
* @param {string} memberId The id of the member to be udpated
|
|
* @param {Date} timestamp The event timestamp
|
|
*/
|
|
async updateLastSeenAtWithoutKnownLastSeen(memberId, timestamp) {
|
|
// Note: we are not using Bookshelf / member repository to prevent firing webhooks + to prevent deadlock issues
|
|
// If we would use the member repostiory, we would create a forUpdate lock when editing the member, including when fetching the member labels. Creating a possible deadlock if somewhere else we do the reverse in a transaction.
|
|
const timezone = this._settingsCacheService.get('timezone') || 'Etc/UTC';
|
|
const startOfDayInSiteTimezone = moment.utc(timestamp).tz(timezone).startOf('day').utc().format('YYYY-MM-DD HH:mm:ss');
|
|
const formattedTimestamp = moment.utc(timestamp).format('YYYY-MM-DD HH:mm:ss');
|
|
await this._db.knex('members')
|
|
.where('id', '=', memberId)
|
|
.andWhere(builder => builder
|
|
.where('last_seen_at', '<', startOfDayInSiteTimezone)
|
|
.orWhereNull('last_seen_at')
|
|
)
|
|
.update({
|
|
last_seen_at: formattedTimestamp
|
|
});
|
|
}
|
|
|
|
/**
|
|
* Updates the member.last_seen_at field if it wasn't updated in the current day yet (in the publication timezone)
|
|
* Example: current time is 2022-02-28 18:00:00
|
|
* - memberLastSeenAt is 2022-02-27 23:00:00, timestamp is current time, then `last_seen_at` is set to the current time
|
|
* - memberLastSeenAt is 2022-02-28 01:00:00, timestamp is current time, then `last_seen_at` isn't changed
|
|
* @param {string} memberId The id of the member to be udpated
|
|
* @param {Date} timestamp The event timestamp
|
|
*/
|
|
async updateLastSeenAt(memberId, memberLastSeenAt, timestamp) {
|
|
const timezone = this._settingsCacheService.get('timezone');
|
|
// First, check if memberLastSeenAt is null or before the beginning of the current day in the publication timezone
|
|
// This isn't strictly necessary since we will fetch the member row for update and double check this
|
|
// This is an optimization to avoid unnecessary database queries if last_seen_at is already after the beginning of the current day
|
|
if (memberLastSeenAt === null || moment(moment.utc(timestamp).tz(timezone).startOf('day')).isAfter(memberLastSeenAt)) {
|
|
const membersApi = this._getMembersApi();
|
|
await this._db.knex.transaction(async (trx) => {
|
|
// To avoid a race condition, we lock the member row for update, then the last_seen_at field again to prevent simultaneous updates
|
|
const currentMember = await membersApi.members.get({id: memberId}, {require: true, transacting: trx, forUpdate: true});
|
|
const currentMemberLastSeenAt = currentMember.get('last_seen_at');
|
|
if (currentMemberLastSeenAt === null || moment(moment.utc(timestamp).tz(timezone).startOf('day')).isAfter(currentMemberLastSeenAt)) {
|
|
const memberToUpdate = await currentMember.refresh({transacting: trx, forUpdate: false, withRelated: ['labels', 'newsletters']});
|
|
const updatedMember = await memberToUpdate.save({last_seen_at: moment.utc(timestamp).format('YYYY-MM-DD HH:mm:ss')}, {transacting: trx, patch: true, method: 'update'});
|
|
// The standard event doesn't get emitted inside the transaction, so we do it manually
|
|
this._events.emit('member.edited', updatedMember);
|
|
return Promise.resolve(updatedMember);
|
|
}
|
|
return Promise.resolve(undefined);
|
|
});
|
|
}
|
|
}
|
|
|
|
/**
|
|
* Updates the member.last_seen_at field if it wasn't updated in the current day yet (in the publication timezone)
|
|
* Example: current time is 2022-02-28 18:00:00
|
|
* - memberLastSeenAt is 2022-02-27 23:00:00, timestamp is current time, then `last_seen_at` is set to the current time
|
|
* - memberLastSeenAt is 2022-02-28 01:00:00, timestamp is current time, then `last_seen_at` isn't changed
|
|
* @param {string} memberId The id of the member to be udpated
|
|
* @param {Date} timestamp The event timestamp
|
|
*/
|
|
async updateLastCommentedAt(memberId, timestamp) {
|
|
const membersApi = this._getMembersApi();
|
|
const member = await membersApi.members.get({id: memberId}, {require: true});
|
|
const timezone = this._settingsCacheService.get('timezone');
|
|
|
|
const memberLastSeenAt = member.get('last_seen_at');
|
|
const memberLastCommentedAt = member.get('last_commented_at');
|
|
|
|
if (memberLastSeenAt === null || moment(moment.utc(timestamp).tz(timezone).startOf('day')).isAfter(memberLastSeenAt) || memberLastCommentedAt === null || moment(moment.utc(timestamp).tz(timezone).startOf('day')).isAfter(memberLastCommentedAt)) {
|
|
await membersApi.members.update({
|
|
last_seen_at: moment.utc(timestamp).format('YYYY-MM-DD HH:mm:ss'),
|
|
last_commented_at: moment.utc(timestamp).format('YYYY-MM-DD HH:mm:ss')
|
|
}, {
|
|
id: memberId
|
|
});
|
|
}
|
|
}
|
|
}
|
|
|
|
module.exports = LastSeenAtUpdater;
|