From a6c18b852a8d237343282e6634698b4c563f05d2 Mon Sep 17 00:00:00 2001 From: Thirunarayanan Balathandayuthapani Date: Mon, 4 May 2026 15:08:37 +0530 Subject: [PATCH 1/4] MDEV-34358 Encryption threads consume CPU when no work available Problem: -------- 1. Encryption threads busy-wait when no work is available. When reaching fil_system.space_list.end(), fil_crypt_return_iops() is called with wake=true, causing pthread_cond_broadcast() to wake all threads unnecessarily, leading to CPU waste. 2. Tablespaces with CLOSING/STOPPING flags (set during DDL operations) are skipped during iteration. Since DDL completion doesn't wake encryption threads, these spaces may never be encrypted if threads sleep indefinitely. 3. For default_encrypt_list iteration, when spaces exist but none are acquirable, threads need to wake others for cooperative retry, but this case was not distinguished from fil_system.space_list.end(). 4. IOPS are allocated before searching for tablespaces, wasting resources during iteration when no I/O occurs. Solution: --------- 1. Implement timed wait with exponential backoff (5s -> 10s -> 20s -> 40s -> 60s, max 5 attempts) for fil_system.space_list iteration. After ~135 seconds, switch to indefinite wait. This periodically rechecks for spaces that become available after DDL completes. 2. Use indefinite wait for default_encrypt_list iteration since other threads will retry and wake when needed. 3. fil_space_t::next(): Add default_encrypt_list flag to distinguish between the two iteration modes. Wake other threads only when this flag is true (spaces exist but unacquirable). 4. Move IOPS allocation from before tablespace search to after finding a space that needs rotation. 5. Handle wake logic explicitly at call site based on default_encrypt_list flag. rotate_thread_t changes ------------------------ - default_encrypt_list (bool): Indicates if default_encrypt_list has unacquirable spaces - timed_wait_count (uint8_t): Counts consecutive timeouts for exponential backoff - sleep_timeout_ms (uint16_t): Current timeout in ms (5s -> 60s max) - wait_for_work(): Implements timed/indefinite wait based on iteration mode - increase_sleep_timeout(): Doubles timeout up to 60s - reset_sleep_timeout(): Resets timeout to 5s and clears count --- storage/innobase/fil/fil0crypt.cc | 234 ++++++++++++++++++++++------- storage/innobase/include/fil0fil.h | 7 +- 2 files changed, 182 insertions(+), 59 deletions(-) diff --git a/storage/innobase/fil/fil0crypt.cc b/storage/innobase/fil/fil0crypt.cc index 3d32d1c640bca..f72ebbe474c33 100644 --- a/storage/innobase/fil/fil0crypt.cc +++ b/storage/innobase/fil/fil0crypt.cc @@ -1023,21 +1023,57 @@ static bool fil_crypt_start_encrypting_space(fil_space_t* space) struct rotate_thread_t { explicit rotate_thread_t(uint no) : thread_no(no) {} - uint thread_no; bool first = true; /*!< is position before first space */ + + /** Set when iterating default_encrypt_list. Used to determine + wake behavior:true = wake other threads for cooperative retry, + false = timed wait for fil_system.space_list iteration. */ + bool default_encrypt_list= true; + + /** Number of consecutive timed waits that resulted in timeout (no signal). + Used for exponential backoff when iterating fil_system.space_list: after + 5 timeouts (~135s total: 5+10+20+40+60), switches to indefinite wait to + avoid wasting CPU when spaces remain unavailable. Reset to 0 when woken + by signal or when work is found. */ + uint8_t timed_wait_count= 0; + + /** Current sleep timeout in milliseconds for timed wait. Starts at 5000ms + (5 seconds) and doubles on each timeout up to 60000ms (60 seconds) maximum. + Used only when iterating fil_system.space_list to periodically recheck for + spaces that become available after DDL operations complete. Reset to 5000ms + when woken by signal or when work is found. */ + uint16_t sleep_timeout_ms= 5000; + + uint thread_no; + uint32_t offset = 0; /*!< current page number */ + uint min_key_version_found = 0; /*!< min key version found but not rotated */ + uint estimated_max_iops = 20;/*!< estimation of max iops */ + uint allocated_iops = 0; /*!< allocated iops */ space_list_t::iterator space = fil_system.space_list.end();/*!< current space or .end() */ - uint32_t offset = 0; /*!< current page number */ ulint batch = 0; /*!< #pages to rotate */ - uint min_key_version_found = 0; /*!< min key version found but not rotated */ lsn_t end_lsn = 0; /*!< max lsn when rotating this space */ - uint estimated_max_iops = 20;/*!< estimation of max iops */ - uint allocated_iops = 0; /*!< allocated iops */ ulint cnt_waited = 0; /*!< #times waited during this slot */ uintmax_t sum_waited_us = 0; /*!< wait time during this slot */ + fil_crypt_stat_t crypt_stat; // statistics + + /** Increase sleep timeout for exponential backoff. + Doubles the current timeout up to a maximum of 60 seconds. Called when + timed wait expires without being woken by a signal, indicating no work + became available during the timeout period. */ + void increase_sleep_timeout() { + sleep_timeout_ms = std::min(sleep_timeout_ms * 2, 60000); + } - fil_crypt_stat_t crypt_stat; // statistics + /** Reset sleep timeout to initial value. + Called when thread is woken by signal (config change, new tablespace) + or when work is found, ensuring responsive rechecking when activity + resumes. */ + void reset_sleep_timeout() { + sleep_timeout_ms = 5000; + timed_wait_count = 0; + } /** @return whether this thread should terminate */ bool should_shutdown() const { @@ -1056,6 +1092,55 @@ struct rotate_thread_t { ut_ad(0); return true; } + + + /** Wait for encryption work to become available. + For fil_system.space_list iteration: Uses timed wait with + exponential backoff (5s -> 10s -> 20s -> 40s -> 60s, + max 5 attempts) to periodically recheck for tablespaces + that become available after DDL operations complete. + After 5 timeouts, switches to indefinite wait to avoid + CPU waste. + + For default_encrypt_list iteration: Uses indefinite wait + since nullptr return already indicates temporarily + unacquirable spaces and wakes other threads. + + Timeout resets to 5s when woken by signal (config change, + new tablespace) or when work is found. + + @param recheck If true, skip waiting */ + void wait_for_work(bool recheck) { + + if (recheck || should_shutdown()) { + return; + } + + if (space == fil_system.space_list.end()) { + if (timed_wait_count > 5) { + reset_sleep_timeout(); + timed_wait_count = 0; + goto indefinite_wait; + } + + struct timespec abstime; + set_timespec(abstime, sleep_timeout_ms / 1000); + int ret = my_cond_timedwait(&fil_crypt_threads_cond, + &fil_crypt_threads_mutex.m_mutex, + &abstime); + if (ret == ETIMEDOUT) { + increase_sleep_timeout(); + timed_wait_count++; + } else { + reset_sleep_timeout(); + timed_wait_count= 0; + } + } else { +indefinite_wait: + my_cond_wait(&fil_crypt_threads_cond, + &fil_crypt_threads_mutex.m_mutex); + } + } }; /** Avoid the removal of the tablespace from @@ -1422,20 +1507,33 @@ inline fil_space_t *fil_system_t::default_encrypt_next(fil_space_t *space, return nullptr; } -/** Determine the next tablespace for encryption key rotation. -@param space current tablespace (nullptr to start from the beginning) -@param recheck whether the removal condition needs to be rechecked after -encryption parameters were changed -@param encrypt expected state of innodb_encrypt_tables -@return the next tablespace -@retval fil_system.temp_space if there is no work to do -@retval end() upon reaching the end of the iteration */ +/** Get the next tablespace for encryption key rotation +@param space Current space iterator, or .end() to start from beginning +@param recheck Skip removing processed spaces from default_encrypt_list +@param encrypt Whether encryption is enabled +@param default_encrypt_list set to true when default_encrypt_list + has unacquirable spaces (signals need + to wake other threads); false for + fil_system.space_list iteration +@return iterator to next space to process +Return values depend on which list is being iterated: + +When using default_encrypt_list (fil_crypt_must_default_encrypt() == true): +- Valid space: Space needs processing +- temp_space: List is empty, no work available +- nullptr: Spaces exist in list but none are acquirable (STOPPING/CLOSING) + +When using space_list (fil_crypt_must_default_encrypt() == false): +- Valid space: Space needs processing +- space_list.end(): Reached end of list */ space_list_t::iterator fil_space_t::next(space_list_t::iterator space, - bool recheck, bool encrypt) noexcept + bool recheck, bool encrypt, + bool *default_encrypt_list) noexcept { mysql_mutex_lock(&fil_system.mutex); - if (fil_crypt_must_default_encrypt()) + *default_encrypt_list= fil_crypt_must_default_encrypt(); + if (*default_encrypt_list) { fil_space_t *next_space= fil_system.default_encrypt_next(space == fil_system.space_list.end() @@ -1471,52 +1569,52 @@ space_list_t::iterator fil_space_t::next(space_list_t::iterator space, return space; } -/** Search for a space needing rotation -@param[in,out] key_state Key state -@param[in,out] state Rotation state -@param[in,out] recheck recheck of the tablespace is needed or - still encryption thread does write page 0 -@return whether the thread should keep running */ +/** Find a space that needs rotation +@param key_state Key state +@param state Key rotation state +@param recheck Needs recheck? +@return true if space to rotate found (or reached end of list), +false if shutdown +On return, state->space indicates the result: +- Valid space iterator: Space needs rotation and is ready to process +- fil_system.temp_space: default_encrypt_list is empty (no work exists) +- fil_system.space_list.end(): Reached end of iteration + * For default_encrypt_list: Spaces exist but none are acquirable + (state->default_encrypt_list will be true) + * For fil_system.space_list: All spaces checked (ambiguous - could be + no work or temporarily unacquirable spaces) + +The state->default_encrypt_list flag distinguishes between the two .end() +cases to enable different wake and wait strategies in the caller. */ static bool fil_crypt_find_space_to_rotate( key_state_t* key_state, rotate_thread_t* state, bool* recheck) noexcept { - /* we need iops to start rotating */ - do { - if (state->should_shutdown()) { - if (state->space != fil_system.space_list.end()) { - state->space->release(); - state->space = fil_system.space_list.end(); - } - return false; - } - } while (!fil_crypt_alloc_iops(state)); - if (state->first) { state->first = false; - if (state->space != fil_system.space_list.end()) { + if (state->space != fil_system.space_list.end() && + state->space != + space_list_t::iterator(fil_system.temp_space)) { state->space->release(); } state->space = fil_system.space_list.end(); } state->space = fil_space_t::next(state->space, *recheck, - key_state->key_version != 0); + key_state->key_version != 0, + &state->default_encrypt_list); - bool wake = true; while (state->space != fil_system.space_list.end()) { if (state->space == space_list_t::iterator(fil_system.temp_space)) { - wake = false; - goto done; + return !state->should_shutdown(); } if (state->should_shutdown()) { state->space->release(); -done: state->space = fil_system.space_list.end(); - break; + return false; } mysql_mutex_unlock(&fil_crypt_threads_mutex); @@ -1537,12 +1635,11 @@ static bool fil_crypt_find_space_to_rotate( } state->space = fil_space_t::next(state->space, *recheck, - key_state->key_version != 0); + key_state->key_version != 0, + &state->default_encrypt_list); mysql_mutex_lock(&fil_crypt_threads_mutex); } - /* no work to do; release our allocation of I/O capacity */ - fil_crypt_return_iops(state, wake); return true; } @@ -2029,27 +2126,54 @@ static void fil_crypt_thread() bool recheck = false; wait_for_work: - if (!recheck && !thr.should_shutdown()) { - /* wait for key state changes - * i.e either new key version of change or - * new rotate_key_age */ - my_cond_wait(&fil_crypt_threads_cond, - &fil_crypt_threads_mutex.m_mutex); - } + thr.wait_for_work(recheck); recheck = false; thr.first = true; // restart from first tablespace - key_state_t new_state; /* iterate all spaces searching for those needing rotation */ while (fil_crypt_find_space_to_rotate(&new_state, &thr, &recheck)) { + if (thr.space == fil_system.space_list.end()) { + /* When iterating fil_system.space_list, + reaching .end(), it could mean all spaces + are encrypted, or some spaces were temporarily + unacquirable (CLOSING flag, DDL in progress). + + For default_encrypt_list: Spaces exist but + none are acquirable. Wake other threads + (wake=true) to retry. + + For fil_system.space_list: Use timed wait + (wait_for_work()) to + periodically recheck for spaces that become + available. */ + if (thr.default_encrypt_list) { + pthread_cond_broadcast( + &fil_crypt_threads_cond); + } + + goto wait_for_work; + } + + if (thr.space == + space_list_t::iterator(fil_system.temp_space)) { + /* temp_space : No work exists, don't + wake others */ goto wait_for_work; } /* we found a space to rotate */ + do { + if (thr.should_shutdown()) { + thr.space->release(); + thr.space = fil_system.space_list.end(); + goto func_exit; + } + } while (!fil_crypt_alloc_iops(&thr)); + mysql_mutex_unlock(&fil_crypt_threads_mutex); fil_crypt_start_rotate_space(&new_state, &thr); @@ -2084,14 +2208,8 @@ static void fil_crypt_thread() /* release iops */ fil_crypt_return_iops(&thr); } - - if (thr.space != fil_system.space_list.end()) { - thr.space->release(); - thr.space = fil_system.space_list.end(); - } } - - fil_crypt_return_iops(&thr); +func_exit: srv_n_fil_crypt_threads_started--; pthread_cond_broadcast(&fil_crypt_cond); mysql_mutex_unlock(&fil_crypt_threads_mutex); diff --git a/storage/innobase/include/fil0fil.h b/storage/innobase/include/fil0fil.h index b3a873dc33866..2bed45addf82d 100644 --- a/storage/innobase/include/fil0fil.h +++ b/storage/innobase/include/fil0fil.h @@ -1062,10 +1062,15 @@ struct fil_space_t final @param recheck whether the removal condition needs to be rechecked after encryption parameters were changed @param encrypt expected state of innodb_encrypt_tables + @param default_encrypt_list set to true when default_encrypt_list + has unacquirable spaces (signals need + to wake other threads); + false for fil_system.space_list iteration @return the next tablespace @retval nullptr upon reaching the end of the iteration */ static space_list_t::iterator next(space_list_t::iterator space, - bool recheck, bool encrypt) noexcept; + bool recheck, bool encrypt, + bool *default_encrypt_list) noexcept; #ifdef UNIV_DEBUG bool is_latched() const noexcept { return latch.have_any(); } From 67e8b4f1658bf9e4b2571dc2826b34350b6cffa9 Mon Sep 17 00:00:00 2001 From: Thirunarayanan Balathandayuthapani Date: Mon, 4 May 2026 19:35:44 +0530 Subject: [PATCH 2/4] MDEV-34358: Add debug status variables to verify encryption thread wait behavior Added debug-only status variables Innodb_encryption_timed_waits and Innodb_encryption_indefinite_waits to track encryption thread wait types, enabling verification that threads use timed wait with exponential backoff instead of busy-waiting when idle. The counters are incremented in rotate_thread_t::wait_for_work() and exposed via SHOW STATUS in debug builds only. Also added a debug sync point 'rotate_only_2_timed_waits' to reduce the timed wait threshold from 5 to 2 for faster testing of the indefinite wait transition. --- .../suite/innodb/r/innodb_status_variables.result | 3 ++- .../suite/innodb/t/innodb_status_variables.test | 3 ++- storage/innobase/fil/fil0crypt.cc | 15 ++++++++++++++- storage/innobase/handler/ha_innodb.cc | 6 ++++++ storage/innobase/include/fil0crypt.h | 8 ++++++++ 5 files changed, 32 insertions(+), 3 deletions(-) diff --git a/mysql-test/suite/innodb/r/innodb_status_variables.result b/mysql-test/suite/innodb/r/innodb_status_variables.result index 47d202aa3ac7d..2f87110b8dd91 100644 --- a/mysql-test/suite/innodb/r/innodb_status_variables.result +++ b/mysql-test/suite/innodb/r/innodb_status_variables.result @@ -3,7 +3,8 @@ WHERE variable_name LIKE 'INNODB_%' AND variable_name NOT IN ('INNODB_ADAPTIVE_HASH_HASH_SEARCHES','INNODB_ADAPTIVE_HASH_NON_HASH_SEARCHES', 'INNODB_MEM_ADAPTIVE_HASH', -'INNODB_BUFFERED_AIO_SUBMITTED','INNODB_BUFFER_POOL_PAGES_LATCHED'); +'INNODB_BUFFERED_AIO_SUBMITTED','INNODB_BUFFER_POOL_PAGES_LATCHED', +'INNODB_ENCRYPTION_INDEFINITE_WAITS', 'INNODB_ENCRYPTION_TIMED_WAITS'); variable_name INNODB_BACKGROUND_LOG_SYNC INNODB_BUFFER_POOL_DUMP_STATUS diff --git a/mysql-test/suite/innodb/t/innodb_status_variables.test b/mysql-test/suite/innodb/t/innodb_status_variables.test index 6746a94530fcc..a0571828fbd36 100644 --- a/mysql-test/suite/innodb/t/innodb_status_variables.test +++ b/mysql-test/suite/innodb/t/innodb_status_variables.test @@ -4,4 +4,5 @@ WHERE variable_name LIKE 'INNODB_%' AND variable_name NOT IN ('INNODB_ADAPTIVE_HASH_HASH_SEARCHES','INNODB_ADAPTIVE_HASH_NON_HASH_SEARCHES', 'INNODB_MEM_ADAPTIVE_HASH', - 'INNODB_BUFFERED_AIO_SUBMITTED','INNODB_BUFFER_POOL_PAGES_LATCHED'); + 'INNODB_BUFFERED_AIO_SUBMITTED','INNODB_BUFFER_POOL_PAGES_LATCHED', + 'INNODB_ENCRYPTION_INDEFINITE_WAITS', 'INNODB_ENCRYPTION_TIMED_WAITS'); diff --git a/storage/innobase/fil/fil0crypt.cc b/storage/innobase/fil/fil0crypt.cc index f72ebbe474c33..fffde65f6a387 100644 --- a/storage/innobase/fil/fil0crypt.cc +++ b/storage/innobase/fil/fil0crypt.cc @@ -58,6 +58,14 @@ uint srv_fil_crypt_rotate_key_age; /** Whether the encryption plugin does key rotation */ Atomic_relaxed srv_encrypt_rotate; +#ifdef UNIV_DEBUG +/** Number of times encryption threads used timed wait */ +Atomic_counter fil_crypt_timed_waits; + +/** Number of times encryption threads used indefinite wait */ +Atomic_counter fil_crypt_indefinite_waits; +#endif /* UNIV_DEBUG */ + /** Condition variable for srv_n_fil_crypt_threads_started */ static pthread_cond_t fil_crypt_cond; @@ -1117,12 +1125,16 @@ struct rotate_thread_t { } if (space == fil_system.space_list.end()) { - if (timed_wait_count > 5) { + uint max_timed_waits = 5; + DBUG_EXECUTE_IF("rotate_only_2_timed_waits", + max_timed_waits = 2;); + if (timed_wait_count > max_timed_waits) { reset_sleep_timeout(); timed_wait_count = 0; goto indefinite_wait; } + ut_d(fil_crypt_timed_waits++;); struct timespec abstime; set_timespec(abstime, sleep_timeout_ms / 1000); int ret = my_cond_timedwait(&fil_crypt_threads_cond, @@ -1137,6 +1149,7 @@ struct rotate_thread_t { } } else { indefinite_wait: + ut_d(fil_crypt_indefinite_waits++;); my_cond_wait(&fil_crypt_threads_cond, &fil_crypt_threads_mutex.m_mutex); } diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index d1bcde83b8077..a89e6c0c86acb 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -1106,6 +1106,12 @@ static SHOW_VAR innodb_status_variables[]= { &export_vars.innodb_n_temp_blocks_decrypted, SHOW_LONGLONG}, {"encryption_num_key_requests", &export_vars.innodb_encryption_key_requests, SHOW_LONGLONG}, +#ifdef UNIV_DEBUG + {"encryption_timed_waits", + &fil_crypt_timed_waits, SHOW_SIZE_T}, + {"encryption_indefinite_waits", + &fil_crypt_indefinite_waits, SHOW_SIZE_T}, +#endif /* UNIV_DEBUG */ /* InnoDB bulk operations */ {"bulk_operations", &export_vars.innodb_bulk_operations, SHOW_SIZE_T}, diff --git a/storage/innobase/include/fil0crypt.h b/storage/innobase/include/fil0crypt.h index ab1b668dde68a..c64c638630579 100644 --- a/storage/innobase/include/fil0crypt.h +++ b/storage/innobase/include/fil0crypt.h @@ -375,6 +375,14 @@ Return crypt statistics @param[out] stat Crypt statistics */ void fil_crypt_total_stat(fil_crypt_stat_t *stat); +#ifdef UNIV_DEBUG +/** Number of times encryption threads used timed wait */ +extern Atomic_counter fil_crypt_timed_waits; + +/** Number of times encryption threads used indefinite wait */ +extern Atomic_counter fil_crypt_indefinite_waits; +#endif /* UNIV_DEBUG */ + #include "fil0crypt.inl" #endif /* !UNIV_INNOCHECKSUM */ From 3b3b9a2f61476bd54222f50014f55bc6c7271103 Mon Sep 17 00:00:00 2001 From: Thirunarayanan Balathandayuthapani Date: Tue, 5 May 2026 00:09:40 +0530 Subject: [PATCH 3/4] MDEV-34358 Detect config changes during encryption iteration Problem: ======= When innodb_encrypt_tables or innodb_encryption_rotate_key_age is changed during encryption thread iteration, threads continue with stale configuration values, potentially missing tablespaces that should be encrypted or rotated under the new settings. Solution: ======== Added atomic version counter fil_crypt_settings_version that is incremented whenever innodb_encrypt_tables or innodb_encryption_rotate_key_age changes. Encryption threads capture the version at iteration start and check for changes during iteration. If config changed, threads immediately restart iteration from the beginning to ensure complete coverage with new settings. fil_crypt_settings_version: Atomic counter to track the innodb_encrypt_tables or innodb_encryption_rotate_key_age changes rotate_thread_t::settings_version: To compare the existing fil_crypt_settings_version to restart the encryption from the beginning --- storage/innobase/fil/fil0crypt.cc | 50 +++++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/storage/innobase/fil/fil0crypt.cc b/storage/innobase/fil/fil0crypt.cc index fffde65f6a387..54fb5b1d4ad7d 100644 --- a/storage/innobase/fil/fil0crypt.cc +++ b/storage/innobase/fil/fil0crypt.cc @@ -46,6 +46,12 @@ static bool fil_crypt_threads_inited = false; /** Is encryption enabled/disabled */ ulong srv_encrypt_tables; +/** Version counter for innodb_encrypt_tables changes. +Incremented each time innodb_encrypt_tables or +innodb_encryption_rotate_key_age is modified to signal +encryption threads to restart iteration */ +static Atomic_counter fil_crypt_settings_version; + /** No of key rotation threads requested */ uint srv_n_fil_crypt_threads; @@ -1029,7 +1035,8 @@ static bool fil_crypt_start_encrypting_space(fil_space_t* space) /** State of a rotation thread */ struct rotate_thread_t { - explicit rotate_thread_t(uint no) : thread_no(no) {} + explicit rotate_thread_t(uint no) : + settings_version(fil_crypt_settings_version), thread_no(no){} bool first = true; /*!< is position before first space */ @@ -1052,6 +1059,11 @@ struct rotate_thread_t { when woken by signal or when work is found. */ uint16_t sleep_timeout_ms= 5000; + /** Config version when thread started current iteration. + Used to detect innodb_encrypt_tables changes during iteration + and restart from beginning to ensure complete encryption coverage. */ + uint16_t settings_version; + uint thread_no; uint32_t offset = 0; /*!< current page number */ uint min_key_version_found = 0; /*!< min key version found but not rotated */ @@ -1083,6 +1095,12 @@ struct rotate_thread_t { timed_wait_count = 0; } + /** Check if innodb_encrypt_tables config has changed. + @return true if config changed, requiring iteration restart */ + bool settings_changed() const { + return settings_version != fil_crypt_settings_version; + } + /** @return whether this thread should terminate */ bool should_shutdown() const { mysql_mutex_assert_owner(&fil_crypt_threads_mutex); @@ -1128,9 +1146,8 @@ struct rotate_thread_t { uint max_timed_waits = 5; DBUG_EXECUTE_IF("rotate_only_2_timed_waits", max_timed_waits = 2;); - if (timed_wait_count > max_timed_waits) { + if (timed_wait_count >= max_timed_waits) { reset_sleep_timeout(); - timed_wait_count = 0; goto indefinite_wait; } @@ -1145,7 +1162,6 @@ struct rotate_thread_t { timed_wait_count++; } else { reset_sleep_timeout(); - timed_wait_count= 0; } } else { indefinite_wait: @@ -2137,23 +2153,38 @@ static void fil_crypt_thread() /* if we find a tablespace that is starting, skip over it and recheck it later */ bool recheck = false; - wait_for_work: thr.wait_for_work(recheck); +restart_iteration: recheck = false; thr.first = true; // restart from first tablespace + thr.settings_version = fil_crypt_settings_version; key_state_t new_state; /* iterate all spaces searching for those needing rotation */ while (fil_crypt_find_space_to_rotate(&new_state, &thr, &recheck)) { + /* Check if innodb_encrypt_tables or + innodb_encryption_rotate_key_age changed during + iteration. If changed, restart immediately */ + if (thr.settings_changed()) { + + if (thr.space != fil_system.space_list.end() && + thr.space != space_list_t::iterator( + fil_system.temp_space)) { + thr.space->release(); + } + + thr.space = fil_system.space_list.end(); + goto restart_iteration; + } if (thr.space == fil_system.space_list.end()) { /* When iterating fil_system.space_list, reaching .end(), it could mean all spaces are encrypted, or some spaces were temporarily - unacquirable (CLOSING flag, DDL in progress). + unacquirable (CLOSING flag, DDL in progress). For default_encrypt_list: Spaces exist but none are acquirable. Wake other threads @@ -2329,6 +2360,10 @@ void fil_crypt_set_rotate_key_age(uint val) if (val == 0) fil_crypt_default_encrypt_tables_fill(); mysql_mutex_unlock(&fil_system.mutex); + + /* Increment version to signal threads to restart iteration */ + fil_crypt_settings_version++; + pthread_cond_broadcast(&fil_crypt_threads_cond); mysql_mutex_unlock(&fil_crypt_threads_mutex); } @@ -2383,6 +2418,9 @@ void fil_crypt_set_encrypt_tables(ulong val) mysql_mutex_unlock(&fil_system.mutex); + /* Increment version to signal threads to restart iteration */ + fil_crypt_settings_version++; + pthread_cond_broadcast(&fil_crypt_threads_cond); mysql_mutex_unlock(&fil_crypt_threads_mutex); } From 44bf327b649db1682cdc8d6042563c2ada680972 Mon Sep 17 00:00:00 2001 From: Thirunarayanan Balathandayuthapani Date: Tue, 5 May 2026 13:53:31 +0530 Subject: [PATCH 4/4] - Addressing the review comments --- storage/innobase/fil/fil0crypt.cc | 45 ++++++++++++++++++------------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/storage/innobase/fil/fil0crypt.cc b/storage/innobase/fil/fil0crypt.cc index 54fb5b1d4ad7d..78c986355f79c 100644 --- a/storage/innobase/fil/fil0crypt.cc +++ b/storage/innobase/fil/fil0crypt.cc @@ -1083,7 +1083,9 @@ struct rotate_thread_t { timed wait expires without being woken by a signal, indicating no work became available during the timeout period. */ void increase_sleep_timeout() { - sleep_timeout_ms = std::min(sleep_timeout_ms * 2, 60000); + sleep_timeout_ms = std::min( + static_cast(sleep_timeout_ms * 2u), + static_cast(60000)); } /** Reset sleep timeout to initial value. @@ -1126,14 +1128,15 @@ struct rotate_thread_t { max 5 attempts) to periodically recheck for tablespaces that become available after DDL operations complete. After 5 timeouts, switches to indefinite wait to avoid - CPU waste. + CPU waste. After 5 timeouts, switches to indefinite wait + to avoid CPU waste. - For default_encrypt_list iteration: Uses indefinite wait - since nullptr return already indicates temporarily - unacquirable spaces and wakes other threads. + Timeout resets to 5s when woken by signal when work is found. - Timeout resets to 5s when woken by signal (config change, - new tablespace) or when work is found. + Reason for timed wait even with default_encrypt_list: + Single thread scenario: No other threads to wake this one if + all spaces are temporarily unacquirable + Simplicity: Uniform wait strategy for all iteration modes @param recheck If true, skip waiting */ void wait_for_work(bool recheck) { @@ -1325,10 +1328,6 @@ static bool fil_crypt_alloc_iops(rotate_thread_t *state) mysql_mutex_assert_owner(&fil_crypt_threads_mutex); ut_ad(state->allocated_iops == 0); - /* We have not yet selected the space to rotate, thus - state might not contain space and we can't check - its status yet. */ - uint max_iops = state->estimated_max_iops; if (n_fil_crypt_iops_allocated >= srv_n_fil_crypt_iops) { @@ -2209,15 +2208,23 @@ static void fil_crypt_thread() goto wait_for_work; } - /* we found a space to rotate */ - do { - if (thr.should_shutdown()) { - thr.space->release(); - thr.space = fil_system.space_list.end(); - goto func_exit; - } - } while (!fil_crypt_alloc_iops(&thr)); + if (thr.should_shutdown()) { + thr.space->release(); + thr.space = fil_system.space_list.end(); + goto func_exit; + } + + bool iops_allocated = fil_crypt_alloc_iops(&thr); + + if (!iops_allocated) { + /* IOPS not available. Release space and restart iteration + to find a space again after IOPS become available. */ + thr.space->release(); + thr.space = fil_system.space_list.end(); + continue; + } + thr.reset_sleep_timeout(); mysql_mutex_unlock(&fil_crypt_threads_mutex); fil_crypt_start_rotate_space(&new_state, &thr);