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

PXB-3403: Merge 9.1 #1635

Open
wants to merge 2,599 commits into
base: trunk
Choose a base branch
from
Open

PXB-3403: Merge 9.1 #1635

wants to merge 2,599 commits into from

Conversation

aybek
Copy link
Contributor

@aybek aybek commented Jan 17, 2025

No description provided.

kahatlen and others added 30 commits August 20, 2024 13:19
A query in main.derived_condition_pushdown occasionally failed when
running with the hypergraph optimizer.

The problem was that one of the input tables to a hash join was an
internally generated temporary table, and the hash join code was not
prepared for this possibility. In such tables,
table->pos_in_table_list is nullptr, so the hash join code shouldn't
unconditionally dereference table->pos_in_table_list.

Fixed by adding checks for nullptr.

Change-Id: I299933c95a1b1330f821ee7f2d62f1b01b98cd47
Change-Id: Ia22eb89411136d20f385ececf8c7ed34d8d9fdb9
…noclose]

Move the test which covers the DERIVED_CONDITION_PUSHDOWN and
NO_DERIVED_CONDITION_PUSHDOWN table-level hints to an include file
which can be reused for a hypergraph optimizer variant of the test in
a later patch.

Change-Id: I396cbdbc7bab63cdbf17d6a7718c7dd92b19bf55
Enable a test which covers the DERIVED_CONDITION_PUSHDOWN and
NO_DERIVED_CONDITION_PUSHDOWN table-level hints with the hypergraph
optimizer.

Change-Id: Icb95ccf6a3e64882da368864e8320a3a753f0333
Add a cmake library ext::rpc with necessary INTERFACE properties.
Get rid of the global
   INCLUDE_DIRECTORIES(BEFORE SYSTEM "${TIRPC_INCLUDE_DIR}")
and
   ADD_DEFINITIONS(-DHAVE_TIRPC)

Also some cosmetic cleanup of cmake code.

Change-Id: I0b2e1885bde584bfc4c22b9f8f7590a43e8b4225
…045 are never hit

Various issues here :

 1.  Those error inserts are not hit as they are in SUMA execSUB_CREATE_REQ()
     which is only executed on SUMA when a Subscriber (aka EventOperation) is
     created.
     Just creating the Event is not enough to trigger them, so the test
     covers nothing and naturally they would be left behind when it finishes.

     Solution :
       - Add create EventOperation and check that it fails

 2.  The other error inserts (error2) are related to SUMA Scans, where it
     asks DIH to get distribution info.  The error inserts are intended
     to give code coverage of some 'many fragments' continueB cases.

     a) This has nothing to do with Events
     b) Creating and Dropping events does not involve scanning

     Solution :
       - Split into separate testcase
       - Add index creation which *will* trigger a SUMA scan and will
         cause these error insertions to be hit.

 3.  Error insert 13049 causes a crash when hit.
     This error insert triggers a CONTINUEB, but then carries on with
     the scan.
     When the CONTINUEB arrives, the scan is not in the correct state
     causing a crash.
     Fix by having the code return after queueing the CONTINUEB.

Summary :
  Testcase was a mess, testing nothing.
  Error insert being left behind was a good indicator of a problem.

New test :
  test_event -n SumaScanGetNodesContinueB T1 added to daily-basic--03

Change-Id: I4c4470cc6ef2a53dd470a4e4ac90ca1085504140
Change-Id: Iaab86d1a56ee6cb1c101ba64b23c693d06e6dbe9
Change-Id: I6402e68df1d2cd2a1ee20d6116667d39c583b976
…045 are never hit

Various issues here :

 1.  Those error inserts are not hit as they are in SUMA execSUB_CREATE_REQ()
     which is only executed on SUMA when a Subscriber (aka EventOperation) is
     created.
     Just creating the Event is not enough to trigger them, so the test
     covers nothing and naturally they would be left behind when it finishes.

     Solution :
       - Add create EventOperation and check that it fails

 2.  The other error inserts (error2) are related to SUMA Scans, where it
     asks DIH to get distribution info.  The error inserts are intended
     to give code coverage of some 'many fragments' continueB cases.

     a) This has nothing to do with Events
     b) Creating and Dropping events does not involve scanning

     Solution :
       - Split into separate testcase
       - Add index creation which *will* trigger a SUMA scan and will
         cause these error insertions to be hit.

 3.  Error insert 13049 causes a crash when hit.
     This error insert triggers a CONTINUEB, but then carries on with
     the scan.
     When the CONTINUEB arrives, the scan is not in the correct state
     causing a crash.
     Fix by having the code return after queueing the CONTINUEB.

Summary :
  Testcase was a mess, testing nothing.
  Error insert being left behind was a good indicator of a problem.

New test :
  test_event -n SumaScanGetNodesContinueB T1 added to daily-basic--03

Change-Id: I4c4470cc6ef2a53dd470a4e4ac90ca1085504140
Change-Id: Ia1809ab6933cf4614f612716c996028f13f58922
Change-Id: I29b679727270f635e34f6c3a33559504b6f2a7a4
Change-Id: I259b97c51f98dc252886e84b8411151c99ae674a
Context:
Test inject error 8064 to delay LQH_TRANSREQ during TC takeover.
To force the takeover master node is restarted in 4 different
ways: normal node restart, error insert 7007 (that triggers 9999),
dump code 7213 and dump code 7214. The last 2 are sent to other
node from where master is restarted (via 9999 or CRASH_INSERTION).

Problem is with 7213 that is used to kill the master during
a COMMIT. Node sends 5048 to master (that should crash it) followed
by a 9999 (probably to ensure that master will be restarted if 5048
does not hit immediately).

Problem:
Between send of EI 5048 and 9999 error_insert_value is cleared via
CLEAR_ERROR_INSERT_VALUE macro but, (since
Bug#36082229 [1/2] Check there are no active error inserts set
after NDBAPI test run.
f0ec71b05ea6bc5e131e7d409ba31a1bc7995545),
CLEAR_ERROR_INSERT_VALUE also clears the error_insert_extra variable
that, in this context, is used to store the nodeId of master node.
This way 9999 is sent to self instead of master node.

Solution:
Remove unnecessary CLEAR_ERROR_INSERT_VALUE call between send of EI
5048 and EI 9999.

Change-Id: Ia630b8e07660720886cab54ece620a0ff32ac5a5
Change-Id: Ic63531538a4cd63b5f1ea39e22c60928273d7a35
Change-Id: If24942795f493ccebbe769190958e8f4355400a7
When a user variable is used in a DML statement, the variable and
its value are allocated in the THD::user_var_events_alloc. Before
the DML statement is written to the binlog, the user variable and
its corresponding values from THD::user_var_events_alloc are also
recorded in binlog.

For stored procedures(SP), memory for user variables and their
associated values is allocated within the procedure's execution
memory root as each DML statement is written separately to the
binlog. At the end of stored procedure execution, THD::user_var_events_alloc
is set to nullptr and it is reset to THD::mem_root before executing
the next statement.

However, when executing statements using the Statement handle
interface, THD::user_var_events_alloc is not reset to THD::mem_root
after statement execution or before processing the next statement.
This leads to the reported issue when any statement is executed
following a stored procedure using the Statement handle interface.

To address this issue, THD::user_var_events_alloc is now reset to the
statement execution memory root after executing a sub-statement from
the statement handle interface.

Change-Id: Ie943d36e6f75dfa673e8d9ca5afd9125d21533a9
Change-Id: I91a6ac30c94432080a829b921495aff9a8470fd7
Fixed incorrect object length in unit test causing read past allocated
data. Updated copyright notice to new format. Fixed doc comments.

Change-Id: Id10d46c67a8a875e19aeb55e5c6d0be2d5f423d4
Change-Id: I2b710dfbb42533ee5e5d57a7a90804e7a24d9e08
…runk

Change-Id: I4c5cc97eea7a2522103e153e4422d63ebb19133b
…real_item()

The preparation of table value constructors doesn't keep track of the
number of hidden columns added for expressions in the ORDER BY clause.
This could cause problems later in the resolving and make the server
exit.

Fixed by counting the number of hidden items in table value
constructors the same way as it's done in regular query blocks.

Change-Id: Icb62214afb9a4a56c3bb92c648c1460cbf4b82c0
  - Bug#36804093 ERROR 4153 (HY000): Incorrect double value:
    '-1.7976931348623157e308' for column
  - In debug code, there is a basic validation of characters allowed in
    a double column.  Included character e.
  - In release code, use DBL_MAX for double.
  - Added a test case to cover the scenario.

Change-Id: I610299dc07a5dc846d691a453ffb3263958bbcc1
Unpack curl-8.9.1.tar.gz

$rm -rf buildconf.bat config.guess config.sub configure docs ltmain.sh m4 Makefile packages plan9 projects src tests winbuild

git add curl-8.9.1

Change-Id: I643cbfe146e964e965122e9cb58055aec19ffb7c
Update CURL_VERSION_DIR

Changes to curl cmake files:
  - cmake_minimum_required
  - CMAKE_DEBUG_POSTFIX
  - Keep FILE protocol for HTTP_ONLY
  - find_package(OpenSSL)
  - optional_dependency(ZLIB) to reuse results from cmake/zlib.cmake
  - Do not add "/MP" for clang on Windows

Change-Id: If0d63d027b2ea319767e207cd493d7dbba23cf0f
Delete old source files.

Change-Id: Ic7f63b43a8c2e744b3fc802ede76f947a0d6a53a
Add a cmake library ext::rpc with necessary INTERFACE properties.
Get rid of the global
   INCLUDE_DIRECTORIES(BEFORE SYSTEM "${TIRPC_INCLUDE_DIR}")
and
   ADD_DEFINITIONS(-DHAVE_TIRPC)

Also some cosmetic cleanup of cmake code.

Change-Id: I0b2e1885bde584bfc4c22b9f8f7590a43e8b4225
(cherry picked from commit 8efb8b269bac6a1b617ec5ae7c89a278e0a467b9)
Unpack curl-8.9.1.tar.gz

$rm -rf buildconf.bat config.guess config.sub configure docs ltmain.sh m4 Makefile packages plan9 projects src tests winbuild

git add curl-8.9.1

Change-Id: I643cbfe146e964e965122e9cb58055aec19ffb7c
(cherry picked from commit 9de4385bffbe66f4718d6dddbfbf3dd8fae75ffb)
Update CURL_VERSION_DIR

Changes to curl cmake files:
  - cmake_minimum_required
  - CMAKE_DEBUG_POSTFIX
  - Keep FILE protocol for HTTP_ONLY
  - find_package(OpenSSL)
  - optional_dependency(ZLIB) to reuse results from cmake/zlib.cmake
  - Do not add "/MP" for clang on Windows

Change-Id: If0d63d027b2ea319767e207cd493d7dbba23cf0f
(cherry picked from commit c6863849c0d9a8a9b6760bdeb1904da26a69a006)
Delete old source files.

Change-Id: Ic7f63b43a8c2e744b3fc802ede76f947a0d6a53a
(cherry picked from commit 17111ab258762c5ac206d00ed1f93b8a11b8c4c7)
Tor Didriksen and others added 14 commits September 14, 2024 18:48
…de()

An SQL statement had heap-use-after-free when run as a prepared statement.
The failing statemen was a functional index in a create table statement,
and objects created ended up in the execution MEM_ROOT.

The solution is to switch to the prepare MEM_ROOT when doing resolving
of functional indexes.

Change-Id: Ic54ef5eaaa4b90e1018cb82b9c73f5f6c5e29e01
…-grant-tables=1

Problem: When the server starts with skip-grant-tables and then
privileges checking is re-enabled e.g. by FLUSH PRIVILEGES it is
done in 2 steps: ACL loading and grant loading. The authorization
cache was set to initialized after 1. step, so the authorizations
done between these steps used not fully initialized cache.

Fix:
1) Moved setting initialized flag after grant loading.
2) Added assert before hash table searches.
3) ALL_ACCESS returned in get_*_grant when the flag is not set.

Test: Run an SQL command when FLUSH PRIVILEGES is stopped
between the described steps.

Change-Id: Idafa39296f4fd33aae1725f3f544c5af4724d23b
Additional patch, removing MINIMAL_RELWITHDEBINFO from the RPM spec.

Change-Id: I40a76fcd1c3228ff41a68c6d4954ce6cd63319ad
              causes service to shut down

There are two problems here: With a production binary, we see failures
in check_column_privileges() or failing to obtain a virtual function
for an Item_func_eq object.

With a debug binary, execution fails earlier in an assert:
Query_expression::set_prepared(), Assertion `!is_prepared()' failed.

Due to the assert failing in debug mode, it is difficult to get to the
actual source of the problem, but we can narrow the debug issue down
to establishing a proper execution context for an independent item
to be evaluated in function sp_prepare_func_item(). The assumption
for this function is that the current LEX and query expression found
through thd->lex can be set to "prepared" and "executing" state.

However, sp_prepare_func_item() can be used for two kinds of items.
Some of these are standalone and have an associated LEX object and
a query expression object. Examples of these are the procedure
instructions sp_instr_set and sp_instr_jump_if_not. Others are
expressions that are created below a LEX that is already in an
executing state. Examples are items used to assign values to procedure
arguments after procedure execution, and assignment to local variables
after query execution. The latter is the problem in this case.

The solution to the first problem is to distinguish the two cases and
pass a bool argument for the standalone items and false for the
remaining ones. In sp_prepare_func_item(), we then ensure that
preparation and execution state is set properly for the standalone
items, whereas we ensure by inspecting the current LEX object that we
are already in an executing context for the dependent items.

The second problem is related to the first one. It started appearing
after the fix for bug#35856247 was pushed, which changes the way
base_ref_items are saved in Query_block::save_properties().
But the solution also fixes this problem, since we no longer save
properties for dependent items, which will interfere with the already
saved properties for the current query block.

We also had to set proper execution state in
Sql_cmd_get_diagnostics::execute() and Sql_cmd_load_table::execute()
because otherwise these functions would fail the newly added asserts.

Change-Id: Ib5d31c07994340006818e180eed74e1d6aa23280
…load to HeatWave - rollback [3/3]

Rollback of commit 708cd63af81dc35cfaba2417a8c3a186cadfa8bf
due to HW regression.

Change-Id: Ia401e25e0bd277a4e875ca35d0570ef8e376eee2
binlog_transaction_dependency [1/2]

Changed the data structure from Tree map to ankerl::unordered_dense::map of
variable m_writeset_history in sql/rpl_trx_tracking.h file which enhances
performance.

Changed CMakeFiles to support #include <ankerl/unordered_dense.h>

Change-Id: I59835525559a41d47305463d07e903b88d6a8696
Change-Id: I3663f4e7cb9201ef767a3d8b680c0de92ef57470
Post-push fix: The shared library mysqlrouter_mysqlclient was
installed in the wrong folder on windows.

Change-Id: I446e73dc365d6690f84ae0205f08d4d46f7d70ce
Approved-by: Erlend Dahl <[email protected]>
Change-Id: I8297ae3ef968412549bf7acd2e1e2aa7701cef91
Approved-by: Erlend Dahl <[email protected]>
Change-Id: I276ded1dbb0647b1521b06c8ebde5278cb6e303d
Move the service API header file from the GPL sources to a cloud-only
directory.

Part 1: Delete header file from ./include/mysql/components

Change-Id: I311a85e5b0808c70b2dbc0d37a5aae501de478ce
Used an alternative RapidJSON sequence that doesn't cause the macosx
dyld to keep the shared obbject around post- dlclose().

Also stabilized the firewall.firewall-binlog-entries test by excluding
the autoinc id that is non-consequential for the test

Change-Id: Iaf1df12540f6a61a3827a6d293940bf93a068dd6
group_replication_group_name anymore when storing usage data

Removed the reading of the group_replication_group_name system variable
by the option_tracker component.
Updated the doxygen about this.
Removed setting the variable to some weird value during tests.
Re-recorded the affected test result.
Reading the row updates now ignores the cluster ID too.

Change-Id: I76eba54d52931c3ba7b7a892bac7ab400eb1eb8d
@it-percona-cla
Copy link

it-percona-cla commented Jan 17, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 13 committers have signed the CLA.

✅ aybek
❌ roylyseng
❌ trosten
❌ gkodinov
❌ frazerclement
❌ kaankara
❌ bjornmu
❌ kahatlen
❌ tiagoportelajorge
❌ jdduncan
❌ weigon
❌ dahlerlend
❌ aibek.bukabayev [email protected]


aibek.bukabayev [email protected] seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@aybek aybek changed the title Merge9.1 PXB-3403: Merge 9.1 Jan 17, 2025
@aybek aybek force-pushed the merge9.1 branch 2 times, most recently from 6161279 to e50d308 Compare January 21, 2025 05:43
aybek added 4 commits January 21, 2025 07:07
components/keyrings/common/config/config_reader.cc
upstream: Migrate the keyring_aws plugin to a component 9dde815 9dde815
ps: Implementing KMIP keyring component, added some helper methods 5e1fd79 5e1fd79

components/keyrings/common/config/config_reader.h
upstream: Migrate the keyring_aws plugin to a component 9dde815 9dde815
ps: Implementing KMIP keyring component, added some helper methods 5e1fd79 5e1fd79

components/keyrings/common/json_data/json_writer.cc
upstream: fixed static analysis issues in keyring code 8a7dede

components/keyrings/keyring_file/CMakeLists.txt

components/keyrings/keyring_file/keyring_file.cc
upstream: added Option and option usage tracker component 01e4c24

man/CMakeLists.txt
upstream: Remove FILE globbing, use explicit list of man pages to install. 1c91597

storage/innobase/dict/dict0dd.cc
upstream: added a new line below the code where we converged dfecee6
added new include file 28eb1ff
Increase INSTANT ADD/DROP MAX_ROW_VERSION e64e217

storage/innobase/log/log0recv.cc
upstream: solved the bug of parallel tablespace scan in 8.0 not working as expected 742fd15

storage/innobase/include/ut0log.h
pxb: Use new logging mechanism in xtrabackup 78d62a7
upstream: Upgrade to clang-format 15  8742743

storage/innobase/os/os0file.cc
pxb: introduced new method during keyring_file removal in order to fix the failing test case  6ed6532
upstream: added comment /* _WIN32 */ 5e4407d
upstream: The `ulint purpose` is removed from signature of `os_file_create()`. The `OS_FILE_AIO` and `OS_FILE_NORMAL` are removed. The `ulint type` of `os_file_create()` method is renamed to `purpose` 3881f88

storage/innobase/fil/fil0fil.cc
upstream: Remove extra support for FusionIO 688a682

Test fixes:

storage/innobase/xtrabackup/test/t/xb_export_check_cfg.sh
For EXPORT, the CFG version is bumped to 8 as previously 255 was being written to indicate an invalid version e64e217

storage/innobase/xtrabackup/test/t/xb_duplicate_sdi.sh
storage/innobase/xtrabackup/test/t/duplicate_sdi_datadir.tar.gz
storage/innobase/xtrabackup/test/t/dictionary_sdi.sh
updated the test datadir `duplicate_sdi_datadir.tar.gz` with mysql 8.4 version due to 8.1 can not be upgraded to 9.1

storage/innobase/xtrabackup/test/run.sh
added slash to the end of the basedir variable because upstream updated the keyring components loading path 0fe2378

storage/innobase/xtrabackup/test/t/simulate_different_version.sh
storage/innobase/xtrabackup/src/backup_mysql.cc
updated simulate higher version to 9.1

storage/innobase/xtrabackup/test/suites/keyring/innodb_encryption_mix_plugins.sh
storage/innobase/xtrabackup/test/suites/keyring/pxb-1793.sh
storage/innobase/xtrabackup/test/suites/keyring/shared_tablespace_encryption.sh
use keyring file component instead of keyring file
Upstream added consistency check assertions for redo log in commits 742fd15 7337680
Copy link
Contributor

@satya-bodapati satya-bodapati left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good mostly. Please check the review comments

share/messages_to_error_log.txt Outdated Show resolved Hide resolved
storage/innobase/log/log0recv.cc Outdated Show resolved Hide resolved

#ifndef UNIV_HOTBACKUP
} else {
recv_sys->missing_ids.insert(space_id);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing_ids logic is moved else where or completely removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is completely removed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please share the commit? missing ids is about handling MLOG_FILE_DELETE entries. So if a tablespace is not found but later a MLOG_FILE_DELTE entry is there, it should n't complain. But if a tablespace is missing and no MLOG_FILE_DELETE entry exists, it should error out.

storage/innobase/log/log0recv.cc Outdated Show resolved Hide resolved
storage/innobase/xtrabackup/src/backup_mysql.cc Outdated Show resolved Hide resolved
storage/innobase/xtrabackup/src/xtrabackup.cc Outdated Show resolved Hide resolved
@@ -12900,6 +12900,16 @@ ER_KEYRING_COMPONENT_AWS_READ_ONLY_LIMIT
ER_KEYRING_COMPONENT_AWS_CANNOT_ENCRYPT_READ_ONLY
eng "CMK was changed and the keyring must be re-encrypted, but the keyring is read only. Set the read_only config parameter to false for re-encryption and reload the component."

ER_KEYRING_COMPONENT_NO_CONFIG
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check the PS 9.1 code base and then adjust the xtrabackup error messages after that.

Comment on lines 421 to 425
// we will use xtrabckup 9.1 for all 9.x versions
mysql9x = pxb_version == "9.1" &&
(version_number > 90000 && version_number < 100000);

if (!mysql9x && pxb_version != server_version) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say this appraoch is fine until the last innovation release until the 9.7 LTS.

So please adjust it to fail on 9.7 version

@@ -2634,7 +2634,7 @@ static bool xtrabackup_read_info(char *filename) {
xb_server_version =
xtrabackup::utils::get_version_number(mysql_server_version_str);

// ut_ad(xb_server_version > 80000 && xb_server_version < 90000);
ut_ad(xb_server_version > 80000 && xb_server_version < 91000);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how will this pass if server is 9.2 or 9.3? 🤔 Please ensure it is OK until 9.7. 9.7 should fail.

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.