SmtpMailer – odesílá mail 2×

Tím chci říct, že pole tasks má správný počet položek a počet cyklů taky souhlasí. Tím jsem si jist. Vypadá to, jako by se ten script spustil opakovaně…

Kamil Valenta napsal(a):

Takže máš podezření na paralelní souběh. Nejspíš to tak bude. Jak to tedy máš proti souběhu ošetřeno?
Jak vypadá dataEmailExpiredTask()? Zohledňuje zápisy v sentemail?

Obecně není dobré „něco vytáhnout, dlouho zpracovávat a pak zapisovat někam doběhnutí“. Pokud na to najíždí třeba cron každou minutu, dataEmailExpiredTask() vratí např. ID 1,2,3,4. Poštovní server ale může odesílat celkem pomalu a než to celé doběhne, cron znovu tikne a dataEmailExpiredTask() vratí ID 3,4,5,6.
Tasky #3 a #4 doběhnou z prvního cronu, ale zároveň poběží i z druhého.

Předem díky za odpověď. Vůbec jsem netušil nic o nějakém souběhu, ale chápu, že se něco takového může dít. Tady ta metoda:

<?php
public function dataEmailExpiredTask() { // na produkcnim nastavit CRON

        $select = $this->database->table('firm')->fetchAll();

        if ($select != null) {

            $tasks = []; // ukoly pro firmid, terapeut muze mit vic zaznamu (emaily jednotlive pro kazdy ukol)

            $deadline = DateTime::from(date('Y-m-d'))->modifyClone('-1 day');

            foreach ($select as $se) {

                $sel = $this->database->query("SELECT calendar.*, terapeut.name, terapeut.surname, terapeut.email, terapeut.phone FROM calendar "
                        . "LEFT JOIN terapeut ON terapeut.id = calendar.terapeutid "
                        . "LEFT JOIN users ON users.id = terapeut.userid "
                        . "WHERE users.firmid = $se->id "
                        . "AND ((day = '$deadline' AND dayend IS NULL) OR (dayend = '$deadline' AND start IS NULL)) "
                        . "AND calendar.inserter = 1 "
                        . "AND calendar.status = 1 ORDER BY day");

                if ($sel != null) {

                    foreach ($sel as $s) {

                        $row = $this->database->table('firm')->get($se->id);

                        if ($row) {

                            $emailfirm = $row->email;

                            $phonefirm = $row->phone;
                        }

                        $tasks[] = [
                            'id' => $s->id,
                            'day' => $s->day,
                            'calname' => $s->calname,
                            'terapeutid' => $s->terapeutid,
                            'name' => $s->name,
                            'surname' => $s->surname,
                            'firmid' => $se->id,
                            'email' => $s->email,
                            'emailfirm' => $emailfirm,
                            'phonefirm' => $phonefirm,
                            'phoneuser' => $s->phone
                            ];
                    }
                }
            }
        }
?>

Jak provést takové oštření proti souběhu?

Ještě poznatek, já mám stejný script (presenter) také na odesílání SMS. Tam se to dělo taky. Původně jsem to odesílání měl v actionDefault(), ale změnil jsem to na public function startup() { parent::startup();… Zdá se, že to na SMSkách pomohlo, zatím se to duplikování neděje, ale na emailech to stále duplikuje.

V access logu uvidíš, kolikrát se to spustilo. Máš to ve startupu, pokud je na stránce nějaký ajax třeba, snadno se to spustí vícekrát za jeden plánovaný request.

Přidej si do calendar sloupec např. „notified“, který bude null. Po vygenerování upozornění v něm bude třeba nějaký unikátní otisk.

foreach ($select as $se) {
    $uni = uniqid();
    $this->database->query("UPDATE calendar SET "
                        . "notified='$uni'"
                        . "WHERE notified IS NULL "
                        . "((day = '$deadline' AND dayend IS NULL) OR (dayend = '$deadline' AND start IS NULL)) "
                        . "AND calendar.inserter = 1 "
                        . "AND calendar.status = 1");
    $sel = $this->database->query("SELECT calendar.*, terapeut.name, terapeut.surname, terapeut.email, terapeut.phone FROM calendar "
                        . "LEFT JOIN terapeut ON terapeut.id = calendar.terapeutid "
                        . "LEFT JOIN users ON users.id = terapeut.userid "
                        . "WHERE users.firmid = $se->id "
                        . "AND calendar.notified = '$uni' ORDER BY day");

Mimo téma:

1. jaký máš důvod skládat dotazy takto s parametry a nebindovat je tam do placeholderů? (queryArgs() místo query()…)
2. proč znovu taháš $row = $this->database->table(‚firm‘)->get($se->id); a nesaháš přímo na $se->email a $se->phone?

Editoval Kamil Valenta (28. 7. 2023 10:11)

m.brecher napsal(a):

@adminaeasyweb4ucz

Jak provést takové ošetření proti souběhu?

Odeslaný email je upomínka uživateli na provedení nějakého úkolu – reminder. Reminder upomíná konkrétní úkol a to asi opakovaně, takže by měl být v tabulce reminder, která má vazbu n : 1 na tabulku calendar, kde jsou úkoly. Upomínky by se měly generovat u nesplněných úkolů nějakým pravidelně spouštěným procesem s vyloučením duplicity ideálně vhodně zvoleným unikátním klíčem ve stavu neodesláno. Další proces by měl emaily upomínek odesílat. V případě úspěšného odeslání označit upomínku jako odeslanou. Pokud by odeslání emailu selhalo, zůstane upomínka označena jako neodeslaná a email se proces pokusí odeslat v dalším kole.

Ty nemáš mezi procesem generujícím upomínky dataEmailExpiredTask() a skutečně odeslanými upomínkami žádnou synchronizaci. Vhodná synchronizace spočívá dle mého názoru v lepším návrhu databáze.

Díky, to mi přijde jako dobré řešení.

admin@easyweb4u.cz napsal(a):

Jsou v jiné tabulce – firm

Sám se ztrácíš ve svém ne úplně ideálním pojmenování proměnných. $se je přece z firm.

Já si nemyslím, že NENÍ problém v té metodě $this->notice->dataEmailExpiredTask(); to pole se normálně vygeneruje a pak probíhá iterace. Podle mě to dělá ten mailserver.

Vzhledem ke dvěma záporům souhlasím.

• Chceš do databáze zapisovat, že už se email odeslal.
  • Odeslání může selhat, z různých důvodů.
  • A při dalším spuštění potřebuješ vědět, že už se mail odeslal
• Nechceš odesílání spouštět vícekrát současně, je třeba mít zámky
  • Buď pro cron job nebo pro konkrétní email
  • Sám si job můžeš zamknout přes symfony/lock, všechny joby umí zamykat automaticky můj orisai/nette-scheduler (též si tam nastavíš symfony/lock)
  • Konkrétní maily můžeš zamykat zase přes symfony/lock nebo lze použít zamykání přímo v databází – select for update

Dají se použít i message queues – jeden proces do fronty přidá zprávu, že se má odeslat email. Druhý proces zprávu zpracuje (odešle email) a z fronty zprávu odstraní. Je to v mnohém lepší řešení, ale složitější, např. RabbitMQ není easy na konfiguraci.

Kamil Valenta napsal(a):

admin@easyweb4u.cz napsal(a):

Jsou v jiné tabulce – firm

Sám se ztrácíš ve svém ne úplně ideálním pojmenování proměnných. $se je přece z firm.

Já si nemyslím, že NENÍ problém v té metodě $this->notice->dataEmailExpiredTask(); to pole se normálně vygeneruje a pak probíhá iterace. Podle mě to dělá ten mailserver.

Vzhledem ke dvěma záporům souhlasím.

V tom to není, to pole tasks je tak jak má být. Monokrát zkontrolované. Vypadá to, že se script spustí 2×.