Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added sticky flag realization #825

Merged
merged 4 commits into from
Dec 29, 2016
Merged

Conversation

klinometr
Copy link

@klinometr klinometr commented Dec 7, 2016

Hello Rene,

We've just moved to cloud, and started using XtraDB cluster. We've experienced some troubles with highload, so we opened couple of servers for read and write. But it also cause us some troubles like deadlocks. The possible solution for this was making one Cluster Node for write and others for readonly.
We've searching for solutions and found yours, which is great.
But for now we can't use it due to transactions. For example:

BEGIN; ## Starting transaction
INSERT INTO testTable (value) (10); ## Inserting some value, for example deposit amount
SELECT SUM(value) FROM testTable; ## Finding sum of deposits
INSERT INTO testTableRelation (maxValue) (10); ##Saving sum of deposits for client.
COMMIT; ##Commiting transaction

If I have rules, like:

select rule_id,match_pattern,destination_hostgroup from mysql_query_rules; 
+---------+------------------------+-----------------------+
| rule_id | match_pattern          | destination_hostgroup |
+---------+------------------------+-----------------------+
| 1       | ^SELECT                | 2                     |
| 2       | .*                     | 1                     |
+---------+------------------------+-----------------------+

Select will go through destination hostgroup, which includes readonly servers, which don't know about transaction yet, so I wouldn't get actual sum of deposits. And flagIN, flagOUT works only for each query separately.

So I've come up with an idea:

select rule_id,match_pattern,flagIN,flagOUT,destination_hostgroup,sticky_flag from mysql_query_rules; 
+---------+------------------------+--------+---------+-----------------------+-------------+
| rule_id | match_pattern          | flagIN | flagOUT | destination_hostgroup | sticky_flag |
+---------+------------------------+--------+---------+-----------------------+-------------+
| 1       | ^SELECT .* FOR UPDATE$ | 0      | 1       | 1                     | 1           |
| 2       | ^START TRANSACTION     | 0      | 1       | 1                     | 1           |
| 3       | ^BEGIN                 | 0      | 1       | 1                     | 1           |
| 5       | ^ROLLBACK              | 1      | NULL    | 1                     | 0           |
| 6       | ^COMMIT                | 1      | NULL    | 1                     | 0           |
| 7       | .*                     | 1      | 1       | 1                     | 1           |
| 100     | ^SELECT                | 0      | NULL    | 2                     | 0           |
| 1000    | .*                     | 0      | NULL    | 1                     | 0           |
+---------+------------------------+--------+---------+-----------------------+-------------+

I added new parameter - sticky_flag, which means that if the sticky_flag is true, we will begin next query with current flagOUT

For example:

SELECT * FROM testTableRelation; ## Getting sum of deposits for client
## rule_id = 100, flagIN = 0, flagOUT = 0, sticky_flag = 0, hostgroup = 2,
## Works like earlier (hostgroup = 2, readonly Node)

BEGIN; ## Starting transaction
## rule_id = 3, flagIN = 0, flagOUT = 1, sticky_flag = 1, hostgroup = 1
## Saves flagOUT = 1 to session, so it will be flagIn starting value on next query
## Works like earlier, hostgroup = 1, write Node

INSERT INTO testTable (value) (10); ## Inserting some value, for example deposit amount
## rule_id = 7, flagIN = 1, flagOUT = 1, sticky_flag = 1, hostgroup = 1
## Saves flagOUT = 1 to session, so it will be flagIn starting value on next query
## Works like earlier, hostgroup = 1, write Node

SELECT SUM(value) FROM testTable; ## Finding sum of deposits
## rule_id = 7, flagIN = 1, flagOUT = 1, sticky_flag = 1, hostgroup = 1
## Saves flagOUT = 1 to session, so it will be flagIn starting value on next query
## New working flow, select query went to write Node, cause it's the only node, that knows about transaction, so it will get correct result

INSERT INTO testTableRelation (maxValue) (10); ##Saving sum of deposits for client.
## rule_id = 7, flagIN = 1, flagOUT = 1, sticky_flag = 1, hostgroup = 1
## Saves flagOUT = 1 to session, so it will be flagIn starting value on next query
## Works like earlier, hostgroup = 1, write Node

COMMIT; ##Commiting transaction
## rule_id = 6, flagIN = 1, flagOUT = NULL, sticky_flag = 0, hostgroup = 1
## Resets sticky_flag to false in session, so next time starting value for flagIN will be 0.

SELECT SUM(value) FROM testTable; ## Finding sum of deposits
## rule_id = 100,  flagIN = 0, flagOUT = NULL, sticky_flag = 0, hostgroup = 2
## Works like earlier (hostgroup = 2, readonly Node)

My version is not perfect, also I don't use C++ much, but we've tested it, and it is working like it supposed to.

@renecannao
Copy link
Contributor

@klinometr : thank you very much for the pull request!
I assume you were already familiar with mysql_users.transaction_persistent .
I must say the way you described how to solve this issue is really interesting!
I was already working on a different solution: I am sure you noticed fields sticky_conn and multiplex in mysql_query_rules, but isn't ready yet.
The way sticky_conn will work (isn't implemented yet) is that it will be possible to make a hostgroup/connection sticky based on a query rules.
I think the main difference between the two approaches are:

  • sticky_flag : it applies to the next query only
  • sticky_conn : it applies to all the query until sticky_conn is disabled again

I think the two approaches can live together, as they aren't mutually exclusive.
I will review the code later today: thank you again!

Few comments I have are:

  1. flagOUT is used for current query: making it dual purposes can be confusing
  2. related to point 1, instead of adding a flag (sticky_flag) I think will be less confusing and more efficient to have a column named next_query_flagOUT , so that the current query roles chaining is not affected (similar to mirror_flagOUT).
    Thoughts?

Thanks,
René

@klinometr
Copy link
Author

klinometr commented Dec 7, 2016

@renecannao

  1. About flagOUT - I think you're right
  2. How about next_query_flagIN? Because it is next query flagIN, so it won't confuse anyone.

Thanks,
Maxim

@renecannao
Copy link
Contributor

Hi Maxim,

  1. How about next_query_flagIN ?

I like that too!
Thanks,
René

@klinometr
Copy link
Author

@renecannao

Changed field according to our discussion. Changed a logic slightly.

select rule_id,match_pattern,flagIN,flagOUT,destination_hostgroup,next_query_flagIN from mysql_query_rules;
+---------+------------------------+--------+---------+-----------------------+-------------------+
| rule_id | match_pattern          | flagIN | flagOUT | destination_hostgroup | next_query_flagIN |
+---------+------------------------+--------+---------+-----------------------+-------------------+
| 1       | ^SELECT .* FOR UPDATE$ | 0      | NULL    | 1                     | 1                 |
| 2       | ^START TRANSACTION     | 0      | NULL    | 1                     | 1                 |
| 3       | ^BEGIN                 | 0      | NULL    | 1                     | 1                 |
| 5       | ^ROLLBACK              | 1      | NULL    | 1                     | 0                 |
| 6       | ^COMMIT                | 1      | NULL    | 1                     | 0                 |
| 7       | .*                     | 1      | NULL    | 1                     | NULL              |
| 100     | ^SELECT                | 0      | NULL    | 2                     | NULL              |
| 1000    | .*                     | 0      | NULL    | 1                     | NULL              |
+---------+------------------------+--------+---------+-----------------------+-------------------+

New field next_query_flagIN sets flagIN starting value for next query, this starting value changes, if only next_query_flagIN >= 0 (like mirror_flagOUT works)
It appears that I don't need flagOUT at all, but it still works

Example:

SELECT * FROM testTableRelation; ## Getting sum of deposits for client
## rule_id = 100, flagIN = 0, next_query_flagIN = -1, hostgroup = 2,
## Works like earlier (hostgroup = 2, readonly Node)

BEGIN; ## Starting transaction
## rule_id = 3, flagIN = 0, next_query_flagIN = 1, hostgroup = 1
## Sets next_query_flagIN = 1 to session, so it will be flagIN starting value on next query
## Works like earlier, hostgroup = 1, write Node

INSERT INTO testTable (value) (10); ## Inserting some value, for example deposit amount
## rule_id = 7, flagIN = 1, next_query_flagIN = 1, hostgroup = 1
## Doesn't change next_query_flagIN, so it stays equals 1
## Works like earlier, hostgroup = 1, write Node

SELECT SUM(value) FROM testTable; ## Finding sum of deposits
## rule_id = 7, flagIN = 1, next_query_flagIN = 1, hostgroup = 1
## Doesn't change next_query_flagIN, so it stays equals 1
## New working flow, select query went to write Node, cause it's the only node, that knows about transaction, so it will get correct result

INSERT INTO testTableRelation (maxValue) (10); ##Saving sum of deposits for client.
## rule_id = 7, flagIN = 1, next_query_flagIN = 1, hostgroup = 1
## Doesn't change next_query_flagIN, so it stays equals 1
## Works like earlier, hostgroup = 1, write Node

COMMIT; ##Commiting transaction
## rule_id = 6, flagIN = 1, next_query_flagIN = 0, hostgroup = 1
## Sets next_query_flagIN = 0 to session, so it will be flagIN starting value on next query

SELECT SUM(value) FROM testTable; ## Finding sum of deposits
## rule_id = 100,  flagIN = 0, next_query_flagIN = 0, hostgroup = 2
## Works like earlier (hostgroup = 2, readonly Node)
## Doesn't change next_query_flagIN, so it stays equals 0

Thanks,
Maxim

# Conflicts:
#	include/query_processor.h
#	lib/Query_Processor.cpp
@klinometr
Copy link
Author

Hi @renecannao,
Have you watched the code yet? Or have you finished your sticky_conn?

Thanks,
Maxim

@renecannao renecannao merged commit f35e853 into sysown:v1.4.0 Dec 29, 2016
@renecannao
Copy link
Contributor

@klinometr : thank you for the pull request. Looks good to me: merged.
I will apply some further tuning to it. For example, this changes a variable in the calling session: I would prefer to set the variable in a Query_Processor_Output structure, and let the calling MySQL_Session decide if the variable need to be changed or not.
This can be useful, for example, if we are inside a transaction and mysql_users.transaction_persistent is set.

renecannao added a commit that referenced this pull request Dec 29, 2016
This is a minor improvement on pull request #825
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants