Fix for bug#22738 Events: After stop and start disabled events could reside in the queue

Disabled events weren't removed from the memory queue after the scheduler has been
re-enabled. After recalculation of next execution time of an event, it might get disabled.


sql/event_queue.cc:
  Sort the event queue in a way that the disabled events will always be
  at the end. We will use this for cleaning it, starting from the end.
  
  After recalculating times in the queue, after the scheduler has been enabled
  after disabled state, the queue should be cleaned from DISABLED events.
  The queue is sorted in a way such that the disabled events are at the end.
  Thus, we can start from the end of the queue and remove all DISABLED till we
  find the first with different state.
sql/events.cc:
  Add a comment about possible problem with replication of events,
  disabled events and server restarts.

2 files changed, 42 insertions, 3 deletions

diff --git a/sql/event_queue.cc b/sql/event_queue.cc
index 898fc206c34..719a837cbfb 100644
--- a/sql/event_queue.cc
+++ b/sql/event_queue.cc
@@ -60,8 +60,16 @@ extern "C" int event_queue_element_compare_q(void *, uchar *, uchar *);
 
 int event_queue_element_compare_q(void *vptr, uchar* a, uchar *b)
 {
-  my_time_t lhs = ((Event_queue_element *)a)->execute_at;
-  my_time_t rhs = ((Event_queue_element *)b)->execute_at;
+  Event_queue_element *left = (Event_queue_element *)a;
+  Event_queue_element *right = (Event_queue_element *)b;
+  my_time_t lhs = left->execute_at;
+  my_time_t rhs = right->execute_at;
+
+  if (left->status == Event_queue_element::DISABLED)
+    return right->status != Event_queue_element::DISABLED;
+
+  if (right->status == Event_queue_element::DISABLED)
+    return 1;
 
   return (lhs < rhs ? -1 : (lhs > rhs ? 1 : 0));
 }
@@ -434,8 +442,34 @@ Event_queue::recalculate_activation_times(THD *thd)
     ((Event_queue_element*)queue_element(&queue, i))->update_timing_fields(thd);
   }
   queue_fix(&queue);
+  /*
+    The disabled elements are moved to the end during the `fix`.
+    Start from the end and remove all of the elements which are
+    disabled. When we find the first non-disabled one we break, as we
+    have removed all. The queue has been ordered in a way the disabled
+    events are at the end.
+  */
+  for (i= queue.elements; i > 0; i--)
+  {
+    Event_queue_element *element = (Event_queue_element*)queue_element(&queue, i - 1);
+    if (element->status != Event_queue_element::DISABLED)
+      break;
+    /*
+      This won't cause queue re-order, because we remove
+      always the last element.
+    */
+    queue_remove(&queue, i - 1);
+    delete element;
+  }
   UNLOCK_QUEUE_DATA();
 
+  /*
+    XXX: The events are dropped only from memory and not from disk
+         even if `drop_list[j]->dropped` is TRUE. There will be still on the
+         disk till next server restart.
+         Please add code here to do it.
+  */
+
   DBUG_VOID_RETURN;
 }
 
diff --git a/sql/events.cc b/sql/events.cc
index 87385082a82..700663db7d1 100644
--- a/sql/events.cc
+++ b/sql/events.cc
@@ -1185,7 +1185,12 @@ Events::load_events_from_db(THD *thd)
     {
       /*
         If not created, a stale event - drop if immediately if
-        ON COMPLETION NOT PRESERVE
+        ON COMPLETION NOT PRESERVE.
+        XXX: This won't be replicated, thus the drop won't appear in
+             in the slave. When the slave is restarted it will drop events.
+             However, as the slave will be "out of sync", it might happen that
+             an event created on the master, after master restart, won't be
+             replicated to the slave correctly, as the create will fail there.
       */
       int rc= table->file->ha_delete_row(table->record[0]);
       if (rc)