3

I have a simple table which is an email queue.

CREATE TABLE `emails_queue_batch` (
  `eq_log_id` int(11) NOT NULL DEFAULT '0',
  `eq_to` varchar(120) CHARACTER SET utf8 DEFAULT NULL,
  `eq_bcc` varchar(80) CHARACTER SET utf8 DEFAULT '',
  `eq_from` varchar(80) CHARACTER SET utf8 DEFAULT NULL,
  `eq_title` varchar(100) COLLATE utf8_unicode_ci DEFAULT NULL,
  `eq_headers` varchar(80) CHARACTER SET utf8 DEFAULT NULL,
  `eq_content` longtext CHARACTER SET utf8,
  `eq_sid` int(11) DEFAULT '0',
  `eq_type` int(11) DEFAULT '0' COMMENT 'email type',
  `eq_esp` int(11) DEFAULT '0',
  PRIMARY KEY (`eq_log_id`),
  KEY `email` (`eq_to`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci 

Several threads read repeatedly 50 rows at a time and delete the rows.

To avoid double reading of the same row I used :

    $db->query(" LOCK TABLE $table WRITE ");

    $query= "SELECT * FROM $table LIMIT  ".CHUNK_SIZE. " " ; 

    $emails2send=$db->get_results ($query);

    if (!empty ($emails2send)){

        // DELETE EMAIL 
        $eq_log_ids = array();
        foreach ($emails2send as $email) $eq_log_ids[]= $email->eq_log_id ;

        $query= "DELETE FROM $table WHERE eq_log_id IN ( ".implode(',', $eq_log_ids)." ) ";
        $db->query ($query);

        $db->query (" UNLOCK TABLES "); // unlock the table so other sessions can read next rows
        ........ code processing the read rows here .............
    } else { // if !empty emails2send
        // $emails2send is empty 
        $db->query (" UNLOCK TABLES; ");    
        $stop_running=true; // stop running
    }

Another thread (s) write to the table at the same time. For reason I don't understand this configuration got deadlocked with locked table both for reads and for writes.

My question is: Is this locking the right solution to make sure I read each row once and only once (and delete it).

Or is this better handled as a transaction, and if so which kind? I'm not experienced with transactions.

Nir
  • 24,619
  • 25
  • 81
  • 117
  • 1
    The right solution is to use a real message queue (examples: RabbitMQ or ActiveMQ). Using a transactional database as a de facto queue always results in lock contention and deadlocks, unless you limit yourself to only ONE thread reading from the database-queue. – Bill Karwin Nov 26 '17 at 16:23
  • Thanks, I'm working on it. One problem with a message queue is that if something goes wrong and for example an email batch needs to be cancelled, you cant delete from the queue selectively. – Nir Nov 27 '17 at 10:30
  • At one of my past jobs, we kept tasks in the database, but a "dispatcher" process pulled tasks when they were ready to run, and posted them in a MQ. As long as the dispatcher is single-threaded, it avoids the issues you are asking about. Then the MQ can be read by many worker threads. That strikes a balance—you can delete or modify tasks in the DB, up until they are posted to the MQ. – Bill Karwin Nov 27 '17 at 14:33

2 Answers2

1

Using a transaction is probably better, especially if you're deadlocking.

First, try reducing your batch size from 50 to one and see if things improve. They might. And it's easy. And it's what you'll want to do if you use transactions.

Second, try this kind of sequence of queries.

  START TRANSACTION;
  SELECT @id := table.eq_log_id, table.* FROM table LIMIT 1 FOR UPDATE;
  /* handle the item here */
  DELETE FROM table WHERE eq_log_id = @id;
  COMMIT;

This only works if eq_log_id is a unique (or primary) key. Run this in a loop in your php program until the SELECT operation returns no rows. Then sleep for a while, and try again.

Even better would be to add a TIMESTAMP called processed to your table, with a null default value. Then instead of DELETEing the rows you could update their timestamps. This would give you a way of troubleshooting stuff.

START TRANSACTION;
SELECT @id:=eq_log_id, * FROM table WHERE processed IS NULL LIMIT 1 FOR UPDATE;
/* handle the item here */
UPDATE table SET processed=NOW() WHERE eq_log_id = @id;
COMMIT;

You can run an overnight batch to wipe out all the stale records like this

DELETE FROM table WHERE processed < CURDATE() - INTERVAL 1 DAY;

I suggest this because in production it can be really helpful to see the time history of sent messages.

O. Jones
  • 103,626
  • 17
  • 118
  • 172
1

Plan A:

This assumes you can process N rows in less than, say, 2 seconds. You have N=50 -- this may be too large.

BEGIN;
SELECT ... LIMIT 50  FOR UPDATE;
... process ...
... gather a list of ids to delete ...
DELETE ... WHERE id IN (...)
COMMIT;

The more you grab, the faster it goes, but also the more likely it is to get deadlocks. When a deadlock occurs, simply start the transaction over. Also keep track of how often deadlocks occur, in order to tune the "50".

Plan B:

This is useful when the processing of an item takes "too long" for a transaction. I say 2 seconds is probably "too long".

Grab a row to process:
with autocommit=ON ...
UPDATE ... SET who_is_processing = $me,
               when_grabbed = NOW()
               id = LAST_INSERT_ID(id),
           WHERE when_grabbed IS NULL
             AND any-other-criteria
           LIMIT 1;
$id = SELECT LAST_INSERT_ID();

... process $id ...  (This may or may not involve transactions)

Release the row (or, in your case, delete it):
again, autocommit=ON suffices...
DELETE ... WHERE id = $id;

"Never" use table locks with InnoDB. (There may be use cases, but this is not one.)

Rick James
  • 135,179
  • 13
  • 127
  • 222