Skip to content
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

Fix the spelling of preceding, specified, received and additionally #2

Closed
wants to merge 1 commit into from

Conversation

ottok
Copy link

@ottok ottok commented Dec 2, 2014

I am tired of fixing the same bugs in different downstream packages I work with, so I decided to push my fixes directly to the source at Oracle MySQL. This simple spelling fix is my first in a series of multiple fixes.

I hereby grant Oracle full rights regarding this contribution. Please accept my fix!

@morgo
Copy link

morgo commented Dec 2, 2014

Hi Otto, thank you for the contribution!

We are not currently accepting pull requests from GitHub. May I suggest filing a bug report on bugs.mysql.com?

(I would also be happy to do so on your behalf.)

@ottok
Copy link
Author

ottok commented Dec 3, 2014

Ok, thanks. I filed the pull request now at http://bugs.mysql.com/bug.php?id=75084

@morgo
Copy link

morgo commented Dec 3, 2014

Thank you Otto! I will close this pull request for now.

@morgo morgo closed this Dec 3, 2014
MyDanny pushed a commit that referenced this pull request Mar 10, 2015
Patch #2: As a consequence of earlier patches, my_afree() is
no longer needed. Remove all usage of it.
MyDanny pushed a commit that referenced this pull request Mar 10, 2015
…rows.

Background 1:

When binlog_format = row, CREATE ... SELECT is logged in two pieces,
like:

Anonymous_Gtid
query_log_event(CREATE TABLE without SELECT)
Anonymous_Gtid
query_log_event(BEGIN)
...row events...
query_log_event(COMMIT) (or Xid_log_event)

Internally, there is a call to MYSQL_BIN_LOG::commit after the table has
been created and before the rows are selected.

When gtid_next='ANONYMOUS', we must not release anonymous ownership for
the commit occurring in the middle of the statement (since that would
allow a concurrent client to set gtid_mode=on, making it impossible to
commit the rest of the statement). Also, the commit in the middle of the
statement should not decrease the counter of ongoing GTID-violating
transactions, since that would allow a concurrent client to set
ENFORCE_GTID_CONSISTENCY=ON even if there is an ongoing transaction that
violates GTID-consistency.

The logic to skip releasing anonymous ownership and skip decreasing the
counter is as follows. Before calling mysql_bin_log.commit, it sets the
flag thd->is_commit_in_middle_of_statement. Eventually,
mysql_bin_log.commit calls gtid_state->update_on_commit, which calls
gtid_state->update_gtids_impl, which reads the
thd->is_commit_in_middle_of_statement and accordingly decides to skip
releasing anonymous ownership and/or skips decreasing the counter.

Problem 1:

When thd->is_commit_in_middle_of_statement has been set, it is crucial
that there is another call to update_gtids_impl when the transaction
ends (otherwise the session will keep holding anonymous ownership and
will not decrease the counters). Normally, this happens because
mysql_bin_log.commit is always called, and mysql_bin_log.commit normally
invokes ordered_commit, which calls update_gtids_impl. However, in case
the SELECT part of the statement does not find any rows,
mysql_bin_log.commit skips the call to ordered_commit, so
update_gtids_impl does not get called. This is the first problem we fix
in this commit.

Fix 1:

We fix this problem as follows. After calling mysql_bin_log.commit to
log the CREATE part of CREATE...SELECT, the CREATE...SELECT code sets
thd->pending_gtid_state_update=true (this is a new flag that we
introduce in this patch). If the flag is set, update_gtids_impl clears
it. At the end of mysql_bin_log.commit, we check the flag to see if
update_gtids_impl has been called by any function invoked by
mysql_bin_log.commit. If not, i.e., if the flag is still true at the end
of mysql_bin_log.commit, it means we have reached the corner case where
update_gtids_impl was skipped. Thus we call it explicitly from
mysql_bin_log.commit.

Background 2:

GTID-violating DDL (CREATE...SELECT and CREATE TEMPORARY) is detected in
is_ddl_gtid_compatible, called from gtid_pre_statement_checks, which is
called from mysql_parse just before the implicit pre-commit.
is_ddl_gtid_compatible determines whether an error or warning or nothing
is to be generated, and whether to increment the counters of GTID-
violating transactions. In case an error is generated, it is important
that the error happens before the implicit commit, so that the statement
fails before it commits the ongoing transaction.

Problem 2:

In case a warning is to be generated, and there is an ongoing
transaction, the implicit commit will write to the binlog, and thus it
will call gtid_state->update_gtids_impl, which will decrease the
counters of GTID-violating transactions. Thus, the counters will be zero
for the duration of the transaction.

Fix 2:

We call is_ddl_gtid_compatible *both* before the implicit commit and
after the implicit commit. If an error is to be generated, the error is
generated before the commit. If a warning is to be generated and/or the
counter of GTID-violating transactions is to be increased, then this
happens after the commit.

Code changes #1:

@sql/binlog.cc
- Move MYSQL_BIN_LOG::commit to a new function
MYSQL_BIN_LOG::write_binlog_and_commit_engine. Make
MYSQL_BIN_LOG::commit call this function, and after the return check
thd->pending_gtid_state_update to see if another call to
gtid_state->update_on_[commit|rollback] is needed.
- Simplify MYSQL_BIN_LOG::write_bin_log_and_commit_engine; remove
useless local variable 'error' that would never change its value.

@sql/binlog.h:
- Declaration of new function.

@sql/rpl_gtid_state.cc:
- Set thd->pending_gtid_state_update to false at the end of
update_gtids_impl.

Code changes #2:
@sql/binlog.cc:
- Add two parameters to handle_gtid_consistency and is_ddl_compatible:
handle_error is true in the call to is_ddl_gtid_compatible that happens
*before* the implicit commit and fals in the call to
is_ddl_gtid_compatible that happens *after* the implicit commit. It
tells the function to generate the error, if an error is to be
generated. The other parameter, handle_nonerror, is true in the call to
is_ddl_gtid_compatible that happens *after* the implicit commit and
false in the call that happens *before* the implicit commit. It tells
the function to generate the warnings and increment the counter, if that
needs to be done.

@sql/rpl_gtid_execution.cc:
- Call is_ddl_gtid_compatible after the implicit commit. Pass the two
new parameters to the function.

@sql/sql_class.h:
- Update prototype for is_ddl_gtid_compatible.

@sql/sql_insert.cc:
- Set thd->pending_gtid_state_update = true after committing the CREATE
part of a CREATE...SELECT.

Misc changes:

@sql/binlog.cc
- Add DEBUG_SYNC symbol end_decide_logging_format used in tests.

@sql/rpl_gtid_state.cc:
- For modularity, move out parts of update_gtids_impl to a new function,
end_gtid_violating_transaction.
- Move the lock/unlock of global_sid_lock into update_gtids_impl.
- Make update_gtids_impl release global_sid_lock before the call to
end_gtid_violating_transaction, so as to hold it as short as possible.

@sql/rpl_gtid.h
- Because we release the locks earlier in update_gtids_impl in
rpl_gtid_state.cc, we need to acquire the lock again in
end_[anonymous|automatic]_gtid_violating_transaction, in order to do
some debug assertions.
- Add DBUG_PRINT for the counters.

Test changes:
- Split binlog_enforce_gtid_consistency into six tests, depending on the
type of scenarios it tests:
Three classes of GTID-violation:
    *_create_select_*: test CREATE ... SELECT.
    *_tmp_*: test CREATE TEMPORARY/DROP TEMPORARY inside a transaction.
    *_trx_nontrx_*: test combinations of transactional and
    non-transactional updates in the same statement or in the same
    transaction.
For each class of GTID-violation, one positive and one negative test:
    *_consistent.test: Cases which are *not* GTID-violating
    *_violation.test: Cases which *are* GTID-violating.
- The general logic of these test is:
- extra/binlog_tests/enforce_gtid_consistency.test iterates over all
    values of GTID_MODE, ENFORCE_GTID_CONSISTENCY, and GTID_NEXT. For
    each case, it sources file; the name of the sourced file is
    specified by the file that sources
    extra/binlog_tests/enforce_gtid_consistency.test
- The top-level file in suite/binlog/t invokes
    extra/binlog_tests/enforce_gtid_consistency.test, specifying one
    of the filenames
    extra/binlog_tests/enforce_gtid_consistency_[create_select|tmp|
    trx_nontrx]_[consistent|violation].test.
- Each of the files
    extra/binlog_tests/enforce_gtid_consistency_[create_select|tmp|
    trx_nontrx]_[consistent|violation].test
    sets up a number of test scenarios. Each test scenario is executed
    by sourcing
    extra/binlog_tests/enforce_gtid_consistency_statement.inc.
- extra/binlog_tests/enforce_gtid_consistency_statement.inc executes
    the specified statement, checks that warnings are generated and
    counters incremented/decremented as specified by the caller.
- Since the tests set GTID_MODE explicitly, it doesn't make sense to run
the test in both combinations GTID_MODE=ON/OFF. However, for the
*_trx_nontrx_* cases, it is important to test that it works both with
binlog_direct_non_transactional_updates=on and off. The suite is never
run with those combinations. To leverage from the combinations
GTID_MODE=ON/OFF, we run the test with
binlog_direct_non_transactional_updates=on if GTID_MODE=ON, and we run
the test with binlog_direct_non_transactional_updates=off if
GTID_MODE=OFF.
bkandasa pushed a commit that referenced this pull request Aug 4, 2015
bkandasa pushed a commit that referenced this pull request Aug 4, 2015
Two places in replication code were causing Valgrind errors:

 1. Memory leak in Relay_log_info::wait_for_gtid_set, since it passed
    the return value from Gtid_set::to_string() directly to
    DBUG_PRINT, without storing it anywhere so that it can be freed.

 2. In MYSQL_BIN_LOG::init_gtid_sets would pass a bad pointer to
    DBUG_PRINT in some cases.

In problem #1, an underlying problem was that to_string returns newly
allocated memory and this was easy to miss when reading the code that
calls the function.  It would be better to return the value through a
parameter, since that forces the caller to store it in a variable, and
then it is more obvious that the value must be freed.  And in fact
such a function existed already, so we fix the problem by removing the
(redundant) no-args version of Gtid_set::to_string and using the one-
or two-argument function instead.

In problem #2, print an empty string if we detect that the pointer
will be bad.

These bugs were found when adding some debug printouts to
read_gtids_from_binlog.  These debug printouts never made it to the
server code through any other bug report, but would be useful to have
for future debugging, so including them in this patch.

Additionally, removed the call to global_sid_lock->rdlock() used
before Previous_gtids_log_event::get_str().  This is not needed since
Previous_gtids_log_event doesn't use shared resources.
bjornmu pushed a commit that referenced this pull request Oct 21, 2015
Bug #21306392: REMOVE OLD-STYLE MAX_STATEMENT_TIME HINT (REPLACE WITH MAX_EXECUTION_TIME)

After-push test update #2.
bjornmu pushed a commit that referenced this pull request Oct 21, 2015
…TO SELF

Problem: If a multi-column update statement fails when updating one of
the columns in a row, it will go on and update the remaining columns
in that row before it stops and reports an error. If the failure
happens when updating a JSON column, and the JSON column is also
referenced later in the update statement, new and more serious errors
can happen when the update statement attempts to read the JSON column,
as it may contain garbage at this point.

The fix is twofold:

1) Field_json::val_str() currently returns NULL if an error happens.
This is correct for val_str() functions in the Item class hierarchy,
but not for val_str() functions in the Field class hierarchy. The
val_str() functions in the Field classes instead return a pointer to
an empty String object on error. Since callers don't expect it to
return NULL, this caused a crash when a caller unconditionally
dereferenced the returned pointer. The patch makes
Field_json::val_str() return a pointer to an empty String on error to
avoid such crashes.

2) Whereas #1 fixes the immediate crash, Field_json::val_str() may
still read garbage when this situation occurs. This could lead to
unreliable behaviour, and both valgrind and ASAN warn about it. The
patch therefore also makes Field_json::store() start by clearing the
field, so that it will hold an empty value rather than garbage after
an error has happened.

Fix #2 is sufficient to fix the reported problems. Fix #1 is included
for consistency, so that Field_json::val_str() behaves the same way as
the other Field::val_str() functions.

The query in the bug report didn't always crash. Since the root cause
was that it had read garbage, it could be lucky and read something
that looked like a valid value. In that case, Field_json::val_str()
didn't return NULL, and the crash was avoided.

The patch also makes these changes:

- It removes the Field_json::store_dom() function, since it is only
  called from one place. It is now inlined instead.

- It corrects information about return values in the comment that
  describes the ensure_utf8mb4() function.
bjornmu pushed a commit that referenced this pull request Oct 21, 2015
Background:

WAIT_FOR_EXECUTED_GTID_SET waits until a specified set of GTIDs is
included in GTID_EXECUTED. SET GTID_PURGED adds GTIDs to
GTID_EXECUTED. RESET MASTER clears GTID_EXECUTED.

There were multiple issues:

 1. Problem:

    The change in GTID_EXECUTED implied by SET GTID_PURGED did
    not cause WAIT_FOR_EXECUTED_GTID_SET to stop waiting.

    Analysis:

    WAIT_FOR_EXECUTED_GTID_SET waits for a signal to be sent.
    But SET GTID_PURGED never sent the signal.

    Fix:

    Make GTID_PURGED send the signal.

    Changes:
    - sql/rpl_gtid_state.cc:Gtid_state::add_lost_gtids
    - sql/rpl_gtid_state.cc: removal of #ifdef HAVE_GTID_NEXT_LIST
    - sql/rpl_gtid.h: removal of #ifdef HAVE_GTID_NEXT_LIST

 2. Problem:

    There was a race condition where WAIT_FOR_EXECUTED_GTID_SET
    could miss the signal from a commit and go into an infinite
    wait even if GTID_EXECUTED contains all the waited-for GTIDs.

    Analysis:

    In the bug, WAIT_FOR_EXECUTED_GTID_SET took a lock while
    taking a copy of the global state. Then it released the lock,
    analyzed the copy of the global state, and decided whether it
    should wait.  But if the GTID to wait for was committed after
    the lock was released, WAIT_FOR_EXECUTED_GTID_SET would miss
    the signal and go to an infinite wait even if GTID_EXECUTED
    contains all the waited-for GTIDs.

    Fix:

    Refactor the code so that it holds the lock all the way from
    before it reads the global state until it goes to the wait.

    Changes:

    - sql/rpl_gtid_state.cc:Gtid_state::wait_for_gtid_set:
      Most of the changes in this function are to fix this bug.

    Note:

    When the bug existed, it was possible to create a test case
    for this by placing a debug sync point in the section where
    it does not hold the lock.  However, after the bug has been
    fixed this section does not exist, so there is no way to test
    it deterministically.  The bug would also cause the test to
    fail rarely, so a way to test this is to run the test case
    1000 times.

 3. Problem:

    The function would take global_sid_lock.wrlock every time it has
    to wait, and while holding it takes a copy of the entire
    gtid_executed (which implies allocating memory).  This is not very
    optimal: it may process the entire set each time it waits, and it
    may wait once for each member of the set, so in the worst case it
    is O(N^2) where N is the size of the set.

    Fix:

    This is fixed by the same refactoring that fixes problem #2.  In
    particular, it does not re-process the entire Gtid_set for each
    committed transaction. It only removes all intervals of
    gtid_executed for the current sidno from the remainder of the
    wait-for-set.

    Changes:
    - sql/rpl_gtid_set.cc: Add function remove_intervals_for_sidno.
    - sql/rpl_gtid_state.cc: Use remove_intervals_for_sidno and remove
      only intervals for the current sidno. Remove intervals
      incrementally in the innermost while loop, rather than recompute
      the entire set each iteration.

 4. Problem:

    If the client that executes WAIT_FOR_EXECUTED_GTID_SET owns a
    GTID that is included in the set, then there is no chance for
    another thread to commit it, so it will wait forever.  In
    effect, it deadlocks with itself.

    Fix:

    Detect the situation and generate an error.

    Changes:
    - sql/share/errmsg-utf8.txt: new error code
      ER_CANT_WAIT_FOR_EXECUTED_GTID_SET_WHILE_OWNING_A_GTID
    - sql/item_func.cc: check the condition and generate the new error

 5. Various simplfications.

    - sql/item_func.cc:Item_wait_for_executed_gtid_set::val_int:
      - Pointless to set null_value when generating an error.
      - add DBUG_ENTER
      - Improve the prototype for Gtid_state::wait_for_gtid_set so
        that it takes a Gtid_set instead of a string, and also so that
        it requires global_sid_lock.
    - sql/rpl_gtid.h:Mutex_cond_array
      - combine wait functions into one and make it return bool
      - improve some comments
    - sql/rpl_gtid_set.cc:Gtid_set::remove_gno_intervals:
      - Optimize so that it returns early if this set becomes empty

@mysql-test/extra/rpl_tests/rpl_wait_for_executed_gtid_set.inc
- Move all wait_for_executed_gtid_set tests into
  mysql-test/suite/rpl/t/rpl_wait_for_executed_gtid_set.test

@mysql-test/include/kill_wait_for_executed_gtid_set.inc
@mysql-test/include/wait_for_wait_for_executed_gtid_set.inc
- New auxiliary scripts.

@mysql-test/include/rpl_init.inc
- Document undocumented side effect.

@mysql-test/suite/rpl/r/rpl_wait_for_executed_gtid_set.result
- Update result file.

@mysql-test/suite/rpl/t/rpl_wait_for_executed_gtid_set.test
- Rewrote the test to improve coverage and cover all parts of this bug.

@sql/item_func.cc
- Add DBUG_ENTER
- No point in setting null_value when generating an error.
- Do the decoding from text to Gtid_set here rather than in Gtid_state.
- Check for the new error
  ER_CANT_WAIT_FOR_EXECUTED_GTID_SET_WHILE_OWNING_A_GTID

@sql/rpl_gtid.h
- Simplify the Mutex_cond_array::wait functions in the following ways:
  - Make them one function since they share most code. This also allows
    calling the three-argument function with NULL as the last
    parameter, which simplifies the caller.
  - Make it return bool rather than 0/ETIME/ETIMEOUT, to make it more
    easy to use.
- Make is_thd_killed private.
- Add prototype for new Gtid_set::remove_intervals_for_sidno.
- Add prototype for Gtid_state::wait_for_sidno.
- Un-ifdef-out lock_sidnos/unlock_sidnos/broadcast_sidnos since we now
  need them.
- Make wait_for_gtid_set return bool.

@sql/rpl_gtid_mutex_cond_array.cc
- Remove the now unused check_thd_killed.

@sql/rpl_gtid_set.cc
- Optimize Gtid_set::remove_gno_intervals, so that it returns early
  if the Interval list becomes empty.
- Add Gtid_set::remove_intervals_for_sidno. This is just a wrapper
  around the already existing private member function
  Gtid_set::remove_gno_intervals.

@sql/rpl_gtid_state.cc
- Rewrite wait_for_gtid_set to fix problems 2 and 3. See code
  comment for details.
- Factor out wait_for_sidno from wait_for_gtid.
- Enable broadcast_sidnos/lock_sidnos/unlock_sidnos, which were ifdef'ed out.
- Call broadcast_sidnos after updating the state, to fix issue #1.

@sql/share/errmsg-utf8.txt
- Add error message used to fix issue #4.
bjornmu pushed a commit that referenced this pull request Oct 21, 2015
After-push test update #2 to make it query cache-friendly.
bjornmu pushed a commit that referenced this pull request Oct 21, 2015
An assert failure is seen in some queries which have a semijoin and
use the materialization strategy.

The assertion fails if either the length of the key is zero or the
number of key parts is zero. This could indicate two different
problems.

1) If the length is zero, there may not be a problem, as it can
legitimately be zero if, for example, the key is a zero-length string.

2) If the number of key parts is zero, there is a bug, as a key must
have at least one part.

The patch fixes issue #1 by removing the length check in the
assertion.

Issue #2 happens if JOIN::update_equalities_for_sjm() doesn't
recognize the expression selected from a subquery, and fails to
replace it with a reference to a column in a temporary table that
holds the materialized result. This causes it to not recognize it as a
part of the key later, and keyparts could end up as zero. The patch
fixes it by calling real_item() on the expression in order to see
through Item_refs that may wrap the expression if the subquery reads
from a view.
@ottok
Copy link
Author

ottok commented Jan 23, 2016

For the record: fixed in commit 60aeb14

bjornmu pushed a commit that referenced this pull request Feb 5, 2016
Problem:

The binary log group commit sync is failing when committing a group of
transactions into a non-transactional storage engine while other thread
is rotating the binary log.

Analysis:

The binary log group commit procedure (ordered_commit) acquires LOCK_log
during the #1 stage (flush). As it holds the LOCK_log, a binary log
rotation will have to wait until this flush stage to finish before
actually rotating the binary log.

For the #2 stage (sync), the binary log group commit only holds the
LOCK_log if sync_binlog=1. In this case, the rotation has to wait also
for the sync stage to finish.

When sync_binlog>1, the sync stage releases the LOCK_log (to let other
groups to enter the flush stage), holding only the LOCK_sync. In this
case, the rotation can acquire the LOCK_log in parallel with the sync
stage.

For commits into transactional storage engine, the binary log rotation
checks a counter of "flushed but not yet committed" transactions,
waiting until this counter to be zeroed before closing the current
binary log file.  As the commit of the transactions happen in the #3
stage of the binary log group commit, the sync of the binary log in
stage #2 always succeed.

For commits into non-transactional storage engine, the binary log
rotation is checking the "flushed but not yet committed" transactions
counter, but it is zero because it only counts transactions that
contains XIDs. So, the rotation is allowed to take place in parallel
with the #2 stage of the binary log group commit. When the sync is
called at the same time that the rotation has closed the old binary log
file but didn't open the new file yet, the sync is failing with the
following error: 'Can't sync file 'UNOPENED' to disk (Errcode: 9 - Bad
file descriptor)'.

Fix:

For non-transactional only workload, binary log group commit will keep
the LOCK_log when entering #2 stage (sync) if the current group is
supposed to be synced to the binary log file.
bjornmu pushed a commit that referenced this pull request Apr 10, 2017
             TO DISSAPPEAR"

Problem
-------

The test case is failing to make the slave server to "disappear".

Analysis
--------

The "crash_in_a_worker" debug sync point is relying on the fact that the
workload will be parallelized and reach MTS worker #2, but on slow
systems the parallelization will not happen and the server will fail to
"disappear".

Fix
---

Ensure that the workload will be distributed by at all the workers
even on slow systems.
bjornmu pushed a commit that referenced this pull request Apr 10, 2017
Author: Andrei Elkin <[email protected]>
Date:   Fri Nov 25 15:17:17 2016 +0200

    WL#9175 Correct recovery of DDL statements/transactions by binary log

    The patch consists of two parts implementing the WL agenda which is
    is to provide crash-safety for DDL.
    That is a server (a general one, master or slave) must be able to recover
    from crash to commit or rollback every DDL command that was in progress
    on the eve of crash.

    The Commit decision is done to commands that had reached
    Engine-prepared status and got successfully logged into binary log.
    Otherwise they are rolled back.

    In order to achieve the goal some refinements are done to the binlogging
    mechanism, minor addition is done to the server recovery module and some changes
    applied to the slave side.

    The binary log part includes Query-log-event which is made to contain xid
    that is a key item at server recovery. The recovery now is concern with it along
    with its standard location in Xid_log_event.

    The first part deals with the ACL DDL sub-class and
    TRIGGER related queries are fully 2pc-capable. It constructs
    the WL's framework which is proved on these subclasses.
    It also specifies how to cover the rest of DDLs by the WL's framework.
    For those not 2pc-ready DDL cases, sometimes "stub" tests are prepared
    to be refined by responsible worklogs.

    Take a few notes to the low-level details of implementation.

    Note #1.

    Tagging by xid number is done to the exact 2pc-capable DDL subclass.
    For DDL:s that will be ready for xiding in future, there is a tech specification
    how to do so.

    Note #2.

    By virtue of existing mechanisms, the slave applier augments the DDL
    transaction incorporating the slave info table update and the
    Gtid-executed table (either one optionally) at time of the DDL is
    ready for the final commit.
    When for filtering reason the DDL skips committing at its regular
    time, the augmented transaction would still be not empty consisting of
    only the added statements, and it would have to be committed by
    top-level slave specific functions through Log_event::do_update_pos().

    To aid this process Query_log_event::has_committed is introduced.

    Note #3 (QA, please read this.)

    Replication System_table interface that is employed by handler of TABLE type slave info
    had to be refined in few places.

    Note #4 (runtime code).

    While trying to lessen the footprint to the runtime server code few
    concessions had to be conceded. These include changes to
    ha_commit_trans()
    to invoke new pre_commit() and post_commit(), and post_rollback() hooks
    due to the slave extra statement.

    -------------------------------------------------------------------

    The 2nd part patch extends the basic framework,
    xidifies the rest of DDL commands that are
    (at least) committable at recovery. At the moment those include
    all Data Definition Statements except ones related to
    VIEWs, STORED Functions and Procedures.

    DDL Query is recoverable for these subclasses when it has been
    recorded into the binary log and was discovered there at the server
    restart, quite compatible with the DML algorithm.
    However a clean automatic rollback can't be provided for some
    of the commands and the user would have to complete recovery
    manually.
pobrzut pushed a commit that referenced this pull request May 8, 2017
… WITH DEVELOPER STUDIO

When we do a release type build of the server (with both optimized and
debug enabled server/plugins) with Developer Studio, some MTR tests
when run with --debug-server will fail in one of two ways:

1. Tests which try to load a plugin into the mysql client fail with
   missing symbols. This is caused by the plugin having references to
   functions which do not exist in the non-debug client.

2. Some tests on sparc fail with Thread stack overrun.

Fix for issue #1: mtr will have appended /debug to the plugin dir part
when running with --debug-server and if there actually is such a
directory. The fix is to remove any trailing /debug from the
env. variable within the test. This will affect the client only, not
the server. Developer builds will not have put the plugins in a
subdirectory /debug so it makes no different to those.

Fix for issue #2: apparently this thread stack overrun is not feasible
to avoid, so just skip the test if running with debug server on sparc;
there is already an include file to do that.

Also added not_sparc_debug.inc to the "white list" so the tests are
skipped even when running mtr --no-skip.

(cherry picked from commit 9c79e477261ab252e38def436bca3336ef597603)
pobrzut pushed a commit that referenced this pull request May 8, 2017
 (A) move ndb_basename into utility library
 (B) new portability function ndb_getsockname()
akopytov pushed a commit to akopytov/mysql-server that referenced this pull request Aug 25, 2017
In WL-included builds ASAN run witnessed missed ~Query_log_event invocation.
The destruct-or was not called due to the WL's changes in the error propagation
that specifically affect LC MTS.
The failure is exposed in particular by rpl_trigger as the following
stack:

  #0 0x9ecd98 in __interceptor_malloc (/export/home/pb2/test/sb_2-22611026-1489061390.32/mysql-commercial-8.0.1-dmr-linux-x86_64-asan/bin/mysqld+0x9ecd98)
  mysql#1 0x2b1a245 in my_raw_malloc(unsigned long, int) obj/mysys/../../mysqlcom-pro-8.0.1-dmr/mysys/my_malloc.cc:209:12
  mysql#2 0x2b1a245 in my_malloc obj/mysys/../../mysqlcom-pro-8.0.1-dmr/mysys/my_malloc.cc:72
  mysql#3 0x2940590 in Query_log_event::Query_log_event(char const*, unsigned int, binary_log::Format_description_event const*, binary_log::Log_event_type) obj/sql/../../mysqlcom-pro-8.0.1-dmr/sql/log_event.cc:4343:46
  mysql#4 0x293d235 in Log_event::read_log_event(char const*, unsigned int, char const**, Format_description_log_event const*, bool) obj/sql/../../mysqlcom-pro-8.0.1-dmr/sql/log_event.cc:1686:17
  mysql#5 0x293b96f in Log_event::read_log_event()
  mysql#6 0x2a2a1c9 in next_event(Relay_log_info*)

Previously before the WL
Mts_submode_logical_clock::wait_for_workers_to_finish() had not
returned any error even when Coordinator thread is killed.

The WL patch needed to refine such behavior, but at doing so
it also had to attend log_event.cc::schedule_next_event() to register
an error to follow an existing pattern.
While my_error() does not take place the killed Coordinator continued
scheduling, ineffectively though - no Worker gets engaged (legal case
of deferred scheduling), and without noticing its killed status up to
a point when it resets the event pointer in
apply_event_and_update_pos():

  *ptr_ev= NULL; // announcing the event is passed to w-worker

The reset was intended for an assigned Worker to perform the event
destruction or by Coordinator itself when the event is deferred.
As neither is the current case the event gets unattended for its termination.

In contrast in the pre-WL sources the killed Coordinator does find a Worker.
However such Worker could be already down (errored out and exited), in
which case apply_event_and_update_pos() reasonably returns an error and executes

  delete ev

in exec_relay_log_event() error branch.

**Fixed** with deploying my_error() call in log_event.cc::schedule_next_event()
error branch which fits to the existing pattern.
THD::is_error() has been always checked by Coordinator before any attempt to
reset *ptr_ev= NULL. In the errored case Coordinator does not reset and
destroys the event itself in the exec_relay_log_event() error branch pretty similarly to
how the pre-WL sources do.

Tested against rpl_trigger and rpl suites to pass.

Approved on rb#15667.
akopytov pushed a commit to akopytov/mysql-server that referenced this pull request Aug 25, 2017
Some character sets are designated as MY_CS_STRNXFRM, meaning that sorting
needs to go through my_strnxfrm() (implemented by the charset), and some are
not, meaning that a client can do the strnxfrm itself based on
cs->sort_order. However, most of the logic related to the latter has been
removed already (e.g. filesort always uses my_strnxfrm() since 2003), and now
it's mostly in the way. The three main uses left are:

 1. A microoptimization for constructing sort keys in filesort.
 2. A home-grown implementation of Boyer-Moore for accelerating certain
    LIKE patterns that should probably be handled through FTS.
 3. Some optimizations to MyISAM prefix keys.

Given that our default collation (utf8mb4_0900_ai_ci) now is a strnxfrm-based
collation, the benefits of keeping these around for a narrow range of
single-byte locales (like latin1_swedish_ci, cp850 and a bunch of more
obscure locales) seems dubious. We seemingly can't remove the flag entirely
due to mysql#3 seemingly affecting the on-disk MyISAM structure, but we can remove
the code for mysql#1 and mysql#2.

Change-Id: If974e490d451b7278355e33ab1fca993f446b792
bjornmu pushed a commit that referenced this pull request Sep 21, 2017
Patch #2:

find_child_doms() checks for duplicates each time it adds a result to
the result vector. As explained in the header comment for
Json_wrapper::seek(), the duplicate elimination is needed for paths
which contain multiple ellipses, so in most cases it is unnecessary
work.

This patch makes find_child_doms() only check for duplicates in the
case where the path contains multiple ellipses, and only when
inspecting an ellipsis path leg which is not the first one.

Additionally:

Remove checks for empty vector after a successful call to push_back().
That is, replace checks for is_seek_done(result, only_need_one) with a
simple check for only_need_one when we know the result vector cannot
be empty.

Call is_seek_done() from the loop in Json_dom::seek() instead of at
the top of find_child_doms(), so that we can break out of the loop
earlier if we find a match and only need one.

Microbenchmarks (64-bit, Intel Core i7-4770 3.4 GHz, GCC 6.3):

BM_JsonDomSearchEllipsis              25693 ns/iter [+210.9%]
BM_JsonDomSearchEllipsis_OnlyOne      17881 ns/iter [+324.3%]
BM_JsonDomSearchKey                     128 ns/iter [  +0.8%]
BM_JsonBinarySearchEllipsis          231319 ns/iter [ +38.7%]
BM_JsonBinarySearchEllipsis_OnlyOne  222726 ns/iter [ +41.6%]
BM_JsonBinarySearchKey                   86 ns/iter [   0.0%]

Change-Id: I0ee624830680247ec5aed302c0408db00240d441
bjornmu pushed a commit that referenced this pull request Sep 21, 2017
 PLUGIN INSTALLS

Split the UDF initialization/deinitialization into two:
1. Initialization/deinitialization of the global structures
2. Loading of the UDF definitions from the table and removing them from the global

Then kept the #2 at the place of the current initialization/deinitalization
routines and added #2 initialization very early (before component/plugin
initialization) and #2 deinitialization very late (after the plugin/compononent
deinitialization.

Added a test plugin and a regression test.
bjornmu pushed a commit that referenced this pull request Jan 23, 2018
…TABLE_UPGRADE_GUARD

To repeat: cmake -DWITH_ASAN=1 -DWITH_ASAN_SCOPE=1
./mtr --mem --sanitize main.dd_upgrade_error

A few dd tests fail with:
==26861==ERROR: AddressSanitizer: stack-use-after-scope on address 0x7000063bf5e8 at pc 0x00010d4dbe8b bp 0x7000063bda40 sp 0x7000063bda38
READ of size 8 at 0x7000063bf5e8 thread T2
    #0 0x10d4dbe8a in Prealloced_array<st_plugin_int**, 16ul>::empty() const prealloced_array.h:186
    #1 0x10d406a8b in lex_end(LEX*) sql_lex.cc:560
    #2 0x10dae4b6d in dd::upgrade::Table_upgrade_guard::~Table_upgrade_guard() (mysqld:x86_64+0x100f87b6d)
    #3 0x10dadc557 in dd::upgrade::migrate_table_to_dd(THD*, std::__1::basic_string<char, std::__1::char_traits<char>, Stateless_allocator<char, dd::String_type_alloc, My_free_functor> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, Stateless_allocator<char, dd::String_type_alloc, My_free_functor> > const&, bool) (mysqld:x86_64+0x100f7f557)
    #4 0x10dad7e85 in dd::upgrade::migrate_plugin_table_to_dd(THD*) (mysqld:x86_64+0x100f7ae85)
    #5 0x10daec6a1 in dd::upgrade::do_pre_checks_and_initialize_dd(THD*) upgrade.cc:1216
    #6 0x10cd0a5c0 in bootstrap::handle_bootstrap(void*) bootstrap.cc:336

Change-Id: I265ec6dd97ee8076aaf03763840c0cdf9e20325b
Fix: increase lifetime of 'LEX lex;' which is used by 'table_guard'
bjornmu pushed a commit that referenced this pull request Jan 23, 2018
        as a component

   * The validate password plugin is converted to component.
   * For mysql-8.0 we will have both component and plugin(But
     plugin will be installed/uninstalled with below deprecate
     warning)
     "validate password plugin' is deprecated and will be
      removed in a future release. Please use validate_password
      component instead"
   * In the next major release we remove the plugin .so file.
   * With the component enabled, we see the system/status
     variables preceded with "validate_password." instead of
     "validate_password_"(for example, if we take 'length'
     system variable we see it as "validate_password.length"
     instead of "validate_password_length")
   * The packaging script should be installing the component
     by default for new installs

   The way it currently works with deprecations and removals
   is the following:
   1. We add a deprecation warning to using the old way and
      we switch the server to use the new way by default for
      new installations.
   2. We expect that people will upgrade their existing
      installations too to avoid the warning.
   3. In the next major release we remove the old way and
      hope that people did #2.
bjornmu pushed a commit that referenced this pull request Jan 23, 2018
…/BINDINGS/XCOM/XCOM/TASK.C

Problem
----------------------------

A Segmentation fault was encountered on a daily-trunk pushbuild
run for the test 'group_replication.gr_ssl_mode_verify_identity_error'
when starting group replication with the below symptoms -

group_replication.gr_ssl_mode_verify_identity_error w5 [ fail ]
        Test ended at 2017-09-27 17:37:18

CURRENT_TEST: group_replication.gr_ssl_mode_verify_identity_error
mysqltest: At line 58: query 'START GROUP_REPLICATION' failed with wrong
errno 2013: 'Lost connection to MySQL server during query', instead of
3092

Analysis
-----------------------------

The test that fails sporadically is Step #2 of gr_ssl_mode_verify_identity_error.
That step causes a global SSL comunication failure, in which not even the
local connections can be established, thus causing the join operation to fail.

When that happens, GCS will request the XCom thread termination. Since it does
not have socket access, it will contact it directly calling xcom_fsm(exit),
but this is a well-known non-thread safe operation, hence causing this crash.

Fix
-----------------------------

Create a new callback for GCS called should_exit, which is tested within
task_loop, in order to check if we should exit the XCom thread.
dveeden pushed a commit to dveeden/mysql-server that referenced this pull request Feb 4, 2018
Fix misc UBSAN warnings in unit tests.
To repeat:
export UBSAN_OPTIONS="print_stacktrace=1"

./runtime_output_directory/merge_large_tests-t --gtest_filter='-*DeathTest*' > /dev/null

unittest/gunit/gis_algos-t.cc:78:70:
runtime error: downcast of address 0x000012dc0be8 which does not point to an object of type 'Gis_polygon_ring'

include/sql_string.h:683:35: runtime error: null pointer passed as argument 2, which is declared to never be null
    mysql#1 0x373e7af in histograms::Value_map<String>::add_values(String const&, unsigned long long) sql/histograms/value_map.cc:149
    mysql#2 0x294fcf2 in dd_column_statistics_unittest::add_values(histograms::Value_map<String>&) unittest/gunit/dd_column_statistics-t.cc:62

runtime_output_directory/merge_keyring_file_tests-t --gtest_filter='-*DeathTest*' > /dev/null

plugin/keyring/common/keyring_key.cc:82:57: runtime error: null pointer passed as argument 2, which is declared to never be null

Change-Id: I2651362e3373244b72e6893f0e22e67402b49a52
(cherry picked from commit 1fe3f72561994da1d912a257689e1b18106f8828)
bjornmu pushed a commit that referenced this pull request Oct 22, 2018
Post push fix

sql/opt_range.cc:14196:22: runtime error: -256 is outside the range of representable values of type 'unsigned int'
    #0 0x4248a9d in cost_skip_scan(TABLE*, unsigned int, unsigned int, unsigned long long, Cost_estimate*, unsigned long long*, Item*, Opt_trace_object*) sql/opt_range.cc:14196:22
    #1 0x41c524c in get_best_skip_scan(PARAM*, SEL_TREE*, bool) sql/opt_range.cc:14086:5
    #2 0x41b7b65 in test_quick_select(THD*, Bitmap<64u>, unsigned long long, unsigned long long, bool, enum_order, QEP_shared_owner const*, Item*, Bitmap<64u>*, QUICK_SELECT_I**) sql/opt_range.cc:3352:23
    #3 0x458fc08 in get_quick_record_count(THD*, JOIN_TAB*, unsigned long long) sql/sql_optimizer.cc:5542:17
    #4 0x458a0cd in JOIN::estimate_rowcount() sql/sql_optimizer.cc:5290:25

The fix is to handle REC_PER_KEY_UNKNOWN explicitly, to avoid using
-1.0 in computations later.

Change-Id: Ie8a81bdf7323e4f66abcad0a9aca776de8acd945
pull bot referenced this pull request in zhuizhu-95/mysql-server Apr 28, 2023
Fix static analysis warnings for variables that are assigned but never
used.
storage/ndb/test/src/UtilTransactions.cpp:286:16: warning: Although the
value stored to 'eof' is used in the enclosing expression, the value is
never actually read from 'eof' [clang-analyzer-deadcode.DeadStores]

Change-Id: I6d7b1fbae691a8d9750f57a8a11c2b12ff65041d
pull bot referenced this pull request in zhuizhu-95/mysql-server Apr 28, 2023
  # This is the 1st commit message:

  WL#15280: HEATWAVE SUPPORT FOR MDS HA

  Problem Statement
  -----------------
  Currently customers cannot enable heatwave analytics service to their
  HA DBSystem or enable HA if they are using Heatwave enabled DBSystem.
  In this change, we attempt to remove this limitation and provide
  failover support of heatwave in an HA enabled DBSystem.

  High Level Overview
  -------------------
  To support heatwave with HA, we extended the existing feature of auto-
  reloading of tables to heatwave on MySQL server restart (WL-14396). To
  provide seamless failover functionality to tables loaded to heatwave,
  each node in the HA cluster (group replication) must have the latest
  view of tables which are currently loaded to heatwave cluster attached
  to the primary, i.e., the secondary_load flag should be in-sync always.

  To achieve this, we made following changes -
    1. replicate secondary load/unload DDL statements to all the active
       secondary nodes by writing the DDL into the binlog, and
    2. Control how secondary load/unload is executed when heatwave cluster
       is not attached to node executing the command

  Implementation Details
  ----------------------
  Current implementation depends on two key assumptions -
   1. All MDS DBSystems will have RAPID plugin installed.
   2. No non-MDS system will have the RAPID plugin installed.

  Based on these assumptions, we made certain changes w.r.t. how server
  handles execution of secondary load/unload statements.
   1. If secondary load/unload command is executed from a mysql client
      session on a system without RAPID plugin installed (i.e., non-MDS),
      instead of an error, a warning message will be shown to the user,
      and the DDL is allowed to commit.
   2. If secondary load/unload command is executed from a replication
      connection on an MDS system without heatwave cluster attached,
      instead of throwing an error, the DDL is allowed to commit.
   3. If no error is thrown from secondary engine, then the DDL will
      update the secondary_load metadata and write a binlog entry.

  Writing to binlog implies that all the consumer of binlog now need to
  handle this DDL gracefully. This has an adverse effect on Point-in-time
  Recovery. If the PITR backup is taken from a DBSystem with heatwave, it
  may contain traces of secondary load/unload statements in its binlog.
  If such a backup is used to restore a new DBSystem, it will cause failure
  while trying to execute statements from its binlog because
   a) DBSystem will not heatwave cluster attached at this time, and
   b) Statements from binlog are executed from standard mysql client
      connection, thus making them indistinguishable from user executed
      command.
  Customers will be prevented (by control plane) from using PITR functionality
  on a heatwave enabled DBSystem until there is a solution for this.

  Testing
  -------
  This commit changes the behavior of secondary load/unload statements, so it
   - adjusts existing tests' expectations, and
   - adds a new test validating new DDL behavior under different scenarios

  Change-Id: Ief7e9b3d4878748b832c366da02892917dc47d83

  # This is the commit message #2:

  WL#15280: HEATWAVE SUPPORT FOR MDS HA (PITR SUPPORT)

  Problem
  -------
  A PITR backup taken from a heatwave enabled system could have traces
  of secondary load or unload statements in binlog. When such a backup
  is used to restore another system, it can cause failure because of
  following two reasons:

  1. Currently, even if the target system is heatwave enabled, heatwave
  cluster is attached only after PITR restore phase completes.
  2. When entries from binlogs are applied, a standard mysql client
  connection is used. This makes it indistinguishable from other user
  session.

  Since secondary load (or unload) statements are meant to throw error
  when they are executed by user in the absence of a healthy heatwave
  cluster, PITR restore workflow will fail if binlogs from the backup
  have any secondary load (or unload) statements in them.

  Solution
  --------
  To avoid PITR failure, we are introducing a new system variable
  rapid_enable_delayed_secondary_ops. It controls how load or unload
  commands are to be processed by rapid plugin.

    - When turned ON, the plugin silently skips the secondary engine
      operation (load/unload) and returns success to the caller. This
      allows secondary load (or unload) statements to be executed by the
      server in the absence of any heatwave cluster.
    - When turned OFF, it follows the existing behavior.
    - The default value is OFF.
    - The value can only be changed when rapid_bootstrap is IDLE or OFF.
    - This variable cannot be persisted.

  In PITR workflow, Control Plane would set the variable at the start of
  PITR restore and then reset it at the end of workflow. This allows the
  workflow to complete without failure even when heatwave cluster is not
  attached. Since metadata is always updated when secondary load/unload
  DDLs are executed, when heatwave cluster is attached at a later point
  in time, the respective tables get reloaded to heatwave automatically.

  Change-Id: I42e984910da23a0e416edb09d3949989159ef707

  # This is the commit message mysql#3:

  WL#15280: HEATWAVE SUPPORT FOR MDS HA (TEST CHANGES)

  This commit adds new functional tests for the MDS HA + HW integration.

  Change-Id: Ic818331a4ca04b16998155efd77ac95da08deaa1

  # This is the commit message mysql#4:

  WL#15280: HEATWAVE SUPPORT FOR MDS HA
  BUG#34776485: RESTRICT DEFAULT VALUE FOR rapid_enable_delayed_secondary_ops

  This commit does two things:
  1. Add a basic test for newly introduced system variable
  rapid_enable_delayed_secondary_ops, which controls the behavior of
  alter table secondary load/unload ddl statements when rapid cluster
  is not available.

  2. It also restricts the DEFAULT value setting for the system variable
  So, following is not allowed:
  SET GLOBAL rapid_enable_delayed_secondary_ops = default
  This variable is to be used in restricted scenarios and control plane
  only sets it to ON/OFF before and after PITR apply. Allowing set to
  default has no practical use.

  Change-Id: I85c84dfaa0f868dbfc7b1a88792a89ffd2e81da2

  # This is the commit message mysql#5:

  Bug#34726490: ADD DIAGNOSTICS FOR SECONDARY LOAD / UNLOAD DDL

  Problem:
  --------
  If secondary load or unload DDL gets rolled back due to some error after
  it had loaded / unloaded the table in heatwave cluster, there is no undo
  of the secondary engine action. Only secondary_load flag update is
  reverted and binlog is not written. From User's perspective, the table
  is loaded and can be seen on performance_schema. There are also no
  error messages printed to notify that the ddl didn't commit. This
  creates a problem to debug any issue in this area.

  Solution:
  ---------
  The partial undo of secondary load/unload ddl will be handled in
  bug#34592922. In this commit, we add diagnostics to reveal if the ddl
  failed to commit, and from what stage.

  Change-Id: I46c04dd5dbc07fc17beb8aa2a8d0b15ddfa171af

  # This is the commit message mysql#6:

  WL#15280: HEATWAVE SUPPORT FOR MDS HA (TEST FIX)

  Since ALTER TABLE SECONDARY LOAD / UNLOAD DDL statements now write
  to binlog, from Heatwave's perspective, SCN is bumped up.

  In this commit, we are adjusting expected SCN values in certain
  tests which does secondary load/unload and expects SCN to match.

  Change-Id: I9635b3cd588d01148d763d703c72cf50a0c0bb98

  # This is the commit message mysql#7:

  Adding MTR tests for ML in rapid group_replication suite

  Added MTR tests with Heatwave ML queries with in
  an HA setup.

  Change-Id: I386a3530b5bbe6aea551610b6e739ab1cf366439

  # This is the commit message mysql#8:

  WL#15280: HEATWAVE SUPPORT FOR MDS HA (MTR TEST ADJUSTMENT)

  In this commit we have adjusted the existing test to work with the
  new MTR test infrastructure which extends the functionalities to
  HA landscape. With this change, a lot of mannual settings have now
  become redundant and thus removed in this commit.

  Change-Id: Ie1f4fcfdf047bfe8638feaa9f54313d509cbad7e

  # This is the commit message mysql#9:

  WL#15280: HEATWAVE SUPPORT FOR MDS HA (CLANG-TIDY FIX)

  Fix clang-tidy warnings found in previous change#16530, patch#20

  Change-Id: I15d25df135694c2f6a3a9146feebe2b981637662

Change-Id: I3f3223a85bb52343a4619b0c2387856b09438265
pull bot referenced this pull request in zhuizhu-95/mysql-server Apr 28, 2023
…::register_variable

Several problems stacked up together:
1. The component initialization, when failing should clean up after
itself.

Fixed the validate_password component's init method to properly clean up
in case of failures.

2. The validate_password component had an REQUIRES_SERIVCE(registry).

While this is not wrong per se, it collided with the implicit
REQUIRES_SERVICE(registry) done by the BEGIN_COMPONENT_REQUIRES() macro
in that it was using the same placeholder global variable.
So now the same service handle was released twice on error or component
unload.
Fixed by removing the second REQUIRES_SERVICE(registry).

3. The dynamic loader is releasing the newly acquired service references
for the required services on initialization error. However after doing
that it was actually setting the service handle placeholder to NULL.
This is not wrong, but combined with problem #2 was causing a reference
to the registry service to be acquired twice, stored into the same
placeholder and then released just once, since after the first release
the placeholder was set to null and thus the second release is a no-op.

Fixed by not resetting the handle placeholder after releasing the
service reference.

4. The system variable registration service wouldn't release the
intermediate memory slots it was allocating on error.

Fixed by using std::unique_ptr to handle the proper releasing.

Change-Id: Ib2c7ae80736c591838af8c182fda1980be1e1f0e
dveeden pushed a commit to dveeden/mysql-server that referenced this pull request May 5, 2023
The SSL socket table is a big table of pointers to SSL objects,
indexed by socket number.

When an NdbSocket is initialized from an ndb_socket_t, it
will fetch any saved SSL * associated with the socket from the
SSL socket table.

If SSL_TABLE_ABORT_ON_HIT in ssl_socket_table.h is set to 1,
debug builds will abort when an SSL pointer is found but not
expected. This is a intended as a tool to help find and fix
code that might lose the association between a socket and its
SSL.

The table is protected by a read-write lock. As the code base
evolves toward use of NdbSocket with correct life cycles,
the contention on the lock should become less and less. Once
SecureSockets are correctly used everywhere, the table can be
disabled.

Change-Id: Ibb9b858146b8d983ca99f05b61ba0abee11b3ee6
venkatesh-prasad-v added a commit to venkatesh-prasad-v/mysql-server that referenced this pull request Aug 3, 2023
…etwork

https://bugs.mysql.com/bug.php?id=109668

Description
-----------
GR suffered from problems caused by the security probes and network scanner
processes connecting to the group replication communication port. This usually
is not a problem, but poses a serious threat when another member tries to join
the cluster by initialting a connection to the member which is affected by
external processes using the port dedicated for group communication for longer
durations.

On such activites by external processes, the SSL enabled server stalled forever
on the SSL_accept() call waiting for handshake data. Below is the stacktrace:

    Thread 55 (Thread 0x7f7bb77ff700 (LWP 2198598)):
    #0 in read ()
    mysql#1 in sock_read ()
    mysql#2 in BIO_read ()
    mysql#3 in ssl23_read_bytes ()
    mysql#4 in ssl23_get_client_hello ()
    mysql#5 in ssl23_accept ()
    mysql#6 in xcom_tcp_server_startup(Xcom_network_provider*) ()

When the server stalled in the above path forever, it prohibited other members
to join the cluster resulting in the following messages on the joiner server's
logs.

    [ERROR] [MY-011640] [Repl] Plugin group_replication reported: 'Timeout on wait for view after joining group'
    [ERROR] [MY-011735] [Repl] Plugin group_replication reported: '[GCS] The member is already leaving or joining a group.'

Solution
--------
This patch adds two new variables

1. group_replication_xcom_ssl_socket_timeout

   It is a file-descriptor level timeout in seconds for both accept() and
   SSL_accept() calls when group replication is listening on the xcom port.
   When set to a valid value, say for example 5 seconds, both accept() and
   SSL_accept() return after 5 seconds. The default value has been set to 0
   (waits infinitely) for backward compatibility. This variable is effective
   only when GR is configred with SSL.

2. group_replication_xcom_ssl_accept_retries

   It defines the number of retries to be performed before closing the socket.
   For each retry the server thread calls SSL_accept()  with timeout defined by
   the group_replication_xcom_ssl_socket_timeout for the SSL handshake process
   once the connection has been accepted by the first accept() call. The
   default value has been set to 10. This variable is effective only when GR is
   configred with SSL.

Note:
- Both of the above variables are dynamically configurable, but will become
  effective only on START GROUP_REPLICATION.
- This patch is only for the Linux systems.
nawazn pushed a commit that referenced this pull request Oct 25, 2023
Post push fix.

NdbSocket::copy method duplicated the mutex pointer, leaving two objects
referring to one mutex. Typically the source will destroy its mutex,
making it unusable for target object.

Remove copy method.

Change-Id: I2cc36128c343c7bab08d96651b12946ecd87210c
bjornmu pushed a commit that referenced this pull request Oct 25, 2023
Part of WL#15135 Certificate Architecture

This patch introduces class TlsKeyManager, containing all TLS
authentication and key management logic. A single instance of
TlsKeyManager in each node owns the local NodeCertificate, an
SSL_CTX, and a table holding the serial numbers and expiration
dates of all peer certificates.

A large set of TLS-related error codes is introduced in the file
TlsKeyErrors.h.

The unit test testTlsKeyManager-t tests TLS authentication over
client/server connections on localhost.

Change-Id: I2ee42efc268219639691f73a1d7638a336844d88
bjornmu pushed a commit that referenced this pull request Oct 25, 2023
Implement ndb$certificates base table and certificates view.
Update results for tests ndbinfo and ndbinfo plans.

Change-Id: Iab1b89f5eb82ac1b3e0c049dd55eb7d07394070a
bjornmu pushed a commit that referenced this pull request Oct 25, 2023
Move client_authenticate() out of SocketClient::connect() (which
returns void) into a separate SocketClient::authenticate() method
which can return a value.

In SocketAuthenticator, change the signature of the authentication
routines to return an int (which can represent a result code) rather
than a bool. Results less than AuthOk represent failure, and results
greater than or equal to AuthOk represent success.

Remove the username and password variables from SocketAuthSimple;
make them constant strings in the implementation.

There are no functional changes.

Change-Id: I4c25e99f1b9b692db39213dfa63352da8993a8fb
bjornmu pushed a commit that referenced this pull request Oct 25, 2023
This changes TransporterRegistry::connect_ndb_mgmd() to return
NdbSocket rather than ndb_socket_t.

It extends the StartTls test in testMgmd to test upgrading the
TLS MGM protocol socket to a transporter.

Change-Id: Ic3b9ccf39ec78ed25705a4bbbdc5ac2953a35611
bjornmu pushed a commit that referenced this pull request Oct 25, 2023
Post push fix.

NdbSocket::copy method duplicated the mutex pointer, leaving two objects
referring to one mutex. Typically the source will destroy its mutex,
making it unusable for target object.

Fix by use the transfer method instead.

Change-Id: I199c04b870049498463903f6358f79a38649f543
bjornmu pushed a commit that referenced this pull request Jan 16, 2024
If the argument to a window function contains a subquery, the access path
of that subquery would be printed twice when doing 'EXPLAIN FORMAT=TREE'.
When using the Hypergraph optimizer, the subquery path was not printed at
all, whether using FORMAT=TREE or FORMAT=JSON.

This commit fixes this by ensuring that we ignore duplicate paths,
and (for Hypergraph) by traversing the structures needed to find the
relevant Item_subselect objects.

Change-Id: I2abedcf690294f98ce169b74e53f042f46c47a45
bjornmu pushed a commit that referenced this pull request Jan 16, 2024
Post-push fix: Cherry-picking the fix onto mysql-trunk introduced an
unintended duplication of a code block, causing a shadowing-warning
when building with g++. This commit corrects that.

Change-Id: I1b279818ca0d30e32fc8dabb76c647120b531e8f
bjornmu pushed a commit that referenced this pull request Jan 16, 2024
Problem
================================

Group Replication ASAN run failing without any symptom of a
leak, but with shutdown issues:

worker[6] Shutdown report from
/dev/shm/mtr-3771884/var-gr-debug/6/log/mysqld.1.err after tests:
 group_replication.gr_flush_logs
group_replication.gr_delayed_initialization_thread_handler_error
group_replication.gr_sbr_verifications
group_replication.gr_server_uuid_matches_group_name_bootstrap
group_replication.gr_stop_async_on_stop_gr
group_replication.gr_certifier_message_same_member
group_replication.gr_ssl_mode_verify_identity_error_xcom

Analysis and Fix
================================

It ended up being a leak on gr_ssl_mode_verify_identity_error_xcom test:
Direct leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x7f1709fbe1c7 in operator new(unsigned long)
      ../../../../src/libsanitizer/asan/asan_new_delete.cpp:99
    #1 0x7f16ea0df799 in xcom_tcp_server_startup(Xcom_network_provider*)
      (/export/home/tmp/BUG35594709/mysql-trunk/BIN-ASAN/plugin_output_directory
        /group_replication.so+0x65d799)
    #2 0x7f170751e2b2  (/lib/x86_64-linux-gnu/libstdc++.so.6+0xdc2b2)

This happens because we delegated incoming connections
cleanup to the external consumer in incoming_connection_task.
Since it calls incoming_connection() from
Network_provider_manager, in case of a concurrent stop,
a connection could be left orphan in the shared atomic
due to the lack of an Active Provider, thus creating a
memory leak.

The solution is to make this cleanup on
Network_provider_manager, on both stop_provider() and in
stop_all_providers() methods, thus ensuring that no
incoming connection leaks.

Change-Id: I2367c37608ad075dee63785e9f908af5e81374ca
bjornmu pushed a commit that referenced this pull request Jan 16, 2024
Post push fix.

In test program testTlsKeyManager-t a struct sockaddr pointer was passed
to inet_ntop instead of struct in_addr for AF_INET and struct in6_addr
for AF_INET6.

That caused wrong addresses to be printed on error:

  not ok 26 - Client cert for test hostname is OK
   >>> Test of address 2.0.0.0 for msdn.microsoft.com returned error authorization failure: bad hostname
  not ok 27 - Client cert for test hostname is OK
   >>> Test of address a00::2620:1ec:46:0 for msdn.microsoft.com returned error authorization failure: bad hostname
  not ok 28 - Client cert for test hostname is OK
   >>> Test of address a00::2620:1ec:bdf:0 for msdn.microsoft.com returned error authorization failure: bad hostname

Should be 13.107.x.53 or 2620:1ec:x::53.

Changed to use ndb_sockaddr and Ndb_inet_ntop instead.

Change-Id: Iae4bebca26462f9b65c3232e9768c574e767b380
bjornmu pushed a commit that referenced this pull request Jan 16, 2024
Move client_authenticate() out of SocketClient::connect() (which
returns void) into a separate SocketClient::authenticate() method
which can return a value.

In SocketAuthenticator, change the signature of the authentication
routines to return an int (which can represent a result code) rather
than a bool. Results less than AuthOk represent failure, and results
greater than or equal to AuthOk represent success.

Remove the username and password variables from SocketAuthSimple;
make them constant strings in the implementation.

There are no functional changes.

Change-Id: I4c25e99f1b9b692db39213dfa63352da8993a8fb
bjornmu pushed a commit that referenced this pull request Jan 16, 2024
This changes TransporterRegistry::connect_ndb_mgmd() to return
NdbSocket rather than ndb_socket_t.

Back-ported from mysql-trunk.

Change-Id: Ic3b9ccf39ec78ed25705a4bbbdc5ac2953a35611
bjornmu pushed a commit that referenced this pull request Jan 16, 2024
Post-push fix.

ASan reported memory leaks from some EXPLAIN tests, such as
main.explain_tree.

The reason was that the Json_dom objects that were discarded to avoid
describing a subquery twice, were not properly destroyed.

The EXPLAIN code uses unique_ptr to make sure the Json_dom objects are
destroyed, but there are windows in which the objects only exist as
unmanaged raw pointers. This patch closes the window which caused this
memory leak by changing ExplainChild::obj from a raw pointer to a
unique_ptr, so that it gets destroyed even if it doesn't make it into
the final tree that describes the full plan.

Change-Id: I0f0885da867e8a34335ff11f3ae9da883a878ba4
bjornmu pushed a commit that referenced this pull request Jan 16, 2024
BUG#35949017 Schema dist setup lockup
Bug#35948153 Problem setting up events due to stale NdbApi dictionary cache [#2]
Bug#35948153 Problem setting up events due to stale NdbApi dictionary cache [#1]
Bug#32550019 Missing check for ndb_schema_result leads to schema dist timeout

Change-Id: I4a32197992bf8b6899892f21587580788f828f34
msprajap pushed a commit that referenced this pull request Jan 16, 2024
cache [#2]

This is second patch, solving the problem of ineffiecent cache
invalidation when invalidating a table which is known to be invalid but
unknown if it is in the cache or not.

Problem:
Currently the only way to invalidate a table in the NdbApi dictionary
cache is to open the table and then mark it as invalid. In case the
table does not exists in the cache, it will still have to be opened and
thus fetched fom NDB.

This means that in order to get the latest table definition it has to be
fetched two times, although the table definition does not already exist
in the cache. This is inefficient.

Analysis:
In order to avoid the double roundtrip there need to be a function which
marks the table as invalid only if it exists in the cache.

Fix:
Implement a NdbApi function that invalidates table by name if it exists
in the cache.
Replace the old pattern of opening table in order to invalidate it with
the new function.

The old pattern is still a valid use case for invalidating a table after
having worked with it.

Change-Id: I20f275f1fed76d991330348bea4ae72548366467
venkatesh-prasad-v added a commit to venkatesh-prasad-v/mysql-server that referenced this pull request Jan 26, 2024
…and a local

             DDL executed

https://bugs.mysql.com/bug.php?id=113727

Problem
-------
In high concurrency scenarios, MySQL replica can enter into a deadlock due to a
race condition between the replica applier thread and the client thread
performing a binlog group commit.

Analysis
--------
It needs at least 3 threads for this deadlock to happen

1. One client thread
2. Two replica applier threads

How this deadlock happens?
--------------------------
0. Binlog is enabled on replica, but log_replica_updates is disabled.

1. Initially, both "Commit Order" and "Binlog Flush" queues are empty.

2. Replica applier thread 1 enters the group commit pipeline to register in the
   "Commit Order" queue since `log-replica-updates` is disabled on the replica
   node.

3. Since both "Commit Order" and "Binlog Flush" queues are empty, the applier
   thread 1

   3.1. Becomes leader (In Commit_stage_manager::enroll_for()).

   3.2. Registers in the commit order queue.

   3.3. Acquires the lock MYSQL_BIN_LOG::LOCK_log.

   3.4. Commit Order queue is emptied, but the lock MYSQL_BIN_LOG::LOCK_log is
        not yet released.

   NOTE: SE commit for applier thread is already done by the time it reaches
         here.

4. Replica applier thread 2 enters the group commit pipeline to register in the
   "Commit Order" queue since `log-replica-updates` is disabled on the replica
   node.

5. Since the "Commit Order" queue is empty (emptied by applier thread 1 in 3.4), the
   applier thread 2

   5.1. Becomes leader (In Commit_stage_manager::enroll_for())

   5.2. Registers in the commit order queue.

   5.3. Tries to acquire the lock MYSQL_BIN_LOG::LOCK_log. Since it is held by applier
        thread 1 it will wait until the lock is released.

6. Client thread enters the group commit pipeline to register in the
   "Binlog Flush" queue.

7. Since "Commit Order" queue is not empty (there is applier thread 2 in the
   queue), it enters the conditional wait `m_stage_cond_leader` with an
   intention to become the leader for both the "Binlog Flush" and
   "Commit Order" queues.

8. Applier thread 1 releases the lock MYSQL_BIN_LOG::LOCK_log and proceeds to update
   the GTID by calling gtid_state->update_commit_group() from
   Commit_order_manager::flush_engine_and_signal_threads().

9. Applier thread 2 acquires the lock MYSQL_BIN_LOG::LOCK_log.

   9.1. It checks if there is any thread waiting in the "Binlog Flush" queue
        to become the leader. Here it finds the client thread waiting to be
        the leader.

   9.2. It releases the lock MYSQL_BIN_LOG::LOCK_log and signals on the
        cond_var `m_stage_cond_leader` and enters a conditional wait until the
        thread's `tx_commit_pending` is set to false by the client thread
       (will be done in the
       Commit_stage_manager::process_final_stage_for_ordered_commit_group()
       called by client thread from fetch_and_process_flush_stage_queue()).

10. The client thread wakes up from the cond_var `m_stage_cond_leader`.  The
    thread has now become a leader and it is its responsibility to update GTID
    of applier thread 2.

    10.1. It acquires the lock MYSQL_BIN_LOG::LOCK_log.

    10.2. Returns from `enroll_for()` and proceeds to process the
          "Commit Order" and "Binlog Flush" queues.

    10.3. Fetches the "Commit Order" and "Binlog Flush" queues.

    10.4. Performs the storage engine flush by calling ha_flush_logs() from
          fetch_and_process_flush_stage_queue().

    10.5. Proceeds to update the GTID of threads in "Commit Order" queue by
          calling gtid_state->update_commit_group() from
          Commit_stage_manager::process_final_stage_for_ordered_commit_group().

11. At this point, we will have

    - Client thread performing GTID update on behalf if applier thread 2 (from step 10.5), and
    - Applier thread 1 performing GTID update for itself (from step 8).

    Due to the lack of proper synchronization between the above two threads,
    there exists a time window where both threads can call
    gtid_state->update_commit_group() concurrently.

    In subsequent steps, both threads simultaneously try to modify the contents
    of the array `commit_group_sidnos` which is used to track the lock status of
    sidnos. This concurrent access to `update_commit_group()` can cause a
    lock-leak resulting in one thread acquiring the sidno lock and not
    releasing at all.

-----------------------------------------------------------------------------------------------------------
Client thread                                           Applier Thread 1
-----------------------------------------------------------------------------------------------------------
update_commit_group() => global_sid_lock->rdlock();     update_commit_group() => global_sid_lock->rdlock();

calls update_gtids_impl_lock_sidnos()                   calls update_gtids_impl_lock_sidnos()

set commit_group_sidno[2] = true                        set commit_group_sidno[2] = true

                                                        lock_sidno(2) -> successful

lock_sidno(2) -> waits

                                                        update_gtids_impl_own_gtid() -> Add the thd->owned_gtid in `executed_gtids()`

                                                        if (commit_group_sidnos[2]) {
                                                          unlock_sidno(2);
                                                          commit_group_sidnos[2] = false;
                                                        }

                                                        Applier thread continues..

lock_sidno(2) -> successful

update_gtids_impl_own_gtid() -> Add the thd->owned_gtid in `executed_gtids()`

if (commit_group_sidnos[2]) { <=== this check fails and lock is not released.
  unlock_sidno(2);
  commit_group_sidnos[2] = false;
}

Client thread continues without releasing the lock
-----------------------------------------------------------------------------------------------------------

12. As the above lock-leak can also happen the other way i.e, the applier
    thread fails to unlock, there can be different consequences hereafter.

13. If the client thread continues without releasing the lock, then at a later
    stage, it can enter into a deadlock with the applier thread performing a
    GTID update with stack trace.

    Client_thread
    -------------
    mysql#1  __GI___lll_lock_wait
    mysql#2  ___pthread_mutex_lock
    mysql#3  native_mutex_lock                                       <= waits for commit lock while holding sidno lock
    mysql#4  Commit_stage_manager::enroll_for
    mysql#5  MYSQL_BIN_LOG::change_stage
    mysql#6  MYSQL_BIN_LOG::ordered_commit
    mysql#7  MYSQL_BIN_LOG::commit
    mysql#8  ha_commit_trans
    mysql#9  trans_commit_implicit
    mysql#10 mysql_create_like_table
    mysql#11 Sql_cmd_create_table::execute
    mysql#12 mysql_execute_command
    mysql#13 dispatch_sql_command

    Applier thread
    --------------
    mysql#1  ___pthread_mutex_lock
    mysql#2  native_mutex_lock
    mysql#3  safe_mutex_lock
    mysql#4  Gtid_state::update_gtids_impl_lock_sidnos               <= waits for sidno lock
    mysql#5  Gtid_state::update_commit_group
    mysql#6  Commit_order_manager::flush_engine_and_signal_threads   <= acquires commit lock here
    mysql#7  Commit_order_manager::finish
    mysql#8  Commit_order_manager::wait_and_finish
    mysql#9  ha_commit_low
    mysql#10 trx_coordinator::commit_in_engines
    mysql#11 MYSQL_BIN_LOG::commit
    mysql#12 ha_commit_trans
    mysql#13 trans_commit
    mysql#14 Xid_log_event::do_commit
    mysql#15 Xid_apply_log_event::do_apply_event_worker
    mysql#16 Slave_worker::slave_worker_exec_event
    mysql#17 slave_worker_exec_job_group
    mysql#18 handle_slave_worker

14. If the applier thread continues without releasing the lock, then at a later
    stage, it can perform recursive locking while setting the GTID for the next
    transaction (in set_gtid_next()).

    In debug builds the above case hits the assertion
    `safe_mutex_assert_not_owner()` meaning the lock is already acquired by the
    replica applier thread when it tries to re-acquire the lock.

Solution
--------
In the above problematic example, when seen from each thread
individually, we can conclude that there is no problem in the order of lock
acquisition, thus there is no need to change the lock order.

However, the root cause for this problem is that multiple threads can
concurrently access to the array `Gtid_state::commit_group_sidnos`.

In its initial implementation, it was expected that threads should
hold the `MYSQL_BIN_LOG::LOCK_commit` before modifying its contents. But it
was not considered when upstream implemented WL#7846 (MTS:
slave-preserve-commit-order when log-slave-updates/binlog is disabled).

With this patch, we now ensure that `MYSQL_BIN_LOG::LOCK_commit` is acquired
when the client thread (binlog flush leader) when it tries to perform GTID
update on behalf of threads waiting in "Commit Order" queue, thus providing a
guarantee that `Gtid_state::commit_group_sidnos` array is never accessed
without the protection of `MYSQL_BIN_LOG::LOCK_commit`.
bjornmu pushed a commit that referenced this pull request Apr 30, 2024
…nt on Windows and posix [#2]

The posix version of NdbProcess::start_process assumed the arguments
where quoted using " and \ in a way that resembles POSIX sh quoting, and
unquoted spaces were treated as argument separators splitting the
argument to several.

But the Windows version of NdbProcess::start_process did not treat
options in the same way. And the Windows C runtime (CRT) parse arguments
different from POSIX sh. Note that if program do not use CRT when it may
treat the command line in its own way and the quoting done for CRT will
mess up the command line.

On Windows NdbProcess:start_process should only be used for CRT
compatible programs on Windows with respect to argument quoting on
command line, or one should make sure given arguments will not trigger
unwanted quoting. This may be relevant for ndb_sign_keys and
--CA-tool=<batch-file>.

Instead this patch change the intention of start_process to pass
arguments without modification from caller to the called C programs
argument vector in its main entry function.

In posix path that is easy, just pass the incoming C strings to execvp.

On Windows one need to quote for Windows CRT when composing the command
line. Note that the command part of command line have different quoting
than the following arguments have.

Change-Id: I763530c634d3ea460b24e6e01061bbb5f3321ad4
bjornmu pushed a commit that referenced this pull request Apr 30, 2024
Problem:
Starting ´ndb_mgmd --bind-address´ may potentially cause abnormal
program termination in MgmtSrvr destructor when ndb_mgmd restart itself.

  Core was generated by `ndb_mgmd --defa'.
  Program terminated with signal SIGABRT,   Aborted.
  #0  0x00007f8ce4066b8f in raise () from /lib64/libc.so.6
  #1  0x00007f8ce4039ea5 in abort () from /lib64/libc.so.6
  #2  0x00007f8ce40a7d97 in __libc_message () from /lib64/libc.so.6
  #3  0x00007f8ce40af08c in malloc_printerr () from /lib64/libc.so.6
  #4  0x00007f8ce40b132d in _int_free () from /lib64/libc.so.6
  #5  0x00000000006e9ffe in MgmtSrvr::~MgmtSrvr (this=0x28de4b0) at
mysql/8.0/storage/ndb/src/mgmsrv/MgmtSrvr.cpp:
890
  #6  0x00000000006ea09e in MgmtSrvr::~MgmtSrvr (this=0x2) at mysql/8.0/
storage/ndb/src/mgmsrv/MgmtSrvr.cpp:849
  #7  0x0000000000700d94 in mgmd_run () at
mysql/8.0/storage/ndb/src/mgmsrv/main.cpp:260
  #8  0x0000000000700775 in mgmd_main (argc=<optimized out>,
argv=0x28041d0) at mysql/8.0/storage/ndb/src/
mgmsrv/main.cpp:479

Analysis:
While starting up, the ndb_mgmd will allocate memory for bind_address in
order to potentially rewrite the parameter. When ndb_mgmd restart itself
the memory will be released and dangling pointer causing double free.

Fix:
Drop support for bind_address=[::], it is not documented anywhere, is
not useful and doesn't work.
This means the need to rewrite bind_address is gone and bind_address
argument need neither alloc or free.

Change-Id: I7797109b9d8391394587188d64d4b1f398887e94
bjornmu pushed a commit that referenced this pull request Jul 1, 2024
… for connection xxx'.

The new iterator based explains are not impacted.

The issue here is a race condition. More than one thread is using the
query term iterator at the same time (whoch is neithe threas safe nor
reantrant), and part of its state is in the query terms being visited
which leads to interference/race conditions.

a) the explain thread

uses an iterator here:

   Sql_cmd_explain_other_thread::execute

is inspecting the Query_expression of the running query
calling master_query_expression()->find_blocks_query_term which uses
an iterator over the query terms in the query expression:

   for (auto qt : query_terms<>()) {
       if (qt->query_block() == qb) {
           return qt;
       }
   }

the above search fails to find qb due to the interference of the
thread b), see below, and then tries to access a nullpointer:

    * thread #36, name = ‘connection’, stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  frame #0: 0x000000010bb3cf0d mysqld`Query_block::type(this=0x00007f8f82719088) const at sql_lex.cc:4441:11
  frame #1: 0x000000010b83763e mysqld`(anonymous namespace)::Explain::explain_select_type(this=0x00007000020611b8) at opt_explain.cc:792:50
  frame #2: 0x000000010b83cc4d mysqld`(anonymous namespace)::Explain_join::explain_select_type(this=0x00007000020611b8) at opt_explain.cc:1487:21
  frame #3: 0x000000010b837c34 mysqld`(anonymous namespace)::Explain::prepare_columns(this=0x00007000020611b8) at opt_explain.cc:744:26
  frame #4: 0x000000010b83ea0e mysqld`(anonymous namespace)::Explain_join::explain_qep_tab(this=0x00007000020611b8, tabnum=0) at opt_explain.cc:1415:32
  frame #5: 0x000000010b83ca0a mysqld`(anonymous namespace)::Explain_join::shallow_explain(this=0x00007000020611b8) at opt_explain.cc:1364:9
  frame #6: 0x000000010b83379b mysqld`(anonymous namespace)::Explain::send(this=0x00007000020611b8) at opt_explain.cc:770:14
  frame #7: 0x000000010b834147 mysqld`explain_query_specification(explain_thd=0x00007f8fbb111e00, query_thd=0x00007f8fbb919c00, query_term=0x00007f8f82719088, ctx=CTX_JOIN) at opt_explain.cc:2088:20
  frame #8: 0x000000010bd36b91 mysqld`Query_expression::explain_query_term(this=0x00007f8f7a090360, explain_thd=0x00007f8fbb111e00, query_thd=0x00007f8fbb919c00, qt=0x00007f8f82719088) at sql_union.cc:1519:11
  frame #9: 0x000000010bd36c68 mysqld`Query_expression::explain_query_term(this=0x00007f8f7a090360, explain_thd=0x00007f8fbb111e00, query_thd=0x00007f8fbb919c00, qt=0x00007f8f8271d748) at sql_union.cc:1526:13
  frame #10: 0x000000010bd373f7 mysqld`Query_expression::explain(this=0x00007f8f7a090360, explain_thd=0x00007f8fbb111e00, query_thd=0x00007f8fbb919c00) at sql_union.cc:1591:7
  frame #11: 0x000000010b835820 mysqld`mysql_explain_query_expression(explain_thd=0x00007f8fbb111e00, query_thd=0x00007f8fbb919c00, unit=0x00007f8f7a090360) at opt_explain.cc:2392:17
  frame #12: 0x000000010b835400 mysqld`explain_query(explain_thd=0x00007f8fbb111e00, query_thd=0x00007f8fbb919c00, unit=0x00007f8f7a090360) at opt_explain.cc:2353:13
 * frame #13: 0x000000010b8363e4 mysqld`Sql_cmd_explain_other_thread::execute(this=0x00007f8fba585b68, thd=0x00007f8fbb111e00) at opt_explain.cc:2531:11
  frame #14: 0x000000010bba7d8b mysqld`mysql_execute_command(thd=0x00007f8fbb111e00, first_level=true) at sql_parse.cc:4648:29
  frame #15: 0x000000010bb9e230 mysqld`dispatch_sql_command(thd=0x00007f8fbb111e00, parser_state=0x0000700002065de8) at sql_parse.cc:5303:19
  frame #16: 0x000000010bb9a4cb mysqld`dispatch_command(thd=0x00007f8fbb111e00, com_data=0x0000700002066e38, command=COM_QUERY) at sql_parse.cc:2135:7
  frame #17: 0x000000010bb9c846 mysqld`do_command(thd=0x00007f8fbb111e00) at sql_parse.cc:1464:18
  frame #18: 0x000000010b2f2574 mysqld`handle_connection(arg=0x0000600000e34200) at connection_handler_per_thread.cc:304:13
  frame #19: 0x000000010e072fc4 mysqld`pfs_spawn_thread(arg=0x00007f8fba8160b0) at pfs.cc:3051:3
  frame #20: 0x00007ff806c2b202 libsystem_pthread.dylib`_pthread_start + 99
  frame #21: 0x00007ff806c26bab libsystem_pthread.dylib`thread_start + 15

b) the query thread being explained is itself performing LEX::cleanup
and as part of the iterates over the query terms, but still allows
EXPLAIN of the query plan since

   thd->query_plan.set_query_plan(SQLCOM_END, ...)

hasn't been called yet.

     20:frame: Query_terms<(Visit_order)1, (Visit_leaves)0>::Query_term_iterator::operator++() (in mysqld) (query_term.h:613)
     21:frame: Query_expression::cleanup(bool) (in mysqld) (sql_union.cc:1861)
     22:frame: LEX::cleanup(bool) (in mysqld) (sql_lex.h:4286)
     30:frame: Sql_cmd_dml::execute(THD*) (in mysqld) (sql_select.cc:799)
     31:frame: mysql_execute_command(THD*, bool) (in mysqld) (sql_parse.cc:4648)
     32:frame: dispatch_sql_command(THD*, Parser_state*) (in mysqld) (sql_parse.cc:5303)
     33:frame: dispatch_command(THD*, COM_DATA const*, enum_server_command) (in mysqld) (sql_parse.cc:2135)
     34:frame: do_command(THD*) (in mysqld) (sql_parse.cc:1464)
     57:frame: handle_connection(void*) (in mysqld) (connection_handler_per_thread.cc:304)
     58:frame: pfs_spawn_thread(void*) (in mysqld) (pfs.cc:3053)
     65:frame: _pthread_start (in libsystem_pthread.dylib) + 99
     66:frame: thread_start (in libsystem_pthread.dylib) + 15

Solution:

This patch solves the issue by removing iterator state from
Query_term, making the query_term iterators thread safe. This solution
labels every child query_term with its index in its parent's
m_children vector.  The iterator can therefore easily compute the next
child to visit based on Query_term::m_sibling_idx.

A unit test case is added to check reentrancy.

One can also manually verify that we have no remaining race condition
by running two client connections files (with \. <file>) with a big
number of copies of the repro query in one connection and a big number
of EXPLAIN format=json FOR <connection>, e.g.

    EXPLAIN FORMAT=json FOR CONNECTION 8\G

in the other. The actual connection number would need to verified
in connection one, of course.

Change-Id: Ie7d56610914738ccbbecf399ccc4f465f7d26ea7
bjornmu pushed a commit that referenced this pull request Jul 1, 2024
This is a combination of 5 commits.

This is the 1st commit message:

WL#15746: TLS Enhancements for HeatWave-AutoML & Dask Comm. Upgrade

Problem:
--------
- HeatWave-AutoML communication was unauthenticated, unauthorized,
  and unencrypted.
- Dask communication utilized TCP, not aligning with FedRamp
  guidelines.

Solution:
---------
- Introduced TLS and mTLS in HeatWave-AutoML's plugin and driver for
  authentication, authorization, and encryption.
- Applied TLS to Dask to ensure authentication, encryption, and
  authorization.

Dask Authorization (OCID-based):
--------------------------------
1. For each DBsystem:
    - MySQL node sends OCIDs of authorized nodes to the head driver
      via:
        a. rapid_net_nodes
        b. rapid_net_allowed_ocids (older API, mainly for MTR tests)
    - Scenarios:
        a. All OCIDs provided: Dask authorizes.
        b. Any OCID absent: ML call fails with message.
2. During Dask worker registration to the Dask scheduler, a script is
    dispatched to the Dask worker for execution, retrieving the worker's
    OCID for authorization purposes.
    - If the OCID isn't approved, the connection is denied, terminating
      the worker and causing the ML query to fail.
3. For every Dask worker (both as listener and connector), an OCID-
    based authorization is performed post SSL/TLS connection handshake.
    The process compares the OCID from the peer's certificate against
    the allowed_ocids received from the HeatWave-AutoML MySQL plugin.

HWAML Plugin Changes:
---------------------
- Sourced certificate data and SSL setup from disk, incorporating
  SSL/TLS for HWAML.
- Reused "keystore" variable to specify disk location for
  certificate retrieval.
- Certificates and keys expected in PKCS12 format.
- Introduced "have_ml_encryption" variable (default=0).
    > Acts as a switch to explicitly deactivate HWAML network
      encryption, akin to "disable_net_encryption" affecting
      network encryption for HeatWave. Set to 1 to enable.
- Introduced a customized verifier function for verify_callback to
  be set in SSL_CTX_set_verify and used in the handshake process
  of SSL/TLS. The customized verifier function will perform
  instance id (OCID) based authorization on the plugin side during
  standard SSL/TLS handshake process.
- CRL (Certificate Revocation List) checks are also conducted if CRL
  Distribution Points are present and accessible in the provided
  certificate.

HWAML Driver Changes & OCID-based Authorization:
------------------------------------------------
- Introduced "enable_encryption" (default=0).
    > Set to 1 to enable encryption.
- When receiving a new connection request and encryption is on, the
  driver performs OCID-based self-checking, comparing OCID retrieved
  from its own instance principal with the OCID in the
  provided certificate on disk.
- The driver compares OCID from "mysql_compute_id" and extracted OCID
  from mTLS certificate during connection.
- Introduced "cert_dir" argument for certificate directory
  specification.
- Expected files: cert_chain.pem, certificate.pem, private_key.pem.
    > OCID should be in the userID (UID) or CN field of the
      certificate.pem subject.
- CRL (Certificate Revocation List) checks are also conducted post
  handshake, if CRL Distribution Points are present and accessible in
  the provided certificate, alongside OCID authorization.

Encryption Behavior:
--------------------
- If encryption is deactivated on both plugin and driver side, HWAML
  will work without encryption as it was before this commit.

Enabling Encryption:
--------------------
- By default, "have_ml_encryption" and "enable_encryption" are set to 0
    > Encryption is disabled by default.
- For the HWAML plugin:
    > "have_ml_encryption" set to 1 (default is 0).
    > Specify the .pfx file's path using the "keystore".
- For the HWAML Driver:
    > "enable_encryption" set to 1 (default is 0)
    > Specify "mysql_instance_id" and "cert_dir".

Testing:
--------
- MTR has been modified for the encryption setup.
    > Runs with encryption if "OCI_INSTANCE_ID" is set to a valid
      value.
- On OCI (when "OLRAPID_KEYSTORE" is not set):
    > Certificates and keys are generated; PEMs for driver and PKCS12
      for plugin.
- On AWS (when "OLRAPID_KEYSTORE" is set as the path to PKCS12
  keystore files):
    > PEM files are extracted from the provided PKCS12 and used for
      the driver. The plugin uses the provided PKCS12 keystore file.

Change-Id: I553ca135241e03484db6debbe186e6d34d582bf4

This is the commit message #2:

WL#15746 - Adding ML encryption support to BM

Enabling ML encryption on Baumeister:
- Certificates are generated on MySQLd during initialization
- Needed certicates for workers are packaged and sent to worker nodes
- Workers use packaged files to generate their certificates
- Arguments are added to driver.py invoke
- Keystore path is added to mysql config

Change-Id: I11a5cc5926488ff4fbf91bb6c10a091358db7dc9

This is the commit message #3:

WL#15746: Enhanced CRL Daemon Checker

Issue
=====
The previous design assumed a plain HTTPS link for the CRL distribution
point, accessible to all. This assumption no longer holds, as public
accessibility for CRL distribution contradicts OCI guidelines. Now, the
CRL distribution point in certificates provided by the control plane is
expected to be protected by OCI Instance Principal Authentication.
However, using this authentication method introduces a delay of several
seconds, which is impractical for HeatWave-AutoML.

Solution
========
The CRL fetching code now uses OCI Instance Principal Authentication.
To mitigate performance issues, the CRL checking process has been
redesigned. Instead of enforcing CRL checks per connection in MySQL
Plugin and HeatWave-AutoML Driver communications, a daemon thread in
HeatWave-AutoML Driver, Dask scheduler, and Dask Worker now periodically
fetches and verifies the CRL against all active connections. This
separation minimizes performance impacts. Consequently, MySQL Plugin's
CRL checks have been removed, as checks in the Driver, Scheduler, and
Worker sufficiently cover all cluster nodes.

Changes
=======
- Implemented CRL checker as a daemon thread in Driver, Scheduler, and
  Worker.
- Each connection/socket has an associated CRL checker.
- CRL checks occur periodically at set intervals.
- Skips CRL check if the CRL is temporarily unavailable.
- Failing a CRL check results in the associated connection/socket being
  closed. On the Driver, a stop event is triggered (akin to CTRL-C).

Change-Id: Id998cfe9e15d9236291b0ae420d65c2197837966

This is the commit message #4:

WL#15746: Fix Dask workers being shutdown without releasing address

Issue
=====
Dask workers getting shutting but not releasing the address used
properly sometimes.

Solution
========
Reverted some changes in heatwave_cluster.py in dask worker shutdown
function. Hopefully this will fix the address issue

Change-Id: I5a6749b5a25b0ccb73ba7369e545bc010da1b84f

This is the commit message #5:

WL#15746: Implement Dask Worker Join Timeout for Head Node

Issue:
======
In the cluster_shutdown method, the join operation on the head node's
worker process lacked a timeout. This led to potential indefinite
waiting and eventual hanging of the head node.

Solution:
=========
A timeout has been introduced for the worker process join on the head
node. Unlike non-head nodes, which rely on worker join to complete Dask
tasks and cannot have a timeout, the head node can safely implement
this. Now, if the worker process on the head node fails to shut down
post-join, indicating a timeout, it will be manually terminated. This
ensures proper release of associated resources and prevents hanging of
the head node.

Additional Change:
==================
Added Cert Rotation Guard for DASK clusters. This feature initiates on
the first plugin-driver connection when the DASK cluster is off,
recording the certificate's expiry date. During driver idle times,
it checks the current cert's expiry against this date. If it detects a
change, indicating a certificate rotation, it shuts down the DASK
cluster. The cluster restarts on the next connection request, ensuring
the use of the latest certificate.

Change-Id: Ie63a2e2b7664e05e1622d8bd6503663e13fa73cb
laurynas-biveinis pushed a commit to laurynas-biveinis/mysql-server that referenced this pull request Jul 9, 2024
…EXCEPT SELECT 4)

A work-around is to set the optimizer flag to not use hash map
de-duplication for INTERSECT, EXCEPT, like so:

SET optimizer_switch="hash_set_operations=off";

With hash_set_operations enabled, however, we get too may result rows.
For the IN predicate, the set operation is computed repeatedly, with
filters pushed down to set set operation operands:

-> Filter: <in_optimizer>(c.pk,<exists>(select mysql#2))  (cost=2.25 rows=20)
    -> Covering index scan on c using idx_c_col_datetime_key  (cost=2.25 rows=20)
    -> Select mysql#2 (subquery in condition; dependent)
        -> Limit: 1 row(s)  (cost=2.61..2.61 rows=1)
            -> Table scan on <except temporary>  (cost=2.61..2.61 rows=1)
                -> Except materialize with deduplication  (cost=0.1..0.1 rows=1)
                    -> Filter: (<cache>(c.pk) = <ref_null_helper>(2))  (cost=0..0 rows=1)
                        -> Rows fetched before execution  (cost=0..0 rows=1)
                    -> Filter: (<cache>(c.pk) = <ref_null_helper>(4))  (cost=0..0 rows=1)
                        -> Rows fetched before execution  (cost=0..0 rows=1)

Only the row with pk==2 should pass the filters under the except node, and that's
what happens. However, on repeated execution, the hash map used to implement
the Except materialize is not re-initialized to being empty.

The patch adds reinitialization of the hash map for such cases.

Change-Id: Idf2e36f9085e36748900017a0aad420e4e476f78
laurynas-biveinis pushed a commit to laurynas-biveinis/mysql-server that referenced this pull request Jul 9, 2024
…rong result

This error happens both with hash_set_operations on or off. If we look
    at the explain, we can see why it happens:

    -> Filter: <in_optimizer>(c.pk,<exists>(select mysql#2))
        -> Covering index scan on c using idx_c_col_datetime_key
        -> Select mysql#2 (subquery in condition; dependent)
            -> Limit: 1 row(s)  <--------------------- OK optimization
                -> Table scan on <except temporary>
                    -> Except all materialize
                        -> Limit: 1 row(s)  <--------------------------- problem
                            -> Table scan on <union temporary>
                                -> Union all materialize
                                    -> Filter: (<cache>(c.pk) = <ref_null_helper>(2))
                                        -> Rows fetched before execution
                                    -> Filter: (<cache>(c.pk) = <ref_null_helper>(2))
                                        -> Rows fetched before execution
                        -> Filter: (<cache>(c.pk) = <ref_null_helper>(2))
                            -> Rows fetched before execution

There is a limit node on top of the left EXCEPT operand right over the
UNION ALL node which shouldn't be there. It used to be a clever
optimization when MySQL only had UNION set operations, but it clearly
is wrong for EXCEPT ALL. For EXCEPT DISTINCT, UNION and INTERSECT, it
is fine, though.

The solution is to skip pushed down limits in the presence of EXCEPT
ALL inside subqueries, unless it is the top query block in the
subquery's query_expression, i.e. we retain the top-most limit 1 above.

Change-Id: Idf784cbfbe8efbaca03ad17c8c42d73ab7acaa1a
gipulla pushed a commit that referenced this pull request Oct 15, 2024
… and .6node3rpl

Issue #1
 Problem:
   Test fail in 4node4rpl (1 node group).
 Solution:
   Skip test when there is only one NG.

Issue #2
  Problem:
    Test fail in 6node3rpl (2 node group) with timeout.
    Test idea is to restart, with nostart option, *ALL* nodes
    in same node group to check if QMGR handles it wrongly as
    "node group is missing".
    In the test only two nodes in same node group are restarted,
    it works for 2 replica setups but, for 4 replica, test
    hangs waiting cluster to enter a noStart state.
  Solution:
   Instead of restart exactly 2 nodes, restart ALL nodes in a
   given node group.

Change-Id: Iafb0511992a553723013e73593ea10540cd03661
msprajap pushed a commit that referenced this pull request Oct 15, 2024
…N (SELECT 2 EXCEPT SELECT 4)

A work-around is to set the optimizer flag to not use hash map
de-duplication for INTERSECT, EXCEPT, like so:

SET optimizer_switch="hash_set_operations=off";

With hash_set_operations enabled, however, we get too may result rows.
For the IN predicate, the set operation is computed repeatedly, with
filters pushed down to set set operation operands:

-> Filter: <in_optimizer>(c.pk,<exists>(select #2))  (cost=2.25 rows=20)
    -> Covering index scan on c using idx_c_col_datetime_key  (cost=2.25 rows=20)
    -> Select #2 (subquery in condition; dependent)
        -> Limit: 1 row(s)  (cost=2.61..2.61 rows=1)
            -> Table scan on <except temporary>  (cost=2.61..2.61 rows=1)
                -> Except materialize with deduplication  (cost=0.1..0.1 rows=1)
                    -> Filter: (<cache>(c.pk) = <ref_null_helper>(2))  (cost=0..0 rows=1)
                        -> Rows fetched before execution  (cost=0..0 rows=1)
                    -> Filter: (<cache>(c.pk) = <ref_null_helper>(4))  (cost=0..0 rows=1)
                        -> Rows fetched before execution  (cost=0..0 rows=1)

Only the row with pk==2 should pass the filters under the except node, and that's
what happens. However, on repeated execution, the hash map used to implement
the Except materialize is not re-initialized to being empty.

The patch adds reinitialization of the hash map for such cases.

Change-Id: Idf2e36f9085e36748900017a0aad420e4e476f78
laurynas-biveinis pushed a commit to laurynas-biveinis/mysql-server that referenced this pull request Oct 22, 2024
This is a combination of 5 commits.

This is the 1st commit message:

WL#15746: TLS Enhancements for HeatWave-AutoML & Dask Comm. Upgrade

Problem:
--------
- HeatWave-AutoML communication was unauthenticated, unauthorized,
  and unencrypted.
- Dask communication utilized TCP, not aligning with FedRamp
  guidelines.

Solution:
---------
- Introduced TLS and mTLS in HeatWave-AutoML's plugin and driver for
  authentication, authorization, and encryption.
- Applied TLS to Dask to ensure authentication, encryption, and
  authorization.

Dask Authorization (OCID-based):
--------------------------------
1. For each DBsystem:
    - MySQL node sends OCIDs of authorized nodes to the head driver
      via:
        a. rapid_net_nodes
        b. rapid_net_allowed_ocids (older API, mainly for MTR tests)
    - Scenarios:
        a. All OCIDs provided: Dask authorizes.
        b. Any OCID absent: ML call fails with message.
2. During Dask worker registration to the Dask scheduler, a script is
    dispatched to the Dask worker for execution, retrieving the worker's
    OCID for authorization purposes.
    - If the OCID isn't approved, the connection is denied, terminating
      the worker and causing the ML query to fail.
3. For every Dask worker (both as listener and connector), an OCID-
    based authorization is performed post SSL/TLS connection handshake.
    The process compares the OCID from the peer's certificate against
    the allowed_ocids received from the HeatWave-AutoML MySQL plugin.

HWAML Plugin Changes:
---------------------
- Sourced certificate data and SSL setup from disk, incorporating
  SSL/TLS for HWAML.
- Reused "keystore" variable to specify disk location for
  certificate retrieval.
- Certificates and keys expected in PKCS12 format.
- Introduced "have_ml_encryption" variable (default=0).
    > Acts as a switch to explicitly deactivate HWAML network
      encryption, akin to "disable_net_encryption" affecting
      network encryption for HeatWave. Set to 1 to enable.
- Introduced a customized verifier function for verify_callback to
  be set in SSL_CTX_set_verify and used in the handshake process
  of SSL/TLS. The customized verifier function will perform
  instance id (OCID) based authorization on the plugin side during
  standard SSL/TLS handshake process.
- CRL (Certificate Revocation List) checks are also conducted if CRL
  Distribution Points are present and accessible in the provided
  certificate.

HWAML Driver Changes & OCID-based Authorization:
------------------------------------------------
- Introduced "enable_encryption" (default=0).
    > Set to 1 to enable encryption.
- When receiving a new connection request and encryption is on, the
  driver performs OCID-based self-checking, comparing OCID retrieved
  from its own instance principal with the OCID in the
  provided certificate on disk.
- The driver compares OCID from "mysql_compute_id" and extracted OCID
  from mTLS certificate during connection.
- Introduced "cert_dir" argument for certificate directory
  specification.
- Expected files: cert_chain.pem, certificate.pem, private_key.pem.
    > OCID should be in the userID (UID) or CN field of the
      certificate.pem subject.
- CRL (Certificate Revocation List) checks are also conducted post
  handshake, if CRL Distribution Points are present and accessible in
  the provided certificate, alongside OCID authorization.

Encryption Behavior:
--------------------
- If encryption is deactivated on both plugin and driver side, HWAML
  will work without encryption as it was before this commit.

Enabling Encryption:
--------------------
- By default, "have_ml_encryption" and "enable_encryption" are set to 0
    > Encryption is disabled by default.
- For the HWAML plugin:
    > "have_ml_encryption" set to 1 (default is 0).
    > Specify the .pfx file's path using the "keystore".
- For the HWAML Driver:
    > "enable_encryption" set to 1 (default is 0)
    > Specify "mysql_instance_id" and "cert_dir".

Testing:
--------
- MTR has been modified for the encryption setup.
    > Runs with encryption if "OCI_INSTANCE_ID" is set to a valid
      value.
- On OCI (when "OLRAPID_KEYSTORE" is not set):
    > Certificates and keys are generated; PEMs for driver and PKCS12
      for plugin.
- On AWS (when "OLRAPID_KEYSTORE" is set as the path to PKCS12
  keystore files):
    > PEM files are extracted from the provided PKCS12 and used for
      the driver. The plugin uses the provided PKCS12 keystore file.

Change-Id: I553ca135241e03484db6debbe186e6d34d582bf4

This is the commit message mysql#2:

WL#15746 - Adding ML encryption support to BM

Enabling ML encryption on Baumeister:
- Certificates are generated on MySQLd during initialization
- Needed certicates for workers are packaged and sent to worker nodes
- Workers use packaged files to generate their certificates
- Arguments are added to driver.py invoke
- Keystore path is added to mysql config

Change-Id: I11a5cc5926488ff4fbf91bb6c10a091358db7dc9

This is the commit message mysql#3:

WL#15746: Enhanced CRL Daemon Checker

Issue
=====
The previous design assumed a plain HTTPS link for the CRL distribution
point, accessible to all. This assumption no longer holds, as public
accessibility for CRL distribution contradicts OCI guidelines. Now, the
CRL distribution point in certificates provided by the control plane is
expected to be protected by OCI Instance Principal Authentication.
However, using this authentication method introduces a delay of several
seconds, which is impractical for HeatWave-AutoML.

Solution
========
The CRL fetching code now uses OCI Instance Principal Authentication.
To mitigate performance issues, the CRL checking process has been
redesigned. Instead of enforcing CRL checks per connection in MySQL
Plugin and HeatWave-AutoML Driver communications, a daemon thread in
HeatWave-AutoML Driver, Dask scheduler, and Dask Worker now periodically
fetches and verifies the CRL against all active connections. This
separation minimizes performance impacts. Consequently, MySQL Plugin's
CRL checks have been removed, as checks in the Driver, Scheduler, and
Worker sufficiently cover all cluster nodes.

Changes
=======
- Implemented CRL checker as a daemon thread in Driver, Scheduler, and
  Worker.
- Each connection/socket has an associated CRL checker.
- CRL checks occur periodically at set intervals.
- Skips CRL check if the CRL is temporarily unavailable.
- Failing a CRL check results in the associated connection/socket being
  closed. On the Driver, a stop event is triggered (akin to CTRL-C).

Change-Id: Id998cfe9e15d9236291b0ae420d65c2197837966

This is the commit message mysql#4:

WL#15746: Fix Dask workers being shutdown without releasing address

Issue
=====
Dask workers getting shutting but not releasing the address used
properly sometimes.

Solution
========
Reverted some changes in heatwave_cluster.py in dask worker shutdown
function. Hopefully this will fix the address issue

Change-Id: I5a6749b5a25b0ccb73ba7369e545bc010da1b84f

This is the commit message mysql#5:

WL#15746: Implement Dask Worker Join Timeout for Head Node

Issue:
======
In the cluster_shutdown method, the join operation on the head node's
worker process lacked a timeout. This led to potential indefinite
waiting and eventual hanging of the head node.

Solution:
=========
A timeout has been introduced for the worker process join on the head
node. Unlike non-head nodes, which rely on worker join to complete Dask
tasks and cannot have a timeout, the head node can safely implement
this. Now, if the worker process on the head node fails to shut down
post-join, indicating a timeout, it will be manually terminated. This
ensures proper release of associated resources and prevents hanging of
the head node.

Additional Change:
==================
Added Cert Rotation Guard for DASK clusters. This feature initiates on
the first plugin-driver connection when the DASK cluster is off,
recording the certificate's expiry date. During driver idle times,
it checks the current cert's expiry against this date. If it detects a
change, indicating a certificate rotation, it shuts down the DASK
cluster. The cluster restarts on the next connection request, ensuring
the use of the latest certificate.

Change-Id: Ie63a2e2b7664e05e1622d8bd6503663e13fa73cb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants