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

[utils][proxysql] Set query_retries_on_failure to 1 explicitly #7238

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

s10
Copy link
Contributor

@s10 s10 commented Oct 14, 2024

Default value is 1, but sometimes we might want more retries instead of 500

This could be done for SELECT queries by configuration:

proxysql:
  query_rules:
    retries_on_select: 3

@s10 s10 marked this pull request as ready for review October 14, 2024 14:21
@fwiesel
Copy link
Member

fwiesel commented Oct 14, 2024

I am not sure, if we can enable that feature without risk.
The scenario I am thinking of is that the (modifying) query actually successfully executed, it just failed to send the information back to the client. (In terms of the two-phase commit protocol, we had an error on the "ack").
My understanding is, that the query will be re-executed, and if the constraints are properly set, we will "simply" have different error (a constraint violation vs connection reset error), but in the worst case, we have now a record twice, which we shouldn't have.

@s10 s10 force-pushed the C5384329/utils-query-retries branch from 765cf01 to 343e7b6 Compare October 14, 2024 15:57
@s10 s10 changed the title [utils][proxysql] Set query_retries_on_failure to 3 [utils][proxysql] Set query_retries_on_failure to 1 explicitly Oct 14, 2024
@s10 s10 self-assigned this Oct 14, 2024
Default value is 1, but sometimes we might want more retries instead of 500

This could be done for SELECT queries by configuration:

proxysql:
  query_rules:
    retries_on_select: 10
@s10 s10 force-pushed the C5384329/utils-query-retries branch from 343e7b6 to 9505dc7 Compare October 16, 2024 14:51
@@ -42,6 +42,7 @@ mysql_variables =
connect_retries_on_failure = {{ default 1000 .global.Values.proxysql.connect_retries_on_failure }}
connect_retries_delay = {{ default 100 .global.Values.proxysql.connect_retries_delay }} {{- /* The default is 1ms, and that means we will run through the retries on failure in no time */}}
connect_timeout_server_max = {{ default 100000 .global.Values.proxysql.connect_timeout_server_max }}
query_retries_on_failure = {{ default 1 .global.Values.proxysql.query_retries_on_failure }} {{- /* The ProxySQL default is 1, and that means queries are being retried once */}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the documentation this is not limited to read only queries and bigger result sets can cause errors. There is also a related issue from 2017 still open. We should check if we can tweak the pxc-db HAProxy setup before we change this setting.

active = 1,
apply = 1,
retries = {{ default 10 .global.Values.proxysql.query_rules.retries_on_select }},
match_digest = "^SELECT",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With SELECT FOR UPDATE this can be a DML command and than the retries can be risky if something has been changed in between, or?

Copy link
Member

@fwiesel fwiesel Oct 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My assumption was, that retries are only done, if it isn't within a transaction. SELECT FOR UPDATE only creates a lock within a transaction.
I've checked the code, and it looks to me, my assumption is correct.

A retry of a single statement from an aborted transaction wouldn't make sense.

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.

3 participants