Console View
|
Categories: connectors experimental galera main |
|
| connectors | experimental | galera | main | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
|
|
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Georg Richter
georg@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
CONC-847: Fix OOB read in unpack_fields() on truncated metadata packet When processing server field packets in unpack_fields(), the 12-byte binary metadata envelope starting at row->data[i] (containing charsetnr, display length, field type, flags, decimals, and filler bytes) is read without validating that row->data[i] stays within row->length. A malformed or truncated field packet sent by a server/proxy can cause row->data[i] to point near or beyond row->length, resulting in an Out-of-Bounds (OOB) read when unpacking binary field metadata. Fix this by introducing a strict boundary check verifying that at least 12 bytes remain in the row buffer starting from row->data[i] before unpacking metadata fields. If the check fails, unpack_fields() fails gracefully, sets CR_MALFORMED_PACKET, and returns NULL. Also add unit tests (test_conc847_valid and test_conc847_invalid) covering both standard field packet parsing and truncated OOB packet handling. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Georg Richter
georg@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Merge branch '3.3-security' into 3.3 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
forkfun
alice.sherepa@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Merge branch '13.0' into 'main' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Georg Richter
georg@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
CONC-846: Fix TLS verification check during auth-switch and certificate options logic Two issues resolved in TLS verification logic: 1. In run_plugin_auth(), the verification guard previously evaluated: (mysql->net.tls_verify_status & MARIADB_TLS_VERIFY_TRUST) This allowed non-hashing plugins (e.g. mysql_clear_password) to execute when only hostname verification failed (MARIADB_TLS_VERIFY_HOST = 2), because (2 & 1) evaluated to 0. Updated the check to evaluate any non-zero tls_verify_status, ensuring all verification failures block cleartext auth switches. 2. Fixed TLS verification enabling when ssl_ca or crl options are specified even if MYSQL_OPT_SSL_VERIFY_SERVER_CERT (MARIADB_OPT_TLS_VERIFY_SERVER_CERT) was explicitly disabled. Certificate authority files/CRLs are now correctly honored and loaded according to caller intent. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39492 Parallel Query: give constant select-list items a result field SELECT 42, a FROM t1 crashed the server whenever parallel_worker_threads was set. create_tmp_table() does not give a constant item a field, which is right for a query that materialises its result and can evaluate the constant once outside the table, but the parallel result table is not that. Its layout has to mirror the select list item for item, because a worker projects item i into field i and ships the record image, and the manager sends one Item_field per field to the client. With a constant in the list the table came out one field short, and worker_emit_row() ran off the end of the field array into a NULL Field pointer. The manager's send list was equally short, so even without the crash the client would have been sent the wrong number of columns. Any constant did it, a literal, a folded expression such as 1+1, or a session constant like CONNECTION_ID(). Pass TMP_TABLE_ALL_COLUMNS when building the result table, so every item of the select list gets a field, and assert the layout matches the list afterwards -- the transport is positional, so a mismatch from any other cause has to be refused rather than walked over. That exposed a second problem in the same place. create_tmp_table() overwrites param->func_count with the number of items it actually has to copy, and make_result_table() was called once per worker plus once for the manager from a single TMP_TABLE_PARAM counted once by the caller. A constant needs a field but no copy entry, so the count dropped to zero after the first table and every later one allocated fewer fields than its layout, tripping the assertion in Create_tmp_table::finalize(). Each result table now starts from a freshly counted param, which is what N identical layouts from one param needs in any case. Session constants come out right rather than merely not crashing: the clone is fixed on the manager's thread, so CONNECTION_ID(), USER() and DATABASE() carry the user session's values, not the worker's. The test asserts that against values captured outside the query. parallel_query_worker_side covers a literal, a string literal, a folded expression, an all-constant select list where no column of the scanned table reaches the result, and the session constants, plus a serial-vs-parallel fingerprint over a select list containing a constant so the case is known to run in the workers. Every one of them crashes the server without this commit. This commit was prepared with Claude Code: it found the crash while probing which expressions the gate accepts, traced both causes from the core file, and wrote the tests. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Daniel Black
daniel@mariadb.org |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39813 ST_GeomFromGeoJSON does not control recursion depth Geometry::create_from_json used strings as things that reinitialize the je (json_engine). By reinitializing with json_start the killed pointer gets reset, and also the concept of depth is reset. There are two states in create_from_json: 1. The search if for the type, once found ci is set 2. Search for the argument, arg becomes != T_NONE If the type is found first, ci become the class, and once the arg is found, continued execution on the same je occurs. If the argument comes before the "type", we've save a copy of the json_engine_t to process this argument. In both cases je after the execution is the most progressed pointer. Debug assertions on je state aren't enforced by code, if some invalid GeoJSON is passed, these are actual errors. Because Geometry::create_from_json is recursive, from geometrymcollections, a json_read_value is pushed from inside this function up to Item_func_geometry_from_json::val_str. Other Gis*::init_from_json start from their actual object and don't need a json_read_value to start. Test case an bug report thanks to byteoverride. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Michal Schorm
mschorm@redhat.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
CONC-818 report CR_SERVER_LOST on TLS connection close (#308) When the server closes a connection, ma_tls_read() in the OpenSSL and GnuTLS plugins unconditionally calls ma_tls_set_error(), which sets CR_SSL_CONNECTION_ERROR. The caller ma_net_safe_read() then preserves that error code instead of reporting the correct CR_SERVER_LOST. The Schannel plugin already handles this correctly: it returns 0 on SEC_I_CONTEXT_EXPIRED without setting any TLS error (schannel.c:640-642). Apply the same logic to OpenSSL and GnuTLS. Detect connection close and return 0 without setting error: OpenSSL: - SSL_ERROR_ZERO_RETURN: orderly close (close_notify) - SSL_ERROR_SYSCALL with empty error queue: EOF without close_notify (OpenSSL 1.x) - SSL_ERROR_SSL with SSL_R_UNEXPECTED_EOF_WHILE_READING: same EOF, reported differently by OpenSSL 3.x GnuTLS: - rc == 0: orderly close (close_notify) - GNUTLS_E_PREMATURE_TERMINATION: EOF without close_notify (GnuTLS 3.7.4+) Co-authored-by: Claude AI <[email protected]> |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Akshat Nehra
anehra@amazon.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39981 Fix ST_GEOMFROMGEOJSON returning wrong result with reversed key order ST_GEOMFROMGEOJSON requires JSON keys to appear in a specific order (type before geometries/features/geometry/coordinates). When keys appear in a different order, the JSON scanner is left in an inconsistent state because json_skip_level() is not called to advance past the value when the type has not yet been determined. Fix: add json_skip_level() calls in the geometries, features, and geometry key handlers when the type key has not yet been encountered. This mirrors the existing pattern in the coordinates handler. All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Georg Richter
georg@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Fix leak in test_conc847_valid | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Oleksandr Byelkin
sanja@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| New CC 3.4 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39492 Parallel Query: workers must read the manager's snapshot A parallel worker runs in its own THD, so it runs in its own transaction and opened its own read view at its first read. Nothing tied that view to the one the manager holds: the workers and the manager could each read a different version of the table, and the chunk boundaries the engine partitioned under the manager's view did not even belong to the snapshot the workers scanned. Both directions were visible. Under REPEATABLE READ a worker returned rows another session had committed after the manager's snapshot was taken, and rows the manager's own transaction had written but not committed went missing, because to the worker's view the manager was just another active transaction. The snapshot is now shared through a new transaction_participant method, clone_consistent_snapshot(thd, from_thd), reached from the SQL layer as ha_clone_consistent_snapshot(). Each worker calls it once, on its own thread, before it locks or reads anything; an engine that cannot share a snapshot leaves the method NULL. Sharing at the transaction level rather than through the parallel-scan interface is what makes this cover every table the worker reads: the inner tables of the join are read at the same point in time as the parallel-scanned driving table. InnoDB implements it by copying the source transaction's read view. ReadView::clone() installs the source's ReadViewBase state and opens the copy. It inherits the source's m_creator_trx_id rather than keeping the worker's own, so rows written by the sharing transaction itself stay visible through the copy -- that is what changes_visible() uses the creator id for -- and it inherits the source's isolation level, so a READ UNCOMMITTED source is still read without consulting a view at all. The source view is read under its m_mutex, the same protection the purge coordinator takes in ReadView::append_to(), and from_thd's handlerton data under its LOCK_thd_data. The manager's view is pinned in pscan_init_coordinator() before any worker is created and stays open until the workers have been reaped in quiesce_workers(), so there is always a snapshot to copy, and holding it is also what stops purge from removing the row versions the workers still need. A worker that finds no snapshot to adopt fails the query rather than reading a different one. parallel_query_snapshot: rows another session commits after START TRANSACTION WITH CONSISTENT SNAPSHOT stay invisible, both a single row and 999 rows spread over the whole clustered index, while the manager's own uncommitted row is returned. Each case also asserts from the optimizer trace that the query really did run in the workers. All three cases fail without this commit. This commit was prepared with Claude Code: it found that the workers were taking their own read views, then wrote the clone_consistent_snapshot plumbing, ReadView::clone() and the test. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39492 Parallel Query: a copy must share no node, not just no field The gate refused a copy that reached one of the original's Item_field objects. That is the condition under which rebinding damages the original, but it is not the only way a shared node hurts. A node that is not a leaf carries evaluation state, and several workers evaluating one shared object at once tear it. CREATE TABLE t6 (d DATE); SELECT * FROM t6 WHERE LEAST( UTC_TIME(), d ); wraps the constant argument in an Item_cache_time. Item_cache_int::deep_copy() is a shallow copy, so each worker got its own cache object pointing at the manager's UTC_TIME() item, and the workers evaluated that one item together. Time::Time() asserted on a MYSQL_TIME left half written. The shared object is an Item_func, not an Item_field, so the field-based test did not see it. Ask the general question instead: does the copy reach any object the original reaches. Item::find_item_processor() already answers that for one object, so the missing half was a way to enumerate a tree's nodes, added here as Item::collect_all_items_processor(). Together they replace both Field_enumerator helpers, and the check now covers the shallow-copying classes not yet met rather than the three now known. type_temporal_innodb no longer crashes with parallel_worker_threads forced on. It still differs there, in the row number a warning carries: the worker's THD does not track the manager's current row, so a relayed warning says row 0. That belongs with the other per-session counters the workers do not share. parallel_query_clone gains the query above, answered serially and again with workers. Each of that test's two cases fails on its own without this commit, the semijoin one on the in_use assertion in Field::val_int() as before, the new one on Time::Time(). This commit was prepared with Claude Code: it traced the assertion to the shared cache argument, generalised the check, and wrote the test. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Georg Richter
georg@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Fix OOB read in init_read_hdr() via dynamic column header validation In mariadb_dyncol.c, init_read_hdr() computed header pointer offsets and hdr->data_size without validating that the sum of fixed_hdr, header_size, and nmpool_size fit within str->length. Crafted dynamic column blobs with invalid metadata could push pointer offsets past the buffer bounds or cause unsigned integer underflow on hdr->data_size, leading to out-of-bounds reads in downstream functions. Add bounds check in init_read_hdr() to ensure header offsets do not exceed total buffer length, matching mariadb_dyncol_check(). Reported-by: AISLE Research |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39492 Parallel Query: fall back to serial when the engine declines run_worker_side_join() returned 1 -- error -- when init_parallel_workers() reported HA_ERR_UNSUPPORTED, although its own documented contract, and the comment on that very branch, say -1 means "the engine declined, run the query serially". No error was raised on the way out, because nothing had gone wrong: the engine simply refused the scan. do_select() turned that 1 into NESTED_LOOP_ERROR, so the statement failed with an empty diagnostics area and Protocol::end_statement() hit its DA_EMPTY assertion. In a release build the client gets a statement that neither succeeds nor reports an error. The engine declines this way for any read that is not a consistent read: pscan_init_coordinator()'s first check refuses the scan when select_lock_type != LOCK_NONE. So SELECT ... FOR UPDATE, SELECT ... LOCK IN SHARE MODE, CREATE ... AS SELECT and INSERT ... SELECT over a parallel-scannable table all crashed a debug server as soon as parallel_worker_threads was set -- the optimizer picks the table, then execution has nowhere to go. Return -1 so do_select() takes its serial path, which is what make_join_readinfo() left in place for exactly this case by keeping the table's serial read_first_record. parallel_query_fallback is the new test: FOR UPDATE, LOCK IN SHARE MODE, CREATE ... AS SELECT and INSERT ... SELECT each return their rows with workers enabled. All four reach the decline path (the DBUG_PRINT added here fires four times), and the first of them already crashes the server without this commit. This also un-breaks parallel_query_join and parallel_query_worker_side, which have been failing on this branch since they were recorded: both open with a CREATE ... AS SELECT that took the bad path. Their serial-vs-parallel comparisons are built from CREATE ... AS SELECT pairs, so now that the fallback works both sides run serially and those comparisons no longer say anything about worker-side execution. The assertion in parallel_query_join that claimed CTAS "runs worker-side" is corrected to state what the trace key actually reports, which is the optimizer's choice. Restoring a like-for-like comparison at scale needs a technique that does not write from a SELECT, and is left to a follow-up. This commit was prepared with Claude Code: it traced the DA_EMPTY assertion to the wrong return code with a DBUG trace, reduced the repro from CREATE ... AS SELECT to SELECT ... FOR UPDATE, and wrote the test. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Oleksandr Byelkin
sanja@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| new CC 3.3 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39492 Parallel Query: a worker's result table belongs to the worker create_tmp_table() runs on the manager's thread, so every worker's result table came out with the manager in TABLE::in_use, and nothing put the worker there afterwards, unlike the table copies in open_worker_tables(). Field::get_thd() hands out TABLE::in_use, so when worker_emit_row() projected an item into one of those fields and the projection raised a warning, the warning was raised on the manager's THD from the worker's thread. CREATE TABLE t1 (a TIME(6)); INSERT INTO t1 VALUES ('838:59:59.999999'); SELECT a, a + INTERVAL 2 YEAR FROM t1; produces ER_DATETIME_FUNCTION_OVERFLOW per row from Item::save_date_in_field(), which passes field->get_thd() down to the function that raises it. Three things followed. The worker's own error handler never saw the condition, because it is installed on the worker's THD and the condition was raised on the manager's, so instead of being relayed through the message queue it was stored directly in the manager's diagnostics area. That store came from a worker thread while the other workers ran, unsynchronised, on a structure the manager also uses. And the memory for it was charged to the worker's THD, since thread-specific allocations are accounted to the running thread, while the block itself belonged to the manager's Warning_info, so the worker's status_var.local_memory_used was still 2040 at destruction and destroy_background_thd() tripped the not-freed-memory assertion in ~THD. Put the worker in TABLE::in_use once the table is built, next to where its other tables get the same treatment. The warning is then raised on the worker's THD, PWT_error_handler relays it, and finalize_parallel_workers() surfaces it on the manager after the join, which is the path it was always meant to take. parallel_query_worker_side runs the query above serially and again with workers, and records both. The rows and the three warnings match, so the relay is checked for what the user sees rather than only for the absence of a leak. Reverting the one line makes the test abort in ~THD. type_time_hires now passes with parallel_worker_threads forced on. type_temporal_innodb still fails there, on Item_cache_time inside Item_func_min_max: a shallow-copied cache shared between workers, the same family as the copy that is not independent refused in 11481453439, but reached through a cache rather than a field, which that commit's shared-leaf test does not see. This commit was prepared with Claude Code: it traced the leaked block to the diagnostics area with safemalloc's report, found the manager's THD reaching the worker through Field::get_thd(), and wrote the test. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39492 Parallel Query: the workers' statistics belong to the session A worker counts its reads, its lock calls and everything else in its own THD, and ~THD puts them straight into the global counters. Nothing ever reached the session that asked for the work, so SHOW SESSION STATUS was short by whatever the workers did: after a parallel scan of a 1000-row table Handler_read_rnd_next reported around 500, the rows this thread read out of the materialised result, rather than the 1502 the same query reports serially. Nine tests in the main suite noticed, in Handler_read_%, COLUMN_DECOMPRESSIONS and Optimizer_join_prefixes_check_calls. Each worker now copies its status counters into its pwt_worker just before its THD is destroyed, and quiesce_workers() adds them to the session's own after joining every worker, so only one thread ever touches either side and no locking is involved. The worker then clears its own counters, which stops ~THD adding the same numbers to the global counters that the session will pass on later. Only the counters move. Memory accounting stays with the worker's THD, because more of that THD's memory is freed after the snapshot is taken and ~THD has to reconcile all of it with the global counters. Clearing with the clear_for_flush_status offset leaves those fields alone, and the snapshot drops its copies of them. Suppressing ~THD's accounting entirely instead, which is the obvious way to avoid counting the same numbers twice, loses that reconciliation: the server then reports an internal memory accounting error of a couple of hundred thousand bytes at shutdown. Session and global counters were not equally wrong. Global was right all along, because ~THD was adding to it, so this changes which session the work is attributed to and not the total. parallel_query_worker_side compares Handler_read_rnd_next for the same query run serially and in parallel, and requires the parallel run to report at least as much. Written as a comparison rather than an exact figure because a chunked scan ends at each chunk, so it reads a few more than a single scan does. Without the handover the parallel run reports about a third of the serial figure and the test fails. This commit was prepared with Claude Code. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39492 Parallel Query: count the queries the workers ran Nothing reported whether a query had actually been executed in parallel. The optimizer trace records which table the optimizer picked, but the engine can still decline afterwards and the query run serially, so the trace answers a different question, as parallel_query_join's assertion about CREATE ... SELECT used to claim wrongly. The tests that compare a parallel result against a serial one had to infer execution from a side effect instead, that the manager's Handler_read_rnd_next stays low because the workers do the reading. That inference is about to stop holding, the workers' statistics belong in the session and the next commit puts them there. Add Parallel_queries_executed, a session and global status counter, incremented in run_worker_side_join() once the workers are running, which is after the engine has had its chance to decline. include/parallel_query_fingerprint.inc now asserts on it: the serial run must show none and the parallel run must show some. That is the question the tests were always asking. This commit was prepared with Claude Code. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Georg Richter
georg@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Revert "Remove length checks in mthd_stmt_fetch_to_bind, keep only the sentinel" This reverts commit 47a31a98750fd7c805ba67414c6d3df787ce8b2c. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Sergei Golubchik
serg@mariadb.org |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40541 role vs user@localhost acl_cache key confusion localhost connections have ip=0, host="localhost". roles have ip="", host="" |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
bsrikanth-mariadb
srikanth.bondalapati@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40388: sequence.simple fails on replay The problem is that, when recording is enabled for the query such as, explain select * from seq_1_to_10; it recorded the table context having a DDL definition as: - CREATE TABLE `seq_1_to_10` ( -> `seq` bigint(20) unsigned NOT NULL, -> PRIMARY KEY (`seq`) -> ) ENGINE=SEQUENCE DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_uca1400_ai_ci; Now, when that context is replayed, the DDL statement is executed. But, we cannot create such a table, and instead it errors out saying ERROR 1050 (42S01): Table 'seq_1_to_10' already exists. Solution is to use: - CREATE TABLE IF NOT EXISTS seq_1_to_10 ...; ===== Also, there is a different way to use sequences as: - Create sequence s1; Explain select * from s1; Here, we should be recording the DDL statement, but no need to store the stats for it. However, we didn't record the DDL statement earlier. Moreover, sequence's next value should be the same in the replay environment. Solution here is to record the DDL for such a sequence as CREATE TABLE IF NOT EXISTS s1 ...; and also set its start value as the recorded environment's previous value using SELECT SETVAL(s1, prev_value); |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Georg Richter
georg@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Merge branch '3.4-security' into 3.4 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40431 heap: classify rb-tree insert failures, mark the table crashed when key recovery fails `hp_rb_write_key()` reported **every** rejected `tree_insert()` as `HA_ERR_FOUND_DUPP_KEY`, although `tree_insert()` also returns NULL when the allocation of the tree node fails. An out of memory during a BTREE-indexed UPDATE was therefore reported as a duplicate: the user got `ER_DUP_ENTRY` with a fabricated value, possibly naming a key that is not unique at all, `handler::is_fatal_error()` treated the allocation failure as not fatal for callers asking for `HA_CHECK_DUP_KEY`, and the `HA_ERR_OUT_OF_MEM` / `ENOMEM` arms of the `heap_update()` recovery list were unreachable on the rb-tree path. `tree_insert()` now records why it returned NULL in the new `TREE::error` (`TREE_ERROR_OOM` or `TREE_ERROR_DUP_KEY`), and `hp_rb_write_key()` maps that to `HA_ERR_OUT_OF_MEM` or `HA_ERR_FOUND_DUPP_KEY` (`HA_ERR_INTERNAL_ERROR` defensively, should a new NULL source appear). With the classification corrected, the allocation failure lands in the `HA_ERR_OUT_OF_MEM` arm of the `heap_update()` recovery, which moves the already changed keys back, so correcting the error report does not trade the wrong message for a silently corrupt index. The recovery at `err:` keeps its explicit list of error codes: those are the errors we know how to recover from, and recovering from errors whose meaning we do not know is worse than stopping. What changes around it: 1. `info->errkey` is only set for `HA_ERR_FOUND_DUPP_KEY`, the single error it describes; every other failure leaves the `-1` that `err:` starts with. 2. When the recovery itself cannot restore a key -- the re-insert of an old key value fails -- or the failure is outside the list, the index no longer describes the data and the table is marked **crashed** in the new `HP_SHARE::state_changed` (bits and macros modeled on the `state.changed` of Maria, see `storage/maria/maria_def.h`). A crashed table refuses every lock acquisition, read and write with `HA_ERR_CRASHED` ("Index for table is corrupt"). `heap_check_heap()` does not trust the mark: it clears it, re-validates the whole structure and marks the table crashed again when it finds damage, so a check of an intact table drops a mark that no longer protects anything, while a genuinely damaged table stays refused. `hp_clear()` -- reached through TRUNCATE or DELETE without WHERE -- clears the state along with the data, because it rebuilds the indexes from nothing. Refusing a table known to be corrupt beats continuing and delivering wrong results. 3. `delete_key()` is assumed to succeed: it allocates no memory, so it cannot fail unless the table is already inconsistent, and building recovery logic and injected failures for that case would complicate the code for a scenario that cannot happen on a healthy table. If it ever does fail, the error falls outside the recovery list and the table is marked crashed, preserving the damaged state for analysis. The debug-build consistency check in `ha_heap::external_lock(F_UNLCK)` skips tables already marked crashed: their inconsistency is known and deliberate, and re-detecting it would raise a second error into a diagnostics area that can already hold OK, firing the `Diagnostics_area` assertion the check exists to prevent. The allocation failure is injected inside `tree_insert()` itself: `simulate_tree_insert_oom` fails every node allocation until the caller disarms it, `once_simulate_tree_insert_oom` fails only the next one and disarms itself. The two names must not be a prefix of one another, because `DBUG_SET()` matches an existing keyword by prefix and would silently merge instead of adding. Callers arm the keywords themselves and keep an inert guard keyword in the list, because a keyword list that becomes empty while debugging is on matches every keyword. The unit test `hp_test_update` drives the failures through the engine API: the classification and its duplicate-key counterpart, an already moved key being moved back, and the crashed lifecycle -- marking, refusal of reads and writes, the check that clears a stale mark on an intact table and keeps a damaged one crashed, and the reset on emptying. With the defects reintroduced, 8 of its assertions fail. `heap.update_key_rollback` covers the same from SQL; without the fix its first UPDATE reports `ER_DUP_ENTRY 'Duplicate entry 101 for key k2'` on a key that is not unique. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
PranavKTiwari
pranav.tiwari@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| FIxed failing issues. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
PranavKTiwari
pranav.tiwari@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fixed | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
bsrikanth-mariadb
srikanth.bondalapati@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40388: sequence.simple fails on replay The problem is that, when recording is enabled for the query such as, explain select * from seq_1_to_10; it recorded the table context having a DDL definition as: - CREATE TABLE `seq_1_to_10` ( -> `seq` bigint(20) unsigned NOT NULL, -> PRIMARY KEY (`seq`) -> ) ENGINE=SEQUENCE DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_uca1400_ai_ci; Now, when that context is replayed, the DDL statement is executed. But, we cannot create such a table, and instead it errors out saying ERROR 1050 (42S01): Table 'seq_1_to_10' already exists. Solution is to use: - CREATE TABLE IF NOT EXISTS seq_1_to_10 ...; ===== Also, there is a different way to use sequences as: - Create sequence s1; Explain select * from s1; Here, we should be recording the DDL statement, but no need to store the stats for it. However, we didn't record the DDL statement earlier. Moreover, sequence's next value should be the same in the replay environment. Solution here is to record the DDL for such a sequence as CREATE TABLE IF NOT EXISTS s1 ...; and also set its start value as the recorded environment's previous value using SELECT SETVAL(s1, prev_value); |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Georg Richter
georg@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| CONC-820: Clamp server-provided field lengths to maximum bounds | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Oleksandr Byelkin
sanja@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| New CC 3.4 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40447 Cap HEAP block allocations derived from the memory ceiling `init_block()` sizes every `HP_BLOCK` allocation as `max_records / heap_allocation_parts` records. `max_records` is normally computed from the table memory ceiling (`max_heap_table_size` / `tmp_memory_table_size`), not from any estimate of the expected number of rows, so a tuned-up ceiling turns directly into giant allocations: with `tmp_table_size=16G` the **first row written** to an internal temporary table allocates a 256MB..2GB block, and the per-key `HASH_INFO` blocks are inflated the same way. For create-and-drop-per-statement tables (`SHOW`/`INFORMATION_SCHEMA` materializations, `DISTINCT`/`GROUP BY` temporary tables) such allocations are served by `mmap()` and unmapped again on drop by common malloc implementations, so every statement pays page fault-in and zeroing, page-table teardown with TLB-shootdown IPIs, and process-wide `mmap_lock` serialization. On a `SHOW FULL COLUMNS` loop workload with `tmp_table_size=16G` this loses 36% QPS at 16 threads, 50% at 64 and 60% at 128; blob-bearing I_S temporary tables newly qualify for HEAP since MDEV-38975, which exposed the pre-existing sizing heuristic to this workload. Fix: cap ceiling-derived block allocations at `heap_max_allocation_block` (4MB): 1. 4MB stays within the range that mainstream allocators (glibc, jemalloc, tcmalloc, mimalloc) recycle from their free lists instead of returning to the kernel on free, so per-statement blocks are reused with no syscalls at steady state. The nearest boundary is jemalloc's `oversize_threshold` (8MB); glibc's dynamic mmap threshold adapts up to 32MB. 2. 4MB is large enough to keep typical blob values (up to ~1MB) in a single continuation run, preserving zero-copy blob reads. 3. At the default `tmp_table_size` (16MB) blocks come out at 1-2MB, so default-configuration sizing is unchanged; the cap only binds for ceilings above ~64MB. An explicit `min_records` (`CREATE TABLE ... MIN_ROWS=N`) is a real row count expectation and still pre-sizes beyond the cap. The cap compares against the **caller-supplied** `min_records`, not the defaulted "optimize for 1000 rows" value: for rows wider than ~4KB (`heap_max_allocation_block / 1000`) the 1000-row default exceeds `cap_records` and would otherwise silently override the cap (8KB rows -> 8MB blocks, 64KB rows -> 64MB blocks, and a row wider than the cap itself -> an `INT_MAX32`-clamped 1GB block). Wide internal temporary table rows are reachable without `MIN_ROWS`: fields wider than the VARCHAR limit become out-of-row blobs, but many inline columns (multi-table joins with `DISTINCT`/`GROUP BY`, wide `CHAR` columns) can sum past 4KB. Rows wider than the cap itself cannot honor it and degrade to the existing 10-records-per-block floor (e.g. 5MB rows -> 64MB blocks), which is as close to the cap as a block that must hold at least a few whole rows can get. Tables larger than 4MB simply allocate more blocks; the `HP_PTRS` block tree (128-ary) accommodates this with no depth issues. Tests: - `storage/heap/hp_test_block_size-t.c`: unit tests asserting the record and hash key block `alloc_size` cap with a ceiling-derived `max_records` (keyed and keyless), `MIN_ROWS` override, unchanged small-table sizing, and full functionality across the first-block boundary at the capped geometry (45K rows, key reads, `heap_check_heap`); wide-row scenarios asserting the cap holds for 8KB and 64KB rows despite the defaulted `min_records`, that rows wider than the cap degrade to the 10-record floor instead of the 1000-record geometry, and that an explicit `min_records` still overrides the cap for wide rows. - `mysql-test/main/tmp_table_heap_alloc.test`: end-to-end check that `Max_memory_used` stays bounded when a small `SELECT DISTINCT` on a TEXT column and a `SHOW FULL COLUMNS` materialize under a 4GB ceiling. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39492 Parallel Query: give worker tables their place in the join A worker's private table copies came out of open_table_from_share() with TABLE::map still zero, because nothing assigns it outside the optimizer's setup_tables(). Item_field::used_tables() reads TABLE::map, so every item rebound onto a worker copy reported used_tables() == 0 -- it looked like a constant. pwt_clone_rebind() re-fixes the clone, and Item_cond::fix_fields() evaluates any argument that can_eval_in_optimize(), so cloning the condition of SELECT a, b, c FROM t1 WHERE a % 7 = 0 AND b > 100 evaluated "b > 100" on the manager thread, against a worker record buffer that no row had been read into. Debug builds assert in Field_long::val_int() on marked_for_read(); release builds read that unread buffer to compute a null-rejection cache. Any WHERE with more than one predicate was affected -- that is, most of them. Copy map and tablenr from the manager's table when the copy is opened, so a rebound item computes the same used_tables() as the item it was cloned from and nothing evaluates it at clone time. Mark all columns readable there too, before any cloning touches those tables, rather than at the start of worker_run_query() -- the read_set then holds for the whole life of the copy, and the per-table loop in worker_run_query() goes away. This was invisible because no test reached it. parallel_query_join and parallel_query_worker_side compared parallel against serial by building the same result twice with CREATE ... AS SELECT, and writing from a SELECT is a locking read, which makes the engine decline the parallel scan: both sides ran serially, so the multi-predicate WHERE never reached a worker clone. Those comparisons are rewritten to compare something that really does run in the workers. A result set that cannot be written to a table cannot be compared afterwards either, so each query now goes inside a non-mergeable derived table -- the inner select runs in the workers, the outer aggregate runs in the user thread -- and is reduced to COUNT(*), SUM(CRC32(...)) and BIT_XOR(CRC32(...)). That fingerprint is order-independent, which a parallel result needs since its rows arrive in batch-completion order, and it compares the whole multiset at full scale without printing it. The shared method lives in include/parallel_query_fingerprint.inc. Each comparison also asserts that the workers, not the user thread, did the scanning: both runs read the same materialized rows, but only the serial one also scans the driving table, so the serial run's Handler_read_rnd_next must be the larger of the two. That is what stops these comparisons from silently going hollow again -- with the parallel half forced back to 0 workers the check flips from 1 to 0 and the tests fail. Both tests were checked to be load-bearing by mutation: dropping one worker result row in 500 moves every fingerprint (713 -> 712, 5000 -> 4990, 4967 -> 4958, 6000 -> 5988). This commit was prepared with Claude Code: it root-caused the assertion to the zero table map, wrote the fingerprint comparison and the include, and ran the mutation checks. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40448 HEAP unique hash duplicate rejection re-hashes the full key value A rejected duplicate insert into a HEAP unique hash index paid the full-value key hash twice: `hp_write_key()` hashed the key to insert it (the documented contract was "the record was still added and the caller must call hp_delete_key for it"), and `hp_delete_key()` then hashed the same record **again** just to locate the bucket to unlink from. For long key values (BLOB keys, MDEV-38975) the second scan dominates duplicate-heavy workloads: `SELECT DISTINCT` over 20-50KB blob values (~80% duplicates) regressed 25-32% vs Aria tmp tables at low concurrency, with `my_uca_hash_sort_utf8mb4` doing 1.83x the hashing work for the identical query. The hash caching added by `de1765fb64d` (`HASH_INFO::hash_of_key`) already made all chain surgery hash-free; the two full-value hashes per rejected row were the entire cost. Fix: 1. **Probe before insert** (`hp_write_key()`): compute the hash once at the top; for `HA_NOSAME` keys without NULL key parts walk the key's chain under the pre-insert mask comparing cached `HASH_INFO::hash_of_key` values, comparing full key values only on a hash match. On a duplicate return `HA_ERR_FOUND_DUPP_KEY` with nothing modified; otherwise proceed with the linear-hash split and insertion reusing the already-computed hash. Records with equal full hashes share a bucket under any mask, so probing the pre-insert chain finds any duplicate. A successful insert costs exactly one full-value hash, as before; a rejected duplicate drops from two full hashes + insert + undo delete to one hash + compare, the same as a lookup. 2. **Error-path contract change**: `hp_write_key()` no longer inserts the key on a duplicate, so `heap_write()` rolls back only the preceding keys (unconditional `keydef--`, as for BTREE/ENOMEM), and `heap_update()`'s duplicate path now re-inserts the old key for the failing keydef for both algorithms (previously BTREE-only) before rolling back earlier keys. 3. **Delete-side hash reuse** (`hp_delete_key()`): when the row being deleted was positioned via the same hash index (`flag` set and `info->current_hash_ptr->ptr_to_rec == recpos`), take the hash from the index entry instead of re-scanning the key value; a `DBUG_ASSERT` cross-checks the cached hash in debug builds. This removes the remaining full-value hash from `DELETE`/`UPDATE` of rows located through the index (`heap_rkey()` already hashed the key). 4. **`heap_rfirst()`/`heap_rlast()`**: clear `info->current_hash_ptr` when rejecting a hash index with `HA_ERR_WRONG_COMMAND`. Both functions retarget `info->lastinx` before the algorithm check, so a stale `current_hash_ptr` from a previous search on a different key could otherwise satisfy the new cached-hash guard in `hp_delete_key()` and send the bucket lookup to the wrong chain (reachable through the heap API only; the SQL layer never issues ordered reads on hash indexes). New unit test `hp_test_write_dup-t` (105 assertions) wraps the key charset's `hash_sort` collation handler in a counting shim, asserting the exact number of full-value hashes for every operation: 1 per insert attempt (successful or rejected), 1 total for index-read + delete, 1 for an index-positioned re-key. Behavioral coverage: single- and multi-key rollback for INSERT and UPDATE duplicates, NULL key parts, non-unique keys, delete after rejected `heap_rfirst()`/`heap_rlast()`, and a 500-row duplicate-heavy stress across linear-hash splits (exactly 500 hashes; was 827 before the fix). Benchmarked (16 threads, 120s runs, 8c/16t AMD 7840HS; `base` = preview-13.1 without MDEV-38975 = Aria tmp tables, `new` = unfixed, `fix` = this patch): | test (QPS) | base | new | fix | |---------------|------|--------------|---------------------| | `blob_case_c` | 4.65 | 3.36 (-28%) | 5.61 (+21% vs base) | | `blob_mixed` | 5.87 | 4.13 (-30%) | 6.92 (+18% vs base) | `hp_delete_key()` (38% inclusive before) is absent from the fixed profile; the low-concurrency regression becomes a win on top of the existing 2x+ high-concurrency wins. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39492 Parallel Query: check every table the worker will open A worker opens its own copy of every non-const table of the join, not only the one it scans in chunks, but the gate tested only the driving table. An internal tmp table -- a materialized derived table or subquery sitting as an inner table -- has a share built in memory rather than read from a .frm, so open_table_from_share() walked off the end of it and the server died in open_worker_tables(). One statement was enough, SET optimizer_switch='derived_merge=off'; SELECT ta.a, d.n FROM ta, (SELECT a AS k, COUNT(*) AS n FROM tb GROUP BY a) d WHERE d.k = ta.a; and it accounted for the largest group of crashes when the whole main suite was run with parallel_worker_threads forced on: subselect_sj2_mat, subselect_sj2, subselect_sj2_jcl6, derived_split_innodb, subselect-crash_15755 and group_min_max_innodb. Apply table_can_be_parallel_scanned() to every table in the join, and refuse a join tab with no table at all. The function name reads oddly for a table the worker only looks rows up in, but each condition it tests is one a worker-read table needs: an internal tmp table cannot be copied, blob payloads live outside the record buffer and do not survive the row transport whichever table they come from, a partitioned table cannot be opened as a plain copy, and the engine flag is also what tells us the engine can hand the worker the manager's snapshot. That last one closes the other half of this hole, a join whose inner table is in an engine that cannot share a snapshot and would have been read outside the manager's. The six tests above no longer crash. What is left in them is the plan and EXPLAIN churn that the 1/N cost discount causes in forced mode, plus, in the semijoin ones, a wrong result that this crash was hiding: the worker implements none of the semijoin duplicate-elimination strategies. The following commit refuses those plans. parallel_query_excluded gains the derived-table case, which asserts that the optimizer does not choose it and that the answer matches serial execution. This commit was prepared with Claude Code: it reduced the crash to the statement above, made the gate check every table, and wrote the test. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
forkfun
alice.sherepa@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Merge branch '13.0' into 'main' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40012 Parallel Query: execute the join in the worker threads Each parallel worker now runs the whole select-project[-join] query over its own chunk of the driving table -- WHERE filtering, select-list projection and the joins to the other tables -- and ships the final result rows to the manager, which only concatenates them and sends them to the client. This replaces the model where workers shipped raw source records and the manager ran the join. make_join_readinfo()'s gate (can_run_query_in_workers) chooses the worker-side path for an inner select-project[-join] with a parallel- scannable driving table: no tmp table (group/distinct/order/window/ buffer), no LIMIT/SQL_CALC_FOUND_ROWS/procedure/aggregate, no outer join or semijoin, and every non-driving table reached by eq_ref/ref/full scan. do_select() then runs run_worker_side_join() instead of the nested loop; anything ineligible runs serially. Each worker opens a private copy of every non-const table, deep-clones and field-rebinds the conditions and select list, and rebuilds each ref (clone_table_ref, mirroring create_ref_for_key). It scans its driving chunk, runs its own inner nested loop (cp_buffer_from_ref + ha_index_read_map / ha_index_next_same for ref/eq_ref, rnd scan otherwise), projects each full match into a private result table and ships the row image; the manager drains and sends. A killed worker's own ER_QUERY_INTERRUPTED is no longer treated as a fatal evaluation error (PWT_error_handler guards with !thd->killed) so kills keep propagating through kill_signal with the correct kill type. Standalone helpers are attached to the object they operate on (pwt_worker / pwt_manager members rather than file-static functions) and the functions that carry real control flow have DBUG_ENTER tracing. Tests: parallel_query_worker_side (single table) and parallel_query_join (eq_ref, ref with fan-out, 3-table chain, full-scan inner table) compare the parallel result set against serial. parallel_query / parallel_query_oom moved to a plain SELECT (which now runs worker-side) and were re-recorded. This commit was prepared with Claude Code: it wrote the worker-side join execution (worker_join_inner / worker_emit_row, the per-worker table, ref and expression cloning) and the two new tests; the naming, the member-function layout and the DBUG tracing are the author's cleanup. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-36610 Subquery wrongly eliminated by table elimination When equality propagation (build_equal_items()) merges an equality that contains a subquery, such as "t1.a = (SELECT ...)", with an outer join's ON equality, it can inject a reference to that subquery into the ON expression. If the join columns have compatible types the subquery ends up as the constant of a multiple equality (which Item::walk() skips); if they differ (e.g. BIGINT vs INT) the field cannot be merged and the subquery is substituted in as a plain "tbl.col = (SELECT ...)" argument. In the latter case, if that outer join is removed by table elimination, mark_as_eliminated() walks the ON expression and flags the shared Item_subselect as eliminated. The subquery, however, still lives in another part of the query and has to be executed, tripping DBUG_ASSERT(!eliminated) in Item_subselect::exec() (and, in release builds, disabling the subquery cache and hiding it from EXPLAIN). The surviving reference can be: - a WHERE/HAVING/select-list/ORDER/GROUP expression (subquery written there and pushed down into the eliminated ON), or - the ON expression of an outer join that was not eliminated (subquery written in a surviving outer ON and pushed down into an eliminated inner one). Fix: after table elimination, walk the expressions that survive into execution (WHERE, HAVING, select list, ORDER/GROUP BY and the ON expressions of outer joins that were not eliminated) and clear the "eliminated" flag on any subquery still reachable from them. Because a subquery can also be the constant of a multiple equality, and Item::walk() does not visit an Item_equal's constant, Item_equal gets an unmark_as_eliminated_processor() override that descends into its constant explicitly. Assisted by Claude Opus |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Georg Richter
georg@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Merge branch '3.3' into 3.4 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39492 Parallel Query: refuse an expression copy that is not independent A worker repoints the Item_field leaves of its copy of the WHERE condition, the ref values and the select list at its own tables. That is only safe if the copy owns those leaves. Several item classes implement deep_copy() as a shallow copy while still holding child items, every Item_cache and Item_outer_ref among them, so their copy keeps pointing at the original's children, and Item_cache::walk() does visit those children. Rebinding one moved the manager's own Item_field onto worker one's table, then worker two moved it again, so every worker and the manager ended up sharing a field bound to some other thread's table. The in_use assertion in Field::val_int() caught it in a debug build, SELECT STRAIGHT_JOIN count(*) FROM t1 JOIN t2 JOIN t3 WHERE t1.f1 IN (SELECT f1 FROM t4) AND t2.f1 IN (SELECT f1 FROM t5); crashing after semijoin conversion left a cached reference in the condition of a select the workers ran. A release build reads another thread's record buffer, and the manager's own plan is left rebound to a table that is closed when the workers are reaped. The gate now requires more than clonability, it requires the copy to share no Item_field object with the item it came from. Testing it there costs nothing, the gate already deep-copies every candidate item and throws the copy away, and a query that fails the test declines to serial execution at optimize time. pwt_clone_rebind() asserts the same property before it rebinds anything, so a copy that slips through is caught at the point of damage rather than by whatever reads a foreign table later. Checking for a shared leaf rather than enumerating the classes that shallow-copy means the check also covers the classes not yet met. There are more of them than Item_cache: Item_outer_ref and Item_copy_string hold child items and copy shallowly too. The three tests that hit this with parallel_worker_threads forced on -- opt_hints_join_order, innodb_mrr_cpk and subselect_innodb -- no longer crash. parallel_query_clone is the new test: the semijoin query above, plus a correlated reference in a select list and one in a condition, each answered serially and again with workers enabled. Without this commit the first of them trips the assertion in pwt_clone_rebind(), and without that assertion too it reaches the original in_use crash. This commit was prepared with Claude Code: it traced the assertion from the core file to the shallow deep_copy, wrote the shared-leaf check and the test. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
ParadoxV5
paradox.ver5@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39880: Reïmplement MDEV-37146 to include MDEV-39519 MDEV-39519 added MySQL 8 compatibility to `mariadb-dump --dump-slave` by attempting `SHOW REPLICA STATUS` first and then `SHOW SLAVE STATUS`. This conflicted with MDEV-37146, where `mariadb-dump --dump-slave` queries either `SELECT … FROM information_schema.SLAVE_STATUS` or `SHOW ALL SLAVES STATUS` depending on the server version. This commit merges MDEV-37146 and MDEV-39519: * Use MDEV-39519’s strategy based on syntax error handling. * Use MDEV-37146’s preference order: 1. `SELECT … FROM information_schema.SLAVE_STATUS` 2. `SHOW ALL SLAVES STATUS` 3. `SHOW REPLICA STATUS` (for MySQL compatibility _only_) * Send `STOP`/`START REPLICA SQL_THREAD FOR CHANNEL '…'` commands for both MariaDB (est. 10.7) and MySQL. * Refactor column indices to variables set when a query succeeds. * Partially revert MDEV-37146’s removal of `--dump-slave`’s support for pre-GTID & pre-multi-source, but tailored for MySQL compatibility; coverage for MariaDB pre-10.0 is not fully restored. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39492 Parallel Query: worker tables inherit the manager's column bitmaps A worker's table copies were marked with use_all_columns(), which points both read_set and write_set at the share's all_set. Correct, since it is a superset of what the worker evaluates, but coarser than it needs to be in two ways. InnoDB builds its row fetch template from read_set, so with every column marked a worker converts every column of every row it scans to MySQL format, not just the ones its cloned condition and select list reference -- work paid on the hot path that parallel query exists to shorten, and it grows with the width of the table rather than with the query. And an all-columns write_set on a table the worker only ever reads gives up an assertion: a store into a source field no longer trips marked_for_write(). Copy the manager table's read_set and write_set into the copy's own def_read_set/def_write_set instead, and point the copy at those. The optimizer has already marked exactly the columns this query reads, and open_worker_tables() runs after it has finished, so those bitmaps are final; the copy owns its bitmaps, so nothing is shared with the manager, and column_bitmaps_set() signals the engine to rebuild its template. This keeps the property the marking was added for: every column a cloned item could touch is in the read_set before any cloning happens, because the optimizer marked precisely that set. Two cases in parallel_query_worker_side cover the fidelity of the copy, both through the serial-vs-parallel fingerprint: a table with eighteen columns where the query references two, and a virtual column whose base column is read but never projected. Clearing a single bit of the copied read_set makes both of them fail (Field_long::val_int() asserts on marked_for_read()), as does parallel_query_join. Index-only reads are unaffected by this: they need ha_start_keyread(), which the worker-side path never issues, for any read_set. This commit was prepared with Claude Code: it identified the cost of the blanket marking, made the copy, and added the two coverage cases. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39492 Parallel Query: refuse semijoin strategies the worker cannot run The gate refused semijoin materialization, through bush_children, and LooseScan, through loosescan_match_tab, but nothing else. FirstMatch and DuplicateWeedout left the plan looking like an ordinary inner join in the flat table list, and pwt_worker::worker_join_inner() is a plain nested loop that knows nothing of either, so it emitted exactly the duplicates they exist to remove. SET join_cache_level=0; SET optimizer_switch='materialization=on,semijoin=on,firstmatch=on,loosescan=off'; SELECT * FROM t1 JOIN t2 ON (t2.f4 = t1.f3) WHERE ( 8 ) IN (SELECT t3.f1 FROM t3, t4); answers two rows serially. Run by the workers under a FirstMatch plan it answered three, one of them repeated and the other lost, and the count varied with how the chunks fell. It was only reachable after the previous commit stopped this shape of query crashing. That the plan, not the parallel execution, was at fault could be ruled out by keeping parallel_worker_threads at 4, so the cost model still produced the FirstMatch plan, and adding FOR UPDATE, so the engine declined the parallel scan and the same plan ran serially. That answers two rows. Refuse a join tab carrying any semijoin strategy state: sj_strategy, do_firstmatch, check_weed_out_table, flush_weedout_table, first_weedout_table, and loosescan_match_tab as before. Testing the strategy rather than the shape of the join means the two strategies that hide in a flat table list are covered by the same condition as the two that do not. parallel_query_excluded gains the query above in its smallest form, asserting that the optimizer does not choose it and that it answers one row per matching outer row rather than one per inner match. This commit was prepared with Claude Code: it found the wrong result while checking what the previous commit had uncovered, isolated plan from execution, and wrote the test. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||