[MDEV-31746] Problems with tx_isolation after MDEV-21921 Created: 2023-07-19  Updated: 2023-10-10  Resolved: 2023-08-10

Status: Closed
Project: MariaDB Server
Component/s: Variables
Affects Version/s: 11.1.1
Fix Version/s: 11.1.1

Type: Bug Priority: Blocker
Reporter: Lawrin Novitsky Assignee: Sergei Golubchik
Resolution: Fixed Votes: 1
Labels: None

Issue Links:
Problem/Incident
is caused by MDEV-21921 Make transaction_isolation into a sys... Closed
Relates
relates to MDEV-29629 Session state tracking does not track... Closed
relates to MDEV-31751 SET @@tx_isolation=X behaves like SET... In Review
relates to MDEV-15148 Protocol regression testing for the s... Open

 Description   

MDEV-21921 breaks almost all connectors, that read and track tx_isolation value - probably all except only C/C,. We can fix connectors to live with it, that is pretty easy fix. But if it breaks connectors it can also break applications. C/C applications for example.

tx_isolation isn't full alias of tranasaction_isolation - client can't set it and can't track it. It's a bit too radical and/or to fast deprecation to my taste.

If we try to set it directly, we don't get an error, we get warning about deprecation, but the value is not changed.

MariaDB [test]> select @@tx_isolation;
 
+----------------+
 
| @@tx_isolation |
 
+----------------+
 
| READ-COMMITTED |
 
+----------------+
 
1 row in set (0.000 sec)
 
 
 
MariaDB [test]> set @@tx_isolation='REPEATABLE-READ';
 
Query OK, 0 rows affected, 1 warning (0.000 sec)
 
 
 
MariaDB [test]> show warnings;
 
+---------+------+----------------------------------------------------------------------------------------------------------------------+
 
| Level   | Code | Message                                                                                                              |
 
+---------+------+----------------------------------------------------------------------------------------------------------------------+
 
| Warning | 1287 | '@@tx_isolation' is deprecated and will be removed in a future release. Please use '@@transaction_isolation' instead |
 
+---------+------+----------------------------------------------------------------------------------------------------------------------+
 
1 row in set (0.000 sec)
 
 
 
MariaDB [test]> select @@tx_isolation;
 
+----------------+
 
| @@tx_isolation |
 
+----------------+
 
| READ-COMMITTED |
 
+----------------+
 
1 row in set (0.000 sec)

So, the query hasn't really succeeded, but we don't get the error

If we attempt to track tx_isolation, the result is even worse - no error, no warning, the value is set, but the variable is not tracked.

Server version: 11.1.1-MariaDB mariadb.org binary distribution
 
 
 
Copyright (c) 2000, 2018, Oracle, MariaDB Corporation Ab and others.
 
 
 
Type 'help;' or '\h' for help. Type '\c' to clear the current input statement.
 
 
 
MariaDB [test]> SET session_track_system_variables='tx_isolation';
 
Query OK, 0 rows affected (0.000 sec)
 
 
 
MariaDB [test]> show variables like "%track%";
 
+--------------------------------+--------------+
 
| Variable_name                  | Value        |
 
+--------------------------------+--------------+
 
| session_track_schema           | ON           |
 
| session_track_state_change     | OFF          |
 
| session_track_system_variables | tx_isolation |
 
| session_track_transaction_info | OFF          |
 
+--------------------------------+--------------+
 
4 rows in set (0.001 sec)

We verified that with various connectors, that the variable is not actually tracked.
So, the very least that should be done is bringing some consistency - remove tx_isolation from session_track_system_variables and return warning about its deprecation the way it's done with setting of the tx_isolation.

But I think we should consider something more than that up to making it work as before, but with warnings of deprecation. Nobody reads warnings, so maybe something in between would be better - make tracking of tx_isolation work, but leave setting of it not working(it's probably not the blessed way of changing of isolation level anyway). Maybe even make it return error- it makes more sense for me.

 Comments   
Comment by Daniel Black [ 2023-07-20 ]

10.4.31

 
MariaDB [(none)]> select @@tx_isolation;
 
+-----------------+
 
| @@tx_isolation  |
 
+-----------------+
 
| REPEATABLE-READ |
 
+-----------------+
 
1 row in set (0.000 sec)
 
 
 
MariaDB [(none)]> set @@tx_isolation='READ-COMMITTED'
 
    -> ;
 
Query OK, 0 rows affected (0.000 sec)
 
 
 
MariaDB [(none)]> select @@tx_isolation;
 
+-----------------+
 
| @@tx_isolation  |
 
+-----------------+
 
| REPEATABLE-READ |
 
+-----------------+
 
1 row in set (0.000 sec)
 
 
 
MariaDB [(none)]> set @@session.tx_isolation='READ-UNCOMMITTED';
 
Query OK, 0 rows affected (0.000 sec)
 
 
 
MariaDB [(none)]> select @@session.tx_isolation;
 
+------------------------+
 
| @@session.tx_isolation |
 
+------------------------+
 
| READ-UNCOMMITTED       |
 
+------------------------+
 
1 row in set (0.000 sec)
 
 
 
MariaDB [(none)]> select @@tx_isolation;
 
+------------------+
 
| @@tx_isolation   |
 
+------------------+
 
| READ-UNCOMMITTED |
 
+------------------+
 
1 row in set (0.000 sec)
 
 
 
MariaDB [(none)]> SELECT VARIABLE_NAME, SESSION_VALUE, GLOBAL_VALUE FROM
 
    ->  INFORMATION_SCHEMA.SYSTEM_VARIABLES WHERE 
    ->   VARIABLE_NAME IN ('tx_isolation');
 
+---------------+------------------+-----------------+
 
| VARIABLE_NAME | SESSION_VALUE    | GLOBAL_VALUE    |
 
+---------------+------------------+-----------------+
 
| TX_ISOLATION  | READ-UNCOMMITTED | REPEATABLE-READ |
 
+---------------+------------------+-----------------+
 
1 row in set (0.002 sec)
 
 
 
MariaDB [(none)]> set @@tx_isolation='READ-COMMITTED'
 
    -> ;
 
Query OK, 0 rows affected (0.000 sec)
 
 
 
MariaDB [(none)]> SELECT VARIABLE_NAME, SESSION_VALUE, GLOBAL_VALUE FROM  INFORMATION_SCHEMA.SYSTEM_VARIABLES WHERE    VARIABLE_NAME IN ('tx_isolation');
 
+---------------+------------------+-----------------+
 
| VARIABLE_NAME | SESSION_VALUE    | GLOBAL_VALUE    |
 
+---------------+------------------+-----------------+
 
| TX_ISOLATION  | READ-UNCOMMITTED | REPEATABLE-READ |
 
+---------------+------------------+-----------------+
 
1 row in set (0.002 sec)

@@tx_isolation comes through the set variables as a {SHOW_}OPT_DEFAULT rather than a {SHOW_}OPT_SESSION. As such this won't end up in the session tracker (same code in 10.4 as 11.1). Are connectors relying on this behaviour? If they are using @@session.tx_isolation they should be fine.

Testing session tracking next....

Comment by Daniel Black [ 2023-07-20 ]

So two session tracking can exist session_track_transaction_info=STATE will track transaction info (as does the higher CHARACTERISTICS.

The session tracking here is in sql/sys_vars.inl +2412 hasn't materially changes since implemented MDEV-8931. Does it look right? It changes on OPT_DEFAULT but not OPT_SESSION. I'm a bit confused on this, but not a regression.

On variables.

I pulled up wireshark and looked at protocol for:

The session change in the OK packet did list tx_isolation.

 
MariaDB [test]>  SET session_track_system_variables='tx_isolation';
 
Query OK, 0 rows affected (0.000 sec)
 
 
 
MariaDB [test]> set @@session.tx_isolation='READ-COMMITTED';
 
Query OK, 0 rows affected, 1 warning (0.001 sec)

no session change

 
MariaDB [test]> set @@tx_isolation='READ-UNCOMMITTED';
 
Query OK, 0 rows affected, 1 warning (0.001 sec)

(which is probably the same as before)

Setting MariaDB [test]> set @@session.transaction_isolation='READ-UNCOMMITTED';
 
Query OK, 0 rows affected (0.001 sec)

However didn't result in a session change while monitoring tx_isolation. Should it? Should it be the case that monitoring all deprecated variables where a new variable names generates and event? And vis-verse?

Comment by Daniel Black [ 2023-07-20 ]

MDEV-31746.test 11.1-26b96094ecccc5683a22674e290aea109cfbd65a

 
--echo # tracking info on
 
--enable_session_track_info
 
 
 
SET session_track_system_variables='tx_isolation';
 
 
 
set @@session.tx_isolation='READ-COMMITTED';
 
 
 
set @@tx_isolation='READ-UNCOMMITTED';
 
 
 
set @@session.transaction_isolation='REPEATABLE-READ';
 
set @@transaction_isolation='REPEATABLE-READ';
 
 
 
--disable_session_track_info

result

 
# tracking info on
 
SET session_track_system_variables='tx_isolation';
 
set @@session.tx_isolation='READ-COMMITTED';
 
-- Tracker : SESSION_TRACK_SYSTEM_VARIABLES
 
-- tx_isolation
 
-- READ-COMMITTED
 
 
 
Warnings:
 
Warning	1287	'@@tx_isolation' is deprecated and will be removed in a future release. Please use '@@transaction_isolation' instead
 
set @@tx_isolation='READ-UNCOMMITTED';
 
Warnings:
 
Warning	1287	'@@tx_isolation' is deprecated and will be removed in a future release. Please use '@@transaction_isolation' instead
 
set @@session.transaction_isolation='REPEATABLE-READ';
 
set @@transaction_isolation='REPEATABLE-READ';

10.4.31 on first 3 test statements only

 
# tracking info on
 
SET session_track_system_variables='tx_isolation';
 
set @@session.tx_isolation='READ-COMMITTED';
 
-- Tracker : SESSION_TRACK_SYSTEM_VARIABLES
 
-- tx_isolation
 
-- READ-COMMITTED
 
 
 
set @@tx_isolation='READ-UNCOMMITTED';

So no regression, @@tx_isolation isn't tracked, only @@session.tx_isolation.

Comment by Daniel Black [ 2023-07-20 ]

odd behaviour in @@tx_isolation - MDEV-31751 has been there for a while.

As connectors need to track a user pushing SET SESSION transaction_isolation=X while monitoring tx_isolation, resolving with a fast tracked alias implementation like MDEV-29629 as discussed with serg.

Comment by Daniel Black [ 2023-07-27 ]

Review comments given last week.

  • explained the UNIQUE removal
  • no bit number existed as far as I can see so I didn't destroy the simplicity by the complexity of unpacking a bitmask for an alias of Sys_var_bit values that don't exist. The hash function taking two 64bit values to hash isn't excessive for something that currently isn't used.

Did change just now:

  • removed unnecessary overrides
  • changed tracking_id_key_len so that is just sizeof(offset} for all but Sys_var_bit which includes both offet and bitmask. No idea if overhead of virtual functions exceeds the hashing of an extra long long.

note on availability. I'm starting a flight soon to present at conference at weekend and will be on leave without communications immediately afterwards. Please make any additional changes without me.

Comment by Sergei Golubchik [ 2023-08-03 ]

Thanks. I've simplified the fix but kept your test case. Basically I removed the support for Sys_var_bit aliases as we don't have them, so cannot test the new code anyway. One point of controversy less. And removed that logic where if you track both aliases, you get two tracking events after modifying one variable — it would be more backward compatible to get only one event if one variable is modified. This made for a rather simple implementation at the end of the day.

Comment by Daniel Black [ 2023-10-10 ]

compatibility broken

Generated at Thu Feb 08 10:26:10 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.