queueMessage speed is too low

[malas]

In DatabaseSpool::queueMessage do not use

$this->em->flush();

for every message as it drops down performance significantly. Flush should be called from the invoking code (externally):

foreach($messages as $message) {
  $this->getContainer()->get('mailer')->send($message);
}
$this->em->flush();

[dfridrich]

I'd better use flush() in every loop to prevent issues when app stops unexpectedly. If it would flush() after loop, it would send again and again email which have been sent already.

:-1:

[malas]

You can catch exceptions and delay/remove the message which causes it from resending it.

an option not to make SQL transaction after EVERY message would be a truly better way. Just try to send 1000 messages in one loop and check how much time the flushes consume in comparison with generating message content. no more comments would be needed.

[aistis-]

Run into the same problem.

[richsage]

I've started a branch here but won't have time in the next few days to look at it in depth. It marks emails as FAILED. This is very rough & ready code so far :-)

[malas]

@richsage sorry, but how this helps our problem? :)

there is an obvious architecture issue when working with emails and DB. Handling failed emails is a different problem. When working with batch emails it is a blocker.

[richsage]

If an email fails during the send process, your options would be:

• Ignore it, do nothing and carry on - you're not bothered about knowing the outcome
• Mark it as failed, let some other process capture this new state, carry on
• Throw an exception and don't continue

The last option is what happens currently. The middle option is what I have started in my branch.

@malas if you are able to open a PR with your suggested approach, I will happily review but at the moment I don't currently have capacity to work on this change.

[aistis-]

What if use DBAL instead of entities? It would give a better performance.

[malas]

@aistis- could you explain more? how we could control the flush moment when using DBAL?

@richsage i will try to make one in some freeish weekend :)

[aistis-]

@malas depends on flushing strategy:

• if flush is done after every email - no control is needed as it is single insert statement;
• if flush is done in batches - build a batch into a memory and commit it in a single transaction, but as @richsage pointed out, here comes a tricky part to handle failed emails.

The idea would be to avoid using EntityManager::flush() as it flushes all other entities, despite you want it or not :confused:

[sampart]

The idea would be to avoid using EntityManager::flush() as it flushes all other entities, despite you want it or not :confused:

You can change this behaviour thanks to https://github.com/whiteoctober/WhiteOctoberSwiftMailerDBBundle/pull/21 (although that's not in a release yet - if anyone needs it, do shout).