Details
-
New Feature
-
Status: Closed (View Workflow)
-
Critical
-
Resolution: Fixed
Description
Current FLUSH STATUS command has a bunch of issues:
- it resets many session variables and some global variables — but not all session and not all global, the decision was made per variable and was rather ad hoc
- it requires RELOAD global privilege, a user cannot reset status in his session other than by reconnecting
A more useful behavior could be:
- there are clear criteria what variables are reset and what aren't
- FLUSH [ SESSION | LOCAL ] STATUS flushes appropriate session variables, and requires no privileges at all
- FLUSH GLOBAL STATUS requires RELOAD and resets appropriate global variables
original description:
Currently there is now way to flush global status variables, stored in 'global_status_var', which is useful for testing.
Another issue is that FLUSH STATUS resets some global variables (all LONG and LONGLONG C global variables) which 'normal' users should not be allowed to do. This is not dangerous but can be confusing for shared instances.
The proposal is to implement FLUSH SESSION STATUS, which only resets thread local variables and FLUSH GLOBAL STATUS that resets global_status_vars and the currently reset global C variables.
FLUSH STATUS will be mapped to FLUSH SESSION STATUS and can be used without any privileges.
FLUSH GLOBAL STATUS will require the RELOAD_ACL, like now.
The first version, in 11.5 will not reset all global variables with FLUSH GLOBAL STATUS, just the variables that are reset for now.
In 11.6 we will extend it to reset all global summary variables, so please take this into account when using FLUSH GLOBAL VARIABLES.
Attachments
Issue Links
- causes
-
MDEV-33965 Additions to main.status are non-deterministic and inefficient (and fail in buildbot)
-
- Confirmed
-
-
-
- Closed
-
- includes
-
MDEV-4690 FLUSH GLOBAL STATUS
-
- Closed
-
- relates to
-
-
- Closed
-
Activity
To test this feature separately, please use bb-11.5-MDEV-33145-flush-global-status
Results
as of bb-11.5-MDEV-33145-flush-global-status dd182933a:
MDEV-33992(mariadb-admin flush-status) needs to be fixed. It is a trivial change, and I can't find any justification for preserving it the way it is, as it becomes a no-op;- the commits related to the feature need to be checked to make sure that nothing of the essence was added to bb-11.5-monty after the feature branch had been created, and that the one(s) I pulled into the feature branch from bb-11.5-monty were pushed in a right way, I'm not at all sure about it.
- for documentation: the list of status variables contains multiple occurrences of "The global value can be flushed by FLUSH STATUS", it will need to be modified.
Other than that, quality-wise I don't have objective reasons to disapprove pushing the feature into 11.5 and releasing it with the RC.
The minority opinion
I generally agree that having FLUSH GLOBAL STATUS is better than not having it; however, I don't see the feature in its current form as much of an improvement comparing to what we had before, while at the same time it can cause quite a lot of trouble for users who utilize FLUSH STATUS in their tools.
The declared goal which the task was meant to achieve was
there are clear criteria what variables are reset and what aren't
Let's take 3 status variables, Key_writes, Queries, and Com_insert.
We have the following status:
select t1.variable_name, t1.variable_value as global_value, t2.variable_value as session_value from information_schema.global_status t1 join information_schema.session_status t2 on t1.variable_name = t2.variable_name where t1.variable_name in ('Key_writes','Queries','Com_insert'); |
|
|
+---------------+--------------+---------------+ |
| variable_name | global_value | session_value |
|
+---------------+--------------+---------------+ |
| COM_INSERT | 6 | 2 |
|
| KEY_WRITES | 2 | 2 |
|
| QUERIES | 38 | 38 |
|
+---------------+--------------+---------------+ |
On the old server, we do FLUSH STATUS and we get this:
+---------------+--------------+---------------+ |
| variable_name | global_value | session_value |
|
+---------------+--------------+---------------+ |
| COM_INSERT | 6 | 0 |
|
| KEY_WRITES | 0 | 0 |
|
| QUERIES | 40 | 40 |
|
+---------------+--------------+---------------+ |
On the new server, we do FLUSH STATUS and we get this:
+---------------+--------------+---------------+ |
| variable_name | global_value | session_value |
|
+---------------+--------------+---------------+ |
| COM_INSERT | 6 | 0 |
|
| KEY_WRITES | 2 | 2 |
|
| QUERIES | 40 | 40 |
|
+---------------+--------------+---------------+ |
or, we do FLUSH GLOBAL STATUS instead, and we get this
+---------------+--------------+---------------+ |
| variable_name | global_value | session_value |
|
+---------------+--------------+---------------+ |
| COM_INSERT | 2 | 2 |
|
| KEY_WRITES | 0 | 0 |
|
| QUERIES | 40 | 40 |
|
+---------------+--------------+---------------+ |
I don't believe that new behavior brings much clarity to the majority of users. Maybe some will feel satisfaction from FLUSH SESSION STATUS not changing any global value, but that's about the only achievement here, while the main problem remains – no clear criteria what's reset and what's not, at least from the user perspective.
A self-contained MTR test case for this is at the end of the comment.
The change was also said to be useful for testing. Looking at the test changes in the feature patch, I didn't see much (or any) improvement, only a massive replacement of FLUSH with FLUSH GLOBAL. If a test needs to check that a value has or hasn't changed, it can easily save the old value and then compare it with the new one. Maybe it could sometimes cut a corner by expecting that FLUSH will reset everything to 0, and then there is no need to save the old value; but that's not such a big gain, especially given that, as above shows, FLUSH cannot be expected to reset everything to zero.
At the same time, what this change is likely to cause is inconvenience for at least some users caused by breaking backward compatibility. Any monitoring which used FLUSH STATUS in its logic will become unusable. The extent of the problem can be guessed from the amount of tests which had to be changed from FLUSH STATUS to FLUSH GLOBAL STATUS just so they continue doing what they were already doing. All users in all their scripts will have to do the same.
One simple (maybe over-simplified) example of a monitoring script which will stop working after the change: charting the max amount of connections used depending on the day time, e.g. per hour.
- once in an hour, in a loop:
- store MAX_USED_CONNECTIONS
- FLUSH STATUS
Or, a similar idea can be used for alerts instead. Let's say instead of storing the variable for further charting, the tool checks that it is greater than some minimal number of connections you always expect to have. If it didn't reach the minimum, something is wrong, connections aren't happening and investigation is due.
Before the change, all this would work. After the change, MAX_USED_CONNECTIONS will only grow, so the chart won't show anything, and alerts won't be triggered.
All scripts and third-party tools which have something similar in the logic will have to be modified.
If that's what we really want to do, I think a very clear warning needs to be given in the release notes, although I'm not very optimistic about users reading such warnings; besides, they may not know the internal details of third-party tools or even their legacy scripts.
|
Test case for 3 status variables above |
create or replace table t1 (a int, key(a)) engine=MyISAM; |
insert into t1 values (1),(2); |
|
|
--connect (con1,localhost,root,,)
|
insert into t1 values (3),(4); |
drop table t1; |
|
|
select t1.variable_name, t1.variable_value as global_value, t2.variable_value as session_value |
from information_schema.global_status t1 join information_schema.session_status t2 |
on t1.variable_name = t2.variable_name |
where t1.variable_name in ('Key_writes','Queries','Com_insert'); |
|
|
flush status;
|
|
|
select t1.variable_name, t1.variable_value as global_value, t2.variable_value as session_value |
from information_schema.global_status t1 join information_schema.session_status t2 |
on t1.variable_name = t2.variable_name |
where t1.variable_name in ('Key_writes','Queries','Com_insert'); |
|
|
flush global status; |
|
|
select t1.variable_name, t1.variable_value as global_value, t2.variable_value as session_value |
from information_schema.global_status t1 join information_schema.session_status t2 |
on t1.variable_name = t2.variable_name |
where t1.variable_name in ('Key_writes','Queries','Com_insert'); |
A fix for MDEV-33992 was pushed into bb-11.5-monty as https://github.com/MariaDB/server/commit/a7801b24bea623c3dd4187e6e503cd1cafee4430
It was cherry-picked (along with a post-fix) into bb-11.5-MDEV-33145-flush-global-status on top of the commit described in the previous comment, however it cannot be used "as is" as it has made main.mysqladmin fail, and thus requires another post-fix which is not yet available.
A fix for main.mysqladmin was pushed into bb-11.5-monty and cherry-picked into the feature tree.
Pushed to bb-11.5-monty