Add MySQL compression level variable and change default to compression level 3 #4764
Conversation
yashwantsahu20
changed the title
yashwantsahu20
changed the title
| mysql_query(proxysql_admin, query.c_str()); | ||
| } | ||
|
|
||
| int32_t get_compression_level(MYSQL* proxysql_admin) { |
There was a problem hiding this comment.
We have several utility functions in utils.h and utils.cpp for queries that specifically access ProxySQL global_variables , and also functions that help with the boilerplate of queries that return a single value:
show_variableshow_admin_global_variableset_admin_global_variableget_variable_value
And for handling single value queries there is mysql_query_ext_val, which offers boilerplate handling for single values resulset extraction and error handling. Since we have so many, better reuse them, or even reduce them, instead of adding more (like getting rid of the very old ones show_variable and show_admin_global_variable) or even rewriting some in terms of the newer ones if necessary.
| time_proxy_compressed = calculate_query_execution_time(proxysql_compression, query); | ||
| diag("Time taken for query with proxysql with compression level 8: %ld", time_proxy_compressed); | ||
|
|
||
| set_compression_level(proxysql_admin, "3"); |
There was a problem hiding this comment.
Individual value check isn't bad, but since the range is small, a simple for here can be used to verify the whole range, or at least check the range boundaries, checking the failures outside the range. Also, the table runtime_global_variables is never checked, it's possible for a variable to be set, but latter ProxySQL refusing to load it to runtime, since boundaries checks are enforced when LOAD TO RUNTIME is issued to Admin interface. When verifying if any config table is properly changed, it's important to check the table, but also it's runtime_* counterpart.
| MYSQL_QUERY(proxysql, "INSERT INTO sbtest1 (k, c, pad) SELECT FLOOR(RAND() * 10000), REPEAT('a', 120), REPEAT('b', 60) FROM information_schema.tables LIMIT 1000;"); | ||
| } | ||
|
|
||
| std::string query = "SELECT t1.id id1, t1.k k1, t1.c c1, t1.pad pad1, t2.id id2, t2.k k2, t2.c c2, t2.pad pad2 FROM test.sbtest1 t1 JOIN test.sbtest1 t2 LIMIT 90000000"; |
There was a problem hiding this comment.
For performing SELECT queries right after INSERT queries, we should either:
- Disable potential query rules that can lead to query redirection to replicas.
- Insert query annotation in the
SELECTthemselves that target theprimaryhostgroup.
Otherwise slow replication could lead to errors or not performing the testing as it was intended, since all the required data may not be in the target replica server.
| MYSQL_ROW row; | ||
| unsigned long long begin = monotonic_time(); | ||
| unsigned long long row_count = 0; | ||
| MYSQL_QUERY(mysql, query.c_str()); |
There was a problem hiding this comment.
Using this macro here can have unintended consequences. When failing to perform a query, this macro will return EXIT_FAILURE (1), so, if for any reason both queries fail to be executed (broken connection for example), the test for checking the execution time:
unsigned long diff = abs(time_proxy - time_proxy_compressed);
int performance_diff = (diff * 100) / time_proxy;
Would still succeed, as it would be checking 1 - 1 = 0, and 0 passes the ok check right now as a value. Error handling should be encoded here in the return value itself, as they collide with real values from the query/computation.
| unsigned long time_proxy = calculate_query_execution_time(proxysql, query); | ||
| diag("Time taken for query with proxysql without compression: %ld", time_proxy); | ||
|
|
||
| unsigned long time_proxy_compressed = calculate_query_execution_time(proxysql_compression, query); |
There was a problem hiding this comment.
As a sanity check, would be nice to check that the compressed times are bigger than 0, specially if 0 is used to encoded failure in calculate_query_execution_time, which right now lacks this encoding for failures.
| #include "command_line.h" | ||
| #include "utils.h" | ||
|
|
||
| inline unsigned long long monotonic_time() { |
There was a problem hiding this comment.
I think we have used this function already in several tests files, would be nice to place it in utils.h/utils.cpp. So we avoid keep copying it around.
Default compression level was 6 for MySQL clients and this was making ProxySQL slower.
Following changes has been made:
For detail of issue, #4721