MDEV-38632 ALTER EVENT doesn't re-run one-time AT event after first execution#5054
MDEV-38632 ALTER EVENT doesn't re-run one-time AT event after first execution#5054keeper wants to merge 1 commit intoMariaDB:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request addresses MDEV-38632, ensuring that one-time events with ON COMPLETION PRESERVE are correctly rescheduled and re-enabled when modified via ALTER EVENT. The changes update the event queue logic to check if a one-time event is pending based on its execution timestamp and modify the database repository to clear execution history and restore the ENABLED status upon schedule changes. Review feedback highlights the need for safer error handling when retrieving timestamps in sql/event_db_repository.cc to avoid potential undefined behavior from uninitialized memory if get_date fails.
| MYSQL_TIME old_execute_at; | ||
| fields[ET_FIELD_EXECUTE_AT]->get_date(&old_execute_at, | ||
| TIME_NO_ZERO_DATE | | ||
| thd->temporal_round_mode()); |
There was a problem hiding this comment.
If get_date fails (for example, when the field contains a zero date and TIME_NO_ZERO_DATE is used, which occurs when converting an EVERY event to an AT event), old_execute_at remains uninitialized. This leads to undefined behavior in the subsequent my_time_compare call. It is safer to initialize the structure or check the return value of get_date and zero the structure on failure.
| MYSQL_TIME old_execute_at; | |
| fields[ET_FIELD_EXECUTE_AT]->get_date(&old_execute_at, | |
| TIME_NO_ZERO_DATE | | |
| thd->temporal_round_mode()); | |
| MYSQL_TIME old_execute_at; | |
| if (fields[ET_FIELD_EXECUTE_AT]->get_date(&old_execute_at, | |
| TIME_NO_ZERO_DATE | | |
| thd->temporal_round_mode())) | |
| set_zero_time(&old_execute_at, MYSQL_TIMESTAMP_DATETIME); |
| MYSQL_TIME old_last_executed; | ||
| fields[ET_FIELD_LAST_EXECUTED]->get_date(&old_last_executed, | ||
| TIME_NO_ZERO_DATE | | ||
| thd->temporal_round_mode()); |
There was a problem hiding this comment.
Similarly to old_execute_at, old_last_executed should be handled safely if get_date fails to avoid using uninitialized memory in the subsequent my_time_compare call, even though it is currently guarded by a null check.
| MYSQL_TIME old_last_executed; | |
| fields[ET_FIELD_LAST_EXECUTED]->get_date(&old_last_executed, | |
| TIME_NO_ZERO_DATE | | |
| thd->temporal_round_mode()); | |
| MYSQL_TIME old_last_executed; | |
| if (fields[ET_FIELD_LAST_EXECUTED]->get_date(&old_last_executed, | |
| TIME_NO_ZERO_DATE | | |
| thd->temporal_round_mode())) | |
| set_zero_time(&old_last_executed, MYSQL_TIMESTAMP_DATETIME); |
gkodinov
left a comment
There was a problem hiding this comment.
Thank you for your contribution. Please fix the tests failing on debug builds with a dbug assert as follows:
Thread 1 (Thread 0x7f696e2c1640 (LWP 281786)):
#0 0x00007f69751019fc in pthread_kill () from /lib/x86_64-linux-gnu/libc.so.6
No symbol table info available.
#1 0x0000557c6275b316 in my_write_core (sig=6) at /home/buildbot/amd64-ubuntu-2204-debug/build/mysys/stacktrace.c:424
No locals.
#2 0x0000557c61ccebdc in handle_fatal_signal (sig=6) at /home/buildbot/amd64-ubuntu-2204-debug/build/sql/signal_handler.cc:298
curr_time = 1778188402
tm = {tm_sec = 22, tm_min = 13, tm_hour = 21, tm_mday = 7, tm_mon = 4, tm_year = 126, tm_wday = 4, tm_yday = 126, tm_isdst = 0, tm_gmtoff = 0, tm_zone = 0x7f6975244afb "UTC"}
thd = 0x7f6948004e68
#3 <signal handler called>
No symbol table info available.
#4 0x00007f69751019fc in pthread_kill () from /lib/x86_64-linux-gnu/libc.so.6
No symbol table info available.
#5 0x00007f69750ad476 in raise () from /lib/x86_64-linux-gnu/libc.so.6
No symbol table info available.
#6 0x00007f69750937f3 in abort () from /lib/x86_64-linux-gnu/libc.so.6
No symbol table info available.
#7 0x00007f697509371b in ?? () from /lib/x86_64-linux-gnu/libc.so.6
No symbol table info available.
#8 0x00007f69750a4e96 in __assert_fail () from /lib/x86_64-linux-gnu/libc.so.6
No symbol table info available.
#9 0x0000557c61c85ed8 in TIME_from_longlong_datetime_packed (ltime=0x7f696e2bf830, tmp=-9223372036854775808) at /home/buildbot/amd64-ubuntu-2204-debug/build/sql/compat56.cc:270
ymd = 140090158264344
hms = -549755813888
ymdhms = 1848375056
ym = 140090796668720
__PRETTY_FUNCTION__ = "void TIME_from_longlong_datetime_packed(MYSQL_TIME*, longlong)"
#10 0x0000557c61ca2a95 in Field_datetimef::get_TIME (this=0x7f69481b7068, ltime=0x7f696e2bf830, pos=0x7f69481eb018 "", fuzzydate={m_mode = (date_mode_t::FRAC_TRUNCATE | date_mode_t::NO_ZERO_DATE)}) at /home/buildbot/amd64-ubuntu-2204-debug/build/sql/field.cc:7387
__PRETTY_FUNCTION__ = "virtual bool Field_datetimef::get_TIME(MYSQL_TIME*, const uchar*, date_mode_t) const"
tmp = -9223372036854775808
#11 0x0000557c61cb84ce in Field_datetimef::get_date (this=0x7f69481b7068, ltime=0x7f696e2bf830, fuzzydate={m_mode = (date_mode_t::FRAC_TRUNCATE | date_mode_t::NO_ZERO_DATE)}) at /home/buildbot/amd64-ubuntu-2204-debug/build/sql/field.h:4122
No locals.
#12 0x0000557c61f3c213 in mysql_event_fill_row (thd=0x7f6948004e68, table=0x7f69480f1a68, et=0x7f694801b1b8, sp=0x0, sql_mode=0, is_update=1 '\001') at /home/buildbot/amd64-ubuntu-2204-debug/build/sql/event_db_repository.cc:342
old_execute_at = {year = 0, month = 0, day = 0, hour = 0, minute = 0, second = 0, second_part = 0, neg = 0 '\000', time_type = MYSQL_TIMESTAMP_DATE}
schedule_changed = false
tz_name = 0x557c638c3980 <tz_SYSTEM_name>
time = {year = 1970, month = 1, day = 2, hour = 0, minute = 0, second = 0, second_part = 0, neg = 0 '\000', time_type = MYSQL_TIMESTAMP_DATETIME}
scs = 0x557c6388be60 <my_charset_utf8mb3_general1400_as_ci>
f_num = ET_FIELD_NAME
fields = 0x7f69481ebc68
rs = 0
__PRETTY_FUNCTION__ = "bool mysql_event_fill_row(THD*, TABLE*, Event_parse_data*, sp_head*, sql_mode_t, my_bool)"
#13 0x0000557c61f3d545 in Event_db_repository::update_event (this=0x557c6e07f4a0, thd=0x7f6948004e68, parse_data=0x7f694801b1b8, new_dbname=0x0, new_name=0x0) at /home/buildbot/amd64-ubuntu-2204-debug/build/sql/event_db_repository.cc:891
scs = 0x557c6388be60 <my_charset_utf8mb3_general1400_as_ci>
table = 0x7f69480f1a68
sp = 0x0
saved_mode = 0
mdl_savepoint = {m_stmt_ticket = 0x7f69481418f0, m_trans_ticket = 0x7f694825f9d0}
ret = 1
__PRETTY_FUNCTION__ = "bool Event_db_repository::update_event(THD*, Event_parse_data*, LEX_CSTRING*, LEX_CSTRING*)"
#14 0x0000557c619f9917 in Events::update_event (thd=0x7f6948004e68, parse_data=0x7f694801b1b8, new_dbname=0x0, new_name=0x0) at /home/buildbot/amd64-ubuntu-2204-debug/build/sql/events.cc:530
ret = 1665952136
save_binlog_format = BINLOG_FORMAT_STMT
new_element = 0x557c61a55989 <MDL_context::release_transactional_locks(THD*)+209>
__PRETTY_FUNCTION__ = "static bool Events::update_event(THD*, Event_parse_data*, LEX_CSTRING*, LEX_CSTRING*)"
#15 0x0000557c6181f97f in mysql_execute_command (thd=0x7f6948004e68, is_called_from_prepared_stmt=false) at /home/buildbot/amd64-ubuntu-2204-debug/build/sql/sql_parse.cc:5191
res = 0
lex = 0x7f69480094a0
select_lex = 0x7f6948009e00
first_table = 0x0
all_tables = 0x0
unit = 0x7f6948009580
__PRETTY_FUNCTION__ = "int mysql_execute_command(THD*, bool)"
ots = {ctx = 0x7f6948009200, traceable = false}
orig_binlog_format = BINLOG_FORMAT_MIXED
orig_current_stmt_binlog_format = BINLOG_FORMAT_STMT
#16 0x0000557c6182821c in mysql_parse (thd=0x7f6948004e68, rawbuf=0x7f694801b030 "ALTER EVENT event_starts_test ON SCHEDULE AT '1970-01-02 00:00:00' ON COMPLETION PRESERVE DISABLE", length=97, parser_state=0x7f696e2c01d0) at /home/buildbot/amd64-ubuntu-2204-debug/build/sql/sql_parse.cc:7941
found_semicolon = 0x0
error = 32617
lex = 0x7f69480094a0
err = false
__PRETTY_FUNCTION__ = "void mysql_parse(THD*, char*, uint, Parser_state*)"
#17 0x0000557c61814e46 in dispatch_command (command=COM_QUERY, thd=0x7f6948004e68, packet=0x7f694808f299 "ALTER EVENT event_starts_test ON SCHEDULE AT '1970-01-02 00:00:00' ON COMPLETION PRESERVE DISABLE", packet_length=97, blocking=true) at /home/buildbot/amd64-ubuntu-2204-debug/build/sql/sql_parse.cc:1898
packet_end = 0x7f694801b091 ""
parser_state = {m_lip = {lookahead_token = -1, lookahead_yylval = 0x0, m_thd = 0x7f6948004e68, m_ptr = 0x7f694801b092 "\v", m_tok_start = 0x7f694801b092 "\v", m_tok_end = 0x7f694801b092 "\v", m_end_of_query = 0x7f694801b091 "", m_tok_start_prev = 0x7f694801b091 "", m_buf = 0x7f694801b030 "ALTER EVENT event_starts_test ON SCHEDULE AT '1970-01-02 00:00:00' ON COMPLETION PRESERVE DISABLE", m_buf_length = 97, m_echo = true, m_echo_saved = true, m_cpp_buf = 0x7f694801b0f0 "ALTER EVENT event_starts_test ON SCHEDULE AT '1970-01-02 00:00:00' ON COMPLETION PRESERVE DISABLE", m_cpp_ptr = 0x7f694801b151 "", m_cpp_tok_start = 0x7f694801b151 "", m_cpp_tok_start_prev = 0x7f694801b151 "", m_cpp_tok_end = 0x7f694801b151 "", m_body_utf8 = 0x0, m_body_utf8_ptr = 0x7f69481096f1 '\217' <repeats 200 times>..., m_cpp_utf8_processed_ptr = 0x0, next_state = MY_LEX_END, found_semicolon = 0x0, ignore_space = false, stmt_prepare_mode = false, multi_statements = true, hint_comment = false, yylineno = 1, m_digest = 0x0, in_comment = NO_COMMENT, in_comment_saved = (DISCARD_COMMENT | unknown: 0x5a5a5a58), m_cpp_text_start = 0x7f694801b11e "1970-01-02 00:00:00' ON COMPLETION PRESERVE DISABLE", m_cpp_text_end = 0x7f694801b131 "' ON COMPLETION PRESERVE DISABLE", m_underscore_cs = 0x0}, m_yacc = {yacc_yyss = 0x0, yacc_yyvs = 0x0, m_set_signal_info = {m_item = {0x0 <repeats 13 times>}}, m_lock_type = TL_READ_DEFAULT, m_mdl_type = MDL_SHARED_READ}, m_digest_psi = 0x7f6948008f88}
net = 0x7f6948005270
error = false
do_end_of_statement = true
log_slow_done = false
drop_more_results = false
__PRETTY_FUNCTION__ = "dispatch_command_return dispatch_command(enum_server_command, THD*, char*, uint, bool)"
__FUNCTION__ = "dispatch_command"
res = <optimized out>
#18 0x0000557c6181398c in do_command (thd=0x7f6948004e68, blocking=true) at /home/buildbot/amd64-ubuntu-2204-debug/build/sql/sql_parse.cc:1432
return_value = DISPATCH_COMMAND_SUCCESS
packet = 0x7f694808f298 "\003ALTER EVENT event_starts_test ON SCHEDULE AT '1970-01-02 00:00:00' ON COMPLETION PRESERVE DISABLE"
packet_length = 98
net = 0x7f6948005270
command = COM_QUERY
__PRETTY_FUNCTION__ = "dispatch_command_return do_command(THD*, bool)"
__FUNCTION__ = "do_command"
#19 0x0000557c61a3d0d1 in do_handle_one_connection (connect=0x557c6e17f6a8, put_in_cache=true) at /home/buildbot/amd64-ubuntu-2204-debug/build/sql/sql_connect.cc:1503
create_user = true
thr_create_utime = 18796833249641
thd = 0x7f6948004e68
#20 0x0000557c61a3ce77 in handle_one_connection (arg=0x557c6e0aed58) at /home/buildbot/amd64-ubuntu-2204-debug/build/sql/sql_connect.cc:1415
connect = 0x557c6e0aed58
#21 0x0000557c6205898d in pfs_spawn_thread (arg=0x557c6e0aee38) at /home/buildbot/amd64-ubuntu-2204-debug/build/storage/perfschema/pfs.cc:2198
typed_arg = 0x557c6e0aee38
user_arg = 0x557c6e0aed58
user_start_routine = 0x557c61a3ce09 <handle_one_connection(void*)>
pfs = 0x7f69746fe940
klass = 0x557c6d898940
…xecution A one-time event (ON SCHEDULE AT ... ON COMPLETION PRESERVE) could not be rescheduled via ALTER EVENT after its first execution. The event would silently fail to fire again. Root cause: two bugs in the event scheduler. 1. After a one-time AT event executes, compute_next_execution_time() sets status=DISABLED and update_timing_fields_for_event() persists both DISABLED status and last_executed to mysql.event. When ALTER EVENT changes the schedule without explicit ENABLE, status_changed is false, so the DISABLED status is never overwritten in the table. Event_queue::update_event() loads the element as DISABLED and discards it without queuing. 2. Even with explicit ENABLE, compute_next_execution_time() checks only `if (last_executed)` for AT events and unconditionally sets status=DISABLED. Since last_executed is never cleared on reschedule, the event gets re-disabled on every scheduler reload or queue insertion. Fix: - event_db_repository.cc (mysql_event_fill_row): When ALTER EVENT changes execute_at to a new value, clear last_executed in mysql.event. Also re-enable the event if it was auto-disabled by the scheduler after execution — detected by: status_changed is false (user didn't explicitly set ENABLE/DISABLE), stored status is DISABLED, and last_executed >= old execute_at (the scheduler disabled it after running at the previously scheduled time). This avoids incorrectly re-enabling events the user explicitly disabled. - event_data_objects.cc (compute_next_execution_time): For one-time events, only disable if last_executed >= execute_at, meaning the event was actually executed for the current schedule. If execute_at is after last_executed (rescheduled), treat it as pending. This serves as a safety net for server restarts where last_executed may persist from an older schedule. All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc.
e91440e to
cb709ba
Compare
|
Hi @gkodinov , thanks for review! I've addressed the review from Gemini, by:
and that solves the assertion failure in debug test. |
Description
A one-time event (ON SCHEDULE AT ... ON COMPLETION PRESERVE) silently fails to fire after being rescheduled via
ALTER EVENT.Root Cause
Two bugs in the event scheduler:
Bug 1 — DISABLED status persists after reschedule: After a one-time AT event executes,
compute_next_execution_time()setsstatus=DISABLEDandupdate_timing_fields_for_event()persists both the DISABLED status and last_executed tomysql.event. WhenALTER EVENTchanges the schedule without explicit ENABLE,status_changedis false, so the DISABLED status is never overwritten in the table.Event_queue::update_event()loads the element as DISABLED and discards it without queuing.Bug 2 — Stale
last_executedre-disables the event: Even with explicit ENABLE,compute_next_execution_time()checks onlyif (last_executed)for AT events and unconditionally setsstatus=DISABLED. Sincelast_executedis never cleared on reschedule, the event gets re-disabled on server restart or scheduler reload viaload_events_from_db()→create_event()→compute_next_execution_time(). This is the path that affected the original reporter on AWS RDS, where a server restart between the original execution and theALTERcaused the rescheduled event to be silently dropped from the queue.Fix
event_db_repository.cc(mysql_event_fill_row): WhenALTER EVENTchangesexecute_atto a new value, clearlast_executedinmysql.event. Also re-enable the event if it was auto-disabled by the scheduler after execution — detected by:status_changedis false (user didn't explicitly set ENABLE/DISABLE), stored status is DISABLED, andlast_executed >= old execute_at(the scheduler disabled it after running at the previously scheduled time). This avoids incorrectly re-enabling events the user explicitly disabled.event_data_objects.cc(compute_next_execution_time): For one-time events, only disable iflast_executed >= execute_at, meaning the event was actually executed for the current schedule. Ifexecute_atis afterlast_executed(rescheduled), treat it as pending. This serves as a safety net for server restarts wherelast_executedmay persist from an older schedule.How can this PR be tested?
After the change, MTR test
main.mdev38632_alter_onetime_eventshould pass, otherwise it will fail, shows the scheduler was not fired.The MTR test covers:
Copyright
All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc.