TryGhost/Ghost
1
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2024-11-30 21:40:39 +03:00
Code Issues Projects Releases Wiki Activity

Improved performance of sending newsletter emails (#12091)

Browse Source 
no-issue

- switch from `membersService.api.members.list` to using bookshelf `Member.findPage()` with the `{paid: true}` filter to avoid per-member queries (N+1) to decorate members with subscriptions and a heavy post-fetch filter via `contentGating`
- add concurrency to the Mailgun API requests in `bulk-email` service to reduce overall time submitting API requests
- add debug statements with timing output for easier measurements
This commit is contained in:
Kevin Ansfield 2020-08-06 14:19:39 +01:00 committed by GitHub
parent 490f9787fa
commit 2efcf94645
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 50 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
core/server/services/bulk-email/index.js
View File

@ -6,6 +6,7 @@ const mailgunProvider = require('./mailgun');
const configService = require('../../../shared/config'); const configService = require('../../../shared/config');
const settingsCache = require('../settings/cache'); const settingsCache = require('../settings/cache');
const sentry = require('../../../shared/sentry'); const sentry = require('../../../shared/sentry');
const debug = require('ghost-ignition').debug('mega');
/** /**
* An object representing batch request result * An object representing batch request result
@ -84,7 +85,7 @@ module.exports = {
const chunkedRecipients = _.chunk(recipients, BATCH_SIZE); const chunkedRecipients = _.chunk(recipients, BATCH_SIZE);
return Promise.mapSeries(chunkedRecipients, (toAddresses) => { return Promise.map(chunkedRecipients, (toAddresses, chunkIndex) => {
const recipientVariables = {}; const recipientVariables = {};
toAddresses.forEach((email) => { toAddresses.forEach((email) => {
recipientVariables[email] = recipientData[email]; recipientVariables[email] = recipientData[email];
@ -117,6 +118,8 @@ module.exports = {
delete messageData.plaintext; delete messageData.plaintext;
return new Promise((resolve) => { return new Promise((resolve) => {
const batchStartTime = Date.now();
debug(`sending message batch ${chunkIndex + 1} to ${toAddresses.length}`);
mailgunInstance.messages().send(messageData, (error, body) => { mailgunInstance.messages().send(messageData, (error, body) => {
if (error) { if (error) {
// NOTE: logging an error here only but actual handling should happen in more sophisticated batch retry handler // NOTE: logging an error here only but actual handling should happen in more sophisticated batch retry handler
@ -131,12 +134,14 @@ module.exports = {
// NOTE: these are generated variables, so can be regenerated when retry is done // NOTE: these are generated variables, so can be regenerated when retry is done
const data = _.omit(batchData, ['recipient-variables']); const data = _.omit(batchData, ['recipient-variables']);
debug(`failed message batch ${chunkIndex + 1} (${Date.now() - batchStartTime}ms)`);
resolve(new FailedBatch(error, data)); resolve(new FailedBatch(error, data));
} else { } else {
debug(`sent message batch ${chunkIndex + 1} (${Date.now() - batchStartTime}ms)`);
resolve(new SuccessfulBatch(body)); resolve(new SuccessfulBatch(body));
} }
}); });
}); });
}); }, {concurrency: 10});
} }
}; };

71
core/server/services/mega/mega.js
View File

@ -10,7 +10,9 @@ const bulkEmailService = require('../bulk-email');
const models = require('../../models'); const models = require('../../models');
const postEmailSerializer = require('./post-email-serializer'); const postEmailSerializer = require('./post-email-serializer');
const getEmailData = async (postModel, members = []) => { const getEmailData = async (postModel, memberModels = []) => {
const startTime = Date.now();
debug(`getEmailData: starting for ${memberModels.length} members`);
const {emailTmpl, replacements} = await postEmailSerializer.serialize(postModel); const {emailTmpl, replacements} = await postEmailSerializer.serialize(postModel);
emailTmpl.from = membersService.config.getEmailFromAddress(); emailTmpl.from = membersService.config.getEmailFromAddress();
@ -25,44 +27,41 @@ const getEmailData = async (postModel, members = []) => {
const emails = []; const emails = [];
const emailData = {}; const emailData = {};
members.forEach((member) => { memberModels.forEach((memberModel) => {
emails.push(member.email); emails.push(memberModel.get('email'));
// first_name is a computed property only used here for now // first_name is a computed property only used here for now
// TODO: move into model computed property or output serializer? // TODO: move into model computed property or output serializer?
member.first_name = (member.name || '').split(' ')[0]; memberModel.first_name = (memberModel.get('name') || '').split(' ')[0];
// add static data to mailgun template variables // add static data to mailgun template variables
const data = { const data = {
unique_id: member.uuid, unique_id: memberModel.uuid,
unsubscribe_url: postEmailSerializer.createUnsubscribeUrl(member.uuid) unsubscribe_url: postEmailSerializer.createUnsubscribeUrl(memberModel.get('uuid'))
}; };
// add replacement data/requested fallback to mailgun template variables // add replacement data/requested fallback to mailgun template variables
replacements.forEach(({id, memberProp, fallback}) => { replacements.forEach(({id, memberProp, fallback}) => {
data[id] = member[memberProp] || fallback || ''; data[id] = memberModel[memberProp] || fallback || '';
}); });
emailData[member.email] = data; emailData[memberModel.get('email')] = data;
}); });
debug(`getEmailData: done (${Date.now() - startTime}ms)`);
return {emailTmpl, emails, emailData}; return {emailTmpl, emails, emailData};
}; };
const sendEmail = async (postModel, members) => { const sendEmail = async (postModel, memberModels) => {
const membersToSendTo = members.filter((member) => { const {emailTmpl, emails, emailData} = await getEmailData(postModel, memberModels);
return membersService.contentGating.checkPostAccess(postModel.toJSON(), member);
});
const {emailTmpl, emails, emailData} = await getEmailData(postModel, membersToSendTo);
return bulkEmailService.send(emailTmpl, emails, emailData); return bulkEmailService.send(emailTmpl, emails, emailData);
}; };
const sendTestEmail = async (postModel, toEmails) => { const sendTestEmail = async (postModel, toEmails) => {
const recipients = await Promise.all(toEmails.map(async (email) => { const recipients = await Promise.all(toEmails.map(async (email) => {
const member = await membersService.api.members.get({email}); const member = await models.Member.findOne({email});
return member || {email}; return member || new models.Member({email});
})); }));
const {emailTmpl, emails, emailData} = await getEmailData(postModel, recipients); const {emailTmpl, emails, emailData} = await getEmailData(postModel, recipients);
emailTmpl.subject = `[Test] ${emailTmpl.subject}`; emailTmpl.subject = `[Test] ${emailTmpl.subject}`;
@ -80,17 +79,19 @@ const sendTestEmail = async (postModel, toEmails) => {
const addEmail = async (postModel, options) => { const addEmail = async (postModel, options) => {
const knexOptions = _.pick(options, ['transacting', 'forUpdate']); const knexOptions = _.pick(options, ['transacting', 'forUpdate']);
const filterOptions = Object.assign({}, knexOptions, {filter: 'subscribed:true', limit: 1});
// @TODO: improve performance of this members.list call if (postModel.get('visibility') === 'paid') {
debug('addEmail: retrieving members list'); filterOptions.paid = true;
const {members} = await membersService.api.members.list(Object.assign(knexOptions, {filter: 'subscribed:true'}, {limit: 'all'})); }
const membersToSendTo = members.filter((member) => {
return membersService.contentGating.checkPostAccess(postModel.toJSON(), member); const startRetrieve = Date.now();
}); debug('addEmail: retrieving members count');
debug('addEmail: retrieved members list'); const {meta: {pagination: {total: membersCount}}} = await models.Member.findPage(Object.assign({}, knexOptions, filterOptions));
debug(`addEmail: retrieved members count - ${membersCount} members (${Date.now() - startRetrieve}ms)`);
// NOTE: don't create email object when there's nobody to send the email to // NOTE: don't create email object when there's nobody to send the email to
if (!membersToSendTo.length) { if (membersCount === 0) {
return null; return null;
} }
@ -111,7 +112,7 @@ const addEmail = async (postModel, options) => {
return models.Email.add({ return models.Email.add({
post_id: postId, post_id: postId,
status: 'pending', status: 'pending',
email_count: membersToSendTo.length, email_count: membersCount,
subject: emailTmpl.subject, subject: emailTmpl.subject,
html: emailTmpl.html, html: emailTmpl.html,
plaintext: emailTmpl.plaintext, plaintext: emailTmpl.plaintext,
@ -197,16 +198,24 @@ async function pendingEmailHandler(emailModel, options) {
let meta = []; let meta = [];
let error = null; let error = null;
let startEmailSend = null;
try { try {
// Check host limit for allowed member count and throw error if over limit // Check host limit for allowed member count and throw error if over limit
await membersService.checkHostLimit(); await membersService.checkHostLimit();
// No need to fetch list until after we've passed the check // No need to fetch list until after we've passed the check
// @TODO: improve performance of this members.list call const knexOptions = _.pick(options, ['transacting', 'forUpdate']);
const filterOptions = Object.assign({}, knexOptions, {filter: 'subscribed:true', limit: 'all'});
if (postModel.get('visibility') === 'paid') {
filterOptions.paid = true;
}
const startRetrieve = Date.now();
debug('pendingEmailHandler: retrieving members list'); debug('pendingEmailHandler: retrieving members list');
const {members} = await membersService.api.members.list(Object.assign({filter: 'subscribed:true'}, {limit: 'all'})); const {data: members} = await models.Member.findPage(Object.assign({}, knexOptions, filterOptions));
debug('pendingEmailHandler: retrieved members list'); debug(`pendingEmailHandler: retrieved members list - ${members.length} members (${Date.now() - startRetrieve}ms)`);
if (!members.length) { if (!members.length) {
return; return;
@ -220,8 +229,14 @@ async function pendingEmailHandler(emailModel, options) {
// NOTE: meta can contains an array which can be a mix of successful and error responses // NOTE: meta can contains an array which can be a mix of successful and error responses
// needs filtering and saving objects of {error, batchData} form to separate property // needs filtering and saving objects of {error, batchData} form to separate property
debug('pendingEmailHandler: sending email');
startEmailSend = Date.now();
meta = await sendEmail(postModel, members); meta = await sendEmail(postModel, members);
debug(`pendingEmailHandler: sent email (${Date.now() - startEmailSend}ms)`);
} catch (err) { } catch (err) {
if (startEmailSend) {
debug(`pendingEmailHandler: send email failed (${Date.now() - startEmailSend}ms)`);
}
logging.error(new errors.GhostError({ logging.error(new errors.GhostError({
err: err, err: err,
context: i18n.t('errors.services.mega.requestFailed.error') context: i18n.t('errors.services.mega.requestFailed.error')