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.

Sign up for GitHub

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

Jump to bottom

Fix MDEV-29076. If thd->time_zone_used is not used, do not store time… #2185

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

Fix MDEV-29076. If thd->time_zone_used is not used, do not store time… #2185

wants to merge 1 commit into from

Conversation

wangteng13
Copy link

The Jira issue number for this PR is: MDEV-29076 Dynamically update time_zone caused Query_cache stored repetitive select queries.

Description

For the same "Select *" operation, the Query_cache failed to identify them and stores the redundant queries, if changing the option "time_zone" dynamically. This patch identifies whether the query used time_zone, and reduces the redundant queries when changing option "time_zone". Specially, If thd->time_zone_used is not used, do not store 'time_zone' as Query_cache_query_flags.

@CLAassistant
Copy link

CLAassistant commented Jul 11, 2022 •
edited

CLA assistant check
All committers have signed the CLA.

@grooverdan
Copy link
Member

Query_cache::send_result_to_client needs the same adjustment.

Then you need to apply the test in your MDEV to test that changing the time_zone doesn't invalidate the cache.

@wangteng13
Copy link
Author

Query_cache::send_result_to_client needs the same adjustment.

Then you need to apply the test in your MDEV to test that changing the time_zone doesn't invalidate the cache.

Thanks, I would make a new pull request

@grooverdan
Copy link
Member

No, git commit --amend and git push --force to the same branch will update this pr.

@wangteng13
Copy link
Author

No, git commit --amend and git push --force to the same branch will update this pr.

Thanks! I have update the pr. I also apply a new test for the bug.

@grooverdan
Copy link
Member

You've done really well on your first PR. Thanks. If you didn't discover it already mysql-test/mtr --record {testname} will update the result file.

@grooverdan grooverdan self-assigned this Jul 12, 2022
@grooverdan
Copy link
Member

@wangteng13, I've done a number of updates of your PR:

  • squashed the commit messages
  • fixed the qc_info plugin to handle the NULL value
  • move the test into the existing query_cache

What I'm trying to do in the test it to add a query that is dependent of timezone and show that it will be a second insert when the timezone has changed.

I did check the thd->variables.time_zone pointer does change when the timezone is changed.

Can you help construct a test case around the case of a changing timezone and a query who's result depends on it?

If you want to continue from the changed code here:

git fetch origin
git checkout -b fix-bug-server2 origin/fix-bug-server
# make changes
git push origin HEAD:fix-bug-server

@wangteng13 @grooverdan
MDEV-29076 Query_cache stored identical select queries
2b86cf1 
... multiple time due to time_zone changes, when their result wasn't
dependent on the timezone.

Timezone changes do not affect the result of most queries.
The THD structure has a time_zone_used indicator to indicate that it was
used.

In the query cache we use this indicator to normalize the time_zone flag
of the query cache to a fixed value, NULL, so subsequent queries of a
different time_zone will hit the same cached entry.

To handle NULL as a value, the query_cache_info plugin was adjusted to
allow for the NULL values of the time_zone flag.

Reviewer: Daniel Black
@grooverdan grooverdan added the need feedback Can the contributor please address the questions asked. label Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@grooverdan grooverdan

Labels
need feedback Can the contributor please address the questions asked.
Milestone
No milestone
3 participants
@wangteng13 @CLAassistant @grooverdan