From ac2daccf9582f7971e9661cda796b35f4087f3db Mon Sep 17 00:00:00 2001 From: Simon Backx Date: Tue, 7 Feb 2023 11:01:49 +0100 Subject: [PATCH] Added short lived caching to email batch body (#16233) fixes https://github.com/TryGhost/Team/issues/2522 When sending an email for multiple batches at the same time, we now reuse the same email body for each batch in the same segment. This reduces the amount of database queries and makes the sending more reliable in case of database failures. The cache is short lived. After sending the email it is automatically garbage collected. --- .../lib/batch-sending-service.js | 11 +- ghost/email-service/lib/email-body-cache.js | 20 +++ ghost/email-service/lib/sending-service.js | 37 ++++-- .../test/sending-service.test.js | 114 ++++++++++++++++++ 4 files changed, 172 insertions(+), 10 deletions(-) create mode 100644 ghost/email-service/lib/email-body-cache.js diff --git a/ghost/email-service/lib/batch-sending-service.js b/ghost/email-service/lib/batch-sending-service.js index 6d9619fc55..f807cbbb6e 100644 --- a/ghost/email-service/lib/batch-sending-service.js +++ b/ghost/email-service/lib/batch-sending-service.js @@ -2,6 +2,7 @@ const logging = require('@tryghost/logging'); const ObjectID = require('bson-objectid').default; const errors = require('@tryghost/errors'); const tpl = require('@tryghost/tpl'); +const EmailBodyCache = require('./email-body-cache'); const messages = { emailErrorPartialFailure: 'An error occurred, and your newsletter was only partially sent. Please retry sending the remaining emails.', @@ -317,6 +318,9 @@ class BatchSendingService { async sendBatches({email, batches, post, newsletter}) { logging.info(`Sending ${batches.length} batches for email ${email.id}`); + // Reuse same HTML body if we send an email to the same segment + const emailBodyCache = new EmailBodyCache(); + // Loop batches and send them via the EmailProvider let succeededCount = 0; const queue = batches.slice(); @@ -326,7 +330,7 @@ class BatchSendingService { runNext = async () => { const batch = queue.shift(); if (batch) { - if (await this.sendBatch({email, batch, post, newsletter})) { + if (await this.sendBatch({email, batch, post, newsletter, emailBodyCache})) { succeededCount += 1; } await runNext(); @@ -353,7 +357,7 @@ class BatchSendingService { * @param {{email: Email, batch: EmailBatch, post: Post, newsletter: Newsletter}} data * @returns {Promise} True when succeeded, false when failed with an error */ - async sendBatch({email, batch: originalBatch, post, newsletter}) { + async sendBatch({email, batch: originalBatch, post, newsletter, emailBodyCache}) { logging.info(`Sending batch ${originalBatch.id} for email ${email.id}`); // Check the status of the email batch in a 'for update' transaction @@ -397,7 +401,8 @@ class BatchSendingService { members }, { openTrackingEnabled: !!email.get('track_opens'), - clickTrackingEnabled: !!email.get('track_clicks') + clickTrackingEnabled: !!email.get('track_clicks'), + emailBodyCache }); succeeded = true; diff --git a/ghost/email-service/lib/email-body-cache.js b/ghost/email-service/lib/email-body-cache.js new file mode 100644 index 0000000000..734b069133 --- /dev/null +++ b/ghost/email-service/lib/email-body-cache.js @@ -0,0 +1,20 @@ +/** + * This is a cache provider that lives very short in memory, there is no need for persistence. + * It is created when scheduling an email in the batch sending service, and is then passed to the sending service. The sending service + * can optionally use a passed cache provider to reuse the email body for each batch with the same segment. + */ +class EmailBodyCache { + constructor() { + this.cache = new Map(); + } + + get(key) { + return this.cache.get(key) ?? null; + } + + set(key, value) { + this.cache.set(key, value); + } +} + +module.exports = EmailBodyCache; diff --git a/ghost/email-service/lib/sending-service.js b/ghost/email-service/lib/sending-service.js index f3443c9868..89817688e6 100644 --- a/ghost/email-service/lib/sending-service.js +++ b/ghost/email-service/lib/sending-service.js @@ -22,12 +22,14 @@ const logging = require('@tryghost/logging'); /** * @typedef {import("./email-renderer")} EmailRenderer + * @typedef {import("./email-renderer").EmailBody} EmailBody */ /** * @typedef {object} EmailSendingOptions * @prop {boolean} clickTrackingEnabled * @prop {boolean} openTrackingEnabled + * @prop {{get(id: string): EmailBody | null, set(id: string, body: EmailBody): void}} [emailBodyCache] */ /** @@ -85,12 +87,30 @@ class SendingService { * @returns {Promise} */ async send({post, newsletter, segment, members, emailId}, options) { - const emailBody = await this.#emailRenderer.renderBody( - post, - newsletter, - segment, - options - ); + const cacheId = emailId + '-' + (segment ?? 'null'); + + /** + * @type {EmailBody | null} + */ + let emailBody = null; + + if (options.emailBodyCache) { + emailBody = options.emailBodyCache.get(cacheId); + } + + if (!emailBody) { + emailBody = await this.#emailRenderer.renderBody( + post, + newsletter, + segment, + { + clickTrackingEnabled: !!options.clickTrackingEnabled + } + ); + if (options.emailBodyCache) { + options.emailBodyCache.set(cacheId, emailBody); + } + } const recipients = this.buildRecipients(members, emailBody.replacements); return await this.#emailProvider.send({ @@ -102,7 +122,10 @@ class SendingService { recipients, emailId: emailId, replacementDefinitions: emailBody.replacements - }, options); + }, { + clickTrackingEnabled: !!options.clickTrackingEnabled, + openTrackingEnabled: !!options.openTrackingEnabled + }); } /** diff --git a/ghost/email-service/test/sending-service.test.js b/ghost/email-service/test/sending-service.test.js index 65c63878d2..bd8f178278 100644 --- a/ghost/email-service/test/sending-service.test.js +++ b/ghost/email-service/test/sending-service.test.js @@ -1,6 +1,7 @@ const SendingService = require('../lib/sending-service'); const sinon = require('sinon'); const assert = require('assert'); +const EmailBodyCache = require('../lib/email-body-cache'); describe('Sending service', function () { describe('send', function () { @@ -102,6 +103,119 @@ describe('Sending service', function () { )); }); + it('supports cache', async function () { + const emailBodyCache = new EmailBodyCache(); + const sendingService = new SendingService({ + emailRenderer, + emailProvider + }); + + const response = await sendingService.send({ + post: {}, + newsletter: {}, + segment: null, + emailId: '123', + members: [ + { + email: 'member@example.com', + name: 'John' + } + ] + }, { + clickTrackingEnabled: true, + openTrackingEnabled: true, + emailBodyCache + }); + assert.equal(response.id, 'provider-123'); + sinon.assert.calledOnce(sendStub); + sinon.assert.calledOnce(emailRenderer.renderBody); + assert(sendStub.calledWith( + { + subject: 'Hi', + from: 'ghost@example.com', + replyTo: 'ghost+reply@example.com', + html: 'Hi {{name}}', + plaintext: 'Hi', + emailId: '123', + replacementDefinitions: [ + { + id: 'name', + token: '{{name}}', + getValue: sinon.match.func + } + ], + recipients: [ + { + email: 'member@example.com', + replacements: [{ + id: 'name', + token: '{{name}}', + value: 'John' + }] + } + ] + }, + { + clickTrackingEnabled: true, + openTrackingEnabled: true + } + )); + + // Do again and see if cache is used + const response2 = await sendingService.send({ + post: {}, + newsletter: {}, + segment: null, + emailId: '123', + members: [ + { + email: 'member@example.com', + name: 'John' + } + ] + }, { + clickTrackingEnabled: true, + openTrackingEnabled: true, + emailBodyCache + }); + assert.equal(response2.id, 'provider-123'); + sinon.assert.calledTwice(sendStub); + assert(sendStub.getCall(1).calledWith( + { + subject: 'Hi', + from: 'ghost@example.com', + replyTo: 'ghost+reply@example.com', + html: 'Hi {{name}}', + plaintext: 'Hi', + emailId: '123', + replacementDefinitions: [ + { + id: 'name', + token: '{{name}}', + getValue: sinon.match.func + } + ], + recipients: [ + { + email: 'member@example.com', + replacements: [{ + id: 'name', + token: '{{name}}', + value: 'John' + }] + } + ] + }, + { + clickTrackingEnabled: true, + openTrackingEnabled: true + } + )); + + // Didn't call renderBody again + sinon.assert.calledOnce(emailRenderer.renderBody); + }); + it('removes invalid recipients before sending', async function () { const sendingService = new SendingService({ emailRenderer,