-
-
Notifications
You must be signed in to change notification settings - Fork 2k
MDEV-38632 ALTER EVENT doesn't re-run one-time AT event after first execution #5054
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| SET GLOBAL event_scheduler = ON; | ||
| CREATE TABLE test.event_log (id INT AUTO_INCREMENT PRIMARY KEY, ts TIMESTAMP DEFAULT CURRENT_TIMESTAMP); | ||
| # | ||
| # Step 1: Create a one-time AT event with ON COMPLETION PRESERVE | ||
| # | ||
| CREATE EVENT test.mdev38632 | ||
| ON SCHEDULE AT CURRENT_TIMESTAMP + INTERVAL 1 SECOND | ||
| ON COMPLETION PRESERVE ENABLE | ||
| DO INSERT INTO test.event_log(id) VALUES (NULL); | ||
| # Wait for the event to execute | ||
| SELECT COUNT(*) AS exec_count FROM test.event_log; | ||
| exec_count | ||
| 1 | ||
| # Verify: event executed, status is now DISABLED (preserved but done) | ||
| SELECT status, on_completion, last_executed IS NOT NULL AS has_last_exec | ||
| FROM mysql.event WHERE db = 'test' AND name = 'mdev38632'; | ||
| status on_completion has_last_exec | ||
| DISABLED PRESERVE 1 | ||
| # | ||
| # Step 2: ALTER EVENT to reschedule — without explicit ENABLE | ||
| # This is the customer's scenario from the JIRA report. | ||
| # | ||
| ALTER EVENT test.mdev38632 | ||
| ON SCHEDULE AT CURRENT_TIMESTAMP + INTERVAL 1 SECOND; | ||
| # After ALTER with a new schedule, status should be re-enabled automatically. | ||
| SELECT status FROM mysql.event WHERE db = 'test' AND name = 'mdev38632'; | ||
| status | ||
| ENABLED | ||
| # The event should fire again after being rescheduled. | ||
| SELECT COUNT(*) AS exec_count FROM test.event_log; | ||
| exec_count | ||
| 2 | ||
| # | ||
| # Step 3: Try again with explicit ENABLE — verify scheduler reload works. | ||
| # | ||
| ALTER EVENT test.mdev38632 | ||
| ON SCHEDULE AT CURRENT_TIMESTAMP + INTERVAL 1 SECOND | ||
| ON COMPLETION PRESERVE ENABLE; | ||
| # After fix, last_executed should be cleared when schedule changes. | ||
| SELECT status, last_executed IS NOT NULL AS has_last_exec | ||
| FROM mysql.event WHERE db = 'test' AND name = 'mdev38632'; | ||
| status has_last_exec | ||
| ENABLED 0 | ||
| # Restart the scheduler to trigger reload from mysql.event. | ||
| # After fix, the event should remain ENABLED after reload. | ||
| SET GLOBAL event_scheduler = OFF; | ||
| SET GLOBAL event_scheduler = ON; | ||
| # Status should remain ENABLED after scheduler reload. | ||
| SELECT status FROM mysql.event WHERE db = 'test' AND name = 'mdev38632'; | ||
| status | ||
| ENABLED | ||
| # | ||
| # Step 4: User-disabled event should NOT be re-enabled by reschedule | ||
| # | ||
| ALTER EVENT test.mdev38632 | ||
| ON SCHEDULE AT CURRENT_TIMESTAMP + INTERVAL 1 HOUR | ||
| ON COMPLETION PRESERVE ENABLE; | ||
| ALTER EVENT test.mdev38632 DISABLE; | ||
| SELECT status FROM mysql.event WHERE db = 'test' AND name = 'mdev38632'; | ||
| status | ||
| DISABLED | ||
| # Reschedule without explicit ENABLE — should stay DISABLED | ||
| ALTER EVENT test.mdev38632 | ||
| ON SCHEDULE AT CURRENT_TIMESTAMP + INTERVAL 2 HOUR; | ||
| SELECT status FROM mysql.event WHERE db = 'test' AND name = 'mdev38632'; | ||
| status | ||
| DISABLED | ||
| # | ||
| # Step 5: ALTER with same execute_at should not clear last_executed | ||
| # | ||
| DROP EVENT test.mdev38632; | ||
| CREATE EVENT test.mdev38632 | ||
| ON SCHEDULE AT CURRENT_TIMESTAMP + INTERVAL 1 SECOND | ||
| ON COMPLETION PRESERVE ENABLE | ||
| DO INSERT INTO test.event_log(id) VALUES (NULL); | ||
| # Event executed, now ALTER with same body but no schedule change | ||
| ALTER EVENT test.mdev38632 | ||
| DO INSERT INTO test.event_log(id) VALUES (NULL); | ||
| # Status should remain DISABLED (no schedule change, no re-enable) | ||
| SELECT status FROM mysql.event WHERE db = 'test' AND name = 'mdev38632'; | ||
| status | ||
| DISABLED | ||
| # | ||
| # Cleanup | ||
| # | ||
| DROP EVENT IF EXISTS test.mdev38632; | ||
| DROP TABLE test.event_log; | ||
| SET GLOBAL event_scheduler = OFF; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| # | ||
| # MDEV-38632: ALTER EVENT doesn't run a one-time (AT) event after its first | ||
| # execution when ON COMPLETION PRESERVE is used. | ||
| # | ||
| # After an AT event with ON COMPLETION PRESERVE executes: | ||
| # 1. compute_next_execution_time() sets status=DISABLED in mysql.event | ||
| # 2. ALTER EVENT without explicit ENABLE does not reset the status | ||
| # 3. The event queue refuses to queue a DISABLED event | ||
| # 4. Even with explicit ENABLE, last_executed causes re-disable on reload | ||
| # | ||
| # Expected fix: ALTER EVENT with a new schedule should re-enable the event | ||
| # and clear last_executed so it is properly queued. | ||
| # | ||
|
|
||
| --source include/not_embedded.inc | ||
|
|
||
| SET GLOBAL event_scheduler = ON; | ||
| --source include/running_event_scheduler.inc | ||
|
|
||
| CREATE TABLE test.event_log (id INT AUTO_INCREMENT PRIMARY KEY, ts TIMESTAMP DEFAULT CURRENT_TIMESTAMP); | ||
|
|
||
| --echo # | ||
| --echo # Step 1: Create a one-time AT event with ON COMPLETION PRESERVE | ||
| --echo # | ||
| CREATE EVENT test.mdev38632 | ||
| ON SCHEDULE AT CURRENT_TIMESTAMP + INTERVAL 1 SECOND | ||
| ON COMPLETION PRESERVE ENABLE | ||
| DO INSERT INTO test.event_log(id) VALUES (NULL); | ||
|
|
||
| --echo # Wait for the event to execute | ||
| let $wait_condition = SELECT COUNT(*) >= 1 FROM test.event_log; | ||
| --source include/wait_condition.inc | ||
|
|
||
| SELECT COUNT(*) AS exec_count FROM test.event_log; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How is this relevant? Either select from the full table or remove the statement. |
||
|
|
||
| --echo # Verify: event executed, status is now DISABLED (preserved but done) | ||
| SELECT status, on_completion, last_executed IS NOT NULL AS has_last_exec | ||
| FROM mysql.event WHERE db = 'test' AND name = 'mdev38632'; | ||
|
|
||
| --echo # | ||
| --echo # Step 2: ALTER EVENT to reschedule — without explicit ENABLE | ||
| --echo # This is the customer's scenario from the JIRA report. | ||
| --echo # | ||
| ALTER EVENT test.mdev38632 | ||
| ON SCHEDULE AT CURRENT_TIMESTAMP + INTERVAL 1 SECOND; | ||
|
|
||
| --echo # After ALTER with a new schedule, status should be re-enabled automatically. | ||
| SELECT status FROM mysql.event WHERE db = 'test' AND name = 'mdev38632'; | ||
|
|
||
| --echo # The event should fire again after being rescheduled. | ||
| let $wait_condition = SELECT COUNT(*) >= 2 FROM test.event_log; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto here: it's going to always be 2, right? |
||
| --source include/wait_condition.inc | ||
|
|
||
| SELECT COUNT(*) AS exec_count FROM test.event_log; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. redundant, remove it please |
||
|
|
||
| --echo # | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd reduce this to a single line |
||
| --echo # Step 3: Try again with explicit ENABLE — verify scheduler reload works. | ||
| --echo # | ||
| ALTER EVENT test.mdev38632 | ||
| ON SCHEDULE AT CURRENT_TIMESTAMP + INTERVAL 1 SECOND | ||
| ON COMPLETION PRESERVE ENABLE; | ||
|
|
||
| --echo # After fix, last_executed should be cleared when schedule changes. | ||
| SELECT status, last_executed IS NOT NULL AS has_last_exec | ||
| FROM mysql.event WHERE db = 'test' AND name = 'mdev38632'; | ||
|
|
||
| --echo # Restart the scheduler to trigger reload from mysql.event. | ||
| --echo # After fix, the event should remain ENABLED after reload. | ||
| SET GLOBAL event_scheduler = OFF; | ||
| --source include/check_events_off.inc | ||
| SET GLOBAL event_scheduler = ON; | ||
| --source include/running_event_scheduler.inc | ||
|
|
||
| --echo # Status should remain ENABLED after scheduler reload. | ||
| SELECT status FROM mysql.event WHERE db = 'test' AND name = 'mdev38632'; | ||
|
|
||
| --echo # | ||
| --echo # Step 4: User-disabled event should NOT be re-enabled by reschedule | ||
| --echo # | ||
| ALTER EVENT test.mdev38632 | ||
| ON SCHEDULE AT CURRENT_TIMESTAMP + INTERVAL 1 HOUR | ||
| ON COMPLETION PRESERVE ENABLE; | ||
|
|
||
| ALTER EVENT test.mdev38632 DISABLE; | ||
|
|
||
| SELECT status FROM mysql.event WHERE db = 'test' AND name = 'mdev38632'; | ||
|
|
||
| --echo # Reschedule without explicit ENABLE — should stay DISABLED | ||
| ALTER EVENT test.mdev38632 | ||
| ON SCHEDULE AT CURRENT_TIMESTAMP + INTERVAL 2 HOUR; | ||
|
|
||
| SELECT status FROM mysql.event WHERE db = 'test' AND name = 'mdev38632'; | ||
|
|
||
| --echo # | ||
| --echo # Step 5: ALTER with same execute_at should not clear last_executed | ||
| --echo # | ||
| DROP EVENT test.mdev38632; | ||
| CREATE EVENT test.mdev38632 | ||
| ON SCHEDULE AT CURRENT_TIMESTAMP + INTERVAL 1 SECOND | ||
| ON COMPLETION PRESERVE ENABLE | ||
| DO INSERT INTO test.event_log(id) VALUES (NULL); | ||
|
|
||
| let $wait_condition = SELECT COUNT(*) >= 3 FROM test.event_log; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 3 here |
||
| --source include/wait_condition.inc | ||
|
|
||
| --echo # Event executed, now ALTER with same body but no schedule change | ||
| ALTER EVENT test.mdev38632 | ||
| DO INSERT INTO test.event_log(id) VALUES (NULL); | ||
|
|
||
| --echo # Status should remain DISABLED (no schedule change, no re-enable) | ||
| SELECT status FROM mysql.event WHERE db = 'test' AND name = 'mdev38632'; | ||
|
|
||
| --echo # | ||
| --echo # Cleanup | ||
| --echo # | ||
| DROP EVENT IF EXISTS test.mdev38632; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there's going to be an event. not having one should be an error. remove the if exists part. |
||
| DROP TABLE test.event_log; | ||
| SET GLOBAL event_scheduler = OFF; | ||
| --source include/check_events_off.inc | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -319,6 +319,54 @@ mysql_event_fill_row(THD *thd, | |
| MYSQL_TIME time; | ||
| my_tz_OFFSET0->gmt_sec_to_TIME(&time, et->execute_at); | ||
|
|
||
| /* | ||
| MDEV-38632: When ALTER EVENT changes execute_at, clear last_executed | ||
| and re-enable the event if it was auto-disabled by the scheduler. | ||
|
|
||
| Only act when execute_at actually changed. Compare the new value | ||
| against the stored one before overwriting. | ||
|
|
||
| Re-enable only if: the user didn't explicitly set status, the stored | ||
| status is DISABLED, and last_executed >= old execute_at (meaning the | ||
| scheduler disabled it after running at that time). | ||
|
|
||
| Note: this heuristic cannot distinguish "scheduler auto-disabled" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe you need to review your ambition level then: if there's no way to distinguish between auto-disabled and disabled, then you probably shouldn't. The way this typically works is that each individual characteristic of an even must be individually manageable. And if one characteristic (enabled) depends on another (one time event executed) this dependency should be documented as such and not extra promises should be made. Or, if that's deemed bad behavior, then you need to ensure the changes work exactly as advertised by adding extra data. So there won't be a gotcha. But this is just my 2 cents. Please take that with the final reviewer. |
||
| from "user explicitly disabled an already-auto-disabled event" | ||
| since both states look identical in mysql.event. In the rare case | ||
| where a user explicitly disables an already-disabled event and then | ||
| reschedules it, the event would be incorrectly re-enabled. | ||
| */ | ||
| if (is_update) | ||
| { | ||
| bool schedule_changed= true; | ||
| MYSQL_TIME old_execute_at; | ||
|
|
||
| if (!fields[ET_FIELD_EXECUTE_AT]->is_null() && | ||
| !fields[ET_FIELD_EXECUTE_AT]->get_date(&old_execute_at, | ||
| TIME_NO_ZERO_DATE | | ||
| thd->temporal_round_mode())) | ||
| schedule_changed= my_time_compare(&time, &old_execute_at) != 0; | ||
|
|
||
| if (schedule_changed) | ||
| { | ||
| if (!et->status_changed && | ||
| fields[ET_FIELD_STATUS]->val_int() == Event_parse_data::DISABLED && | ||
| !fields[ET_FIELD_LAST_EXECUTED]->is_null()) | ||
| { | ||
| MYSQL_TIME old_last_executed; | ||
| if (!fields[ET_FIELD_LAST_EXECUTED]->get_date(&old_last_executed, | ||
| TIME_NO_ZERO_DATE | | ||
| thd->temporal_round_mode())) | ||
| { | ||
| if (my_time_compare(&old_last_executed, &old_execute_at) >= 0) | ||
| rs|= fields[ET_FIELD_STATUS]->store( | ||
| (longlong)Event_parse_data::ENABLED, TRUE); | ||
| } | ||
| } | ||
| fields[ET_FIELD_LAST_EXECUTED]->set_null(); | ||
| } | ||
| } | ||
|
|
||
| fields[ET_FIELD_EXECUTE_AT]->set_notnull(); | ||
| fields[ET_FIELD_EXECUTE_AT]->store_time(&time); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is going to always be 1, right? why the >= 1 then?