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

table invalidation condition enhanced #213

Merged
merged 1 commit into from
Feb 24, 2022

Conversation

JanoValaska
Copy link
Contributor

Description

During our recent efforts to increase speed of our system we noticed that in some situations django-cachalot performs unnecessary table invalidations after SELECT SQL statements.

Problematic situation

Many models in our system have DateTimeFields which track when model instance was created and updated. We name these fields "created_at" and "updated_at". When retrieving objects from the database, Django's ORM performs SELECT SQL statement behind the scenes - e.g. for our models with "created_at" and "updated_at" DateTimeFields the statement will look like this:

'SELECT some_field_name, some_other_field_name, created_at, updated_at from some_table_name;'

This SELECT SQL statement should not invalidate "some_table_name" table, because SELECT SQL statement does not change data in "some_table_name" table, but combination of "in" operator in this block of code and our field names "created_at" and "updated" trigger unnecessary "some_table_name" table invalidation.

'create' in 'SELECT some_field_name, some_other_field_name, created_at, updated_at from some_table_name;' # is True
'update' in 'SELECT some_field_name, some_other_field_name, created_at, updated_at from some_table_name;' # is True

Proposed solution

To get rid of this issue I propose to use "startswith" string method instead of "in" operator. The problematic block of code would look like this after the fix:

                    if sql.startswith('update') or sql.startswith('insert') or sql.startswith('delete') \
                            or sql.startswith('alter') or sql.startswith('create') \
                            or sql.startswith('drop'):

Rationale

These unnecessary table invalidations make table with field name that contains any of these SQL keywords('update', 'insert', 'delete', 'alter', 'create', 'drop') effectively uncacheable, because table is invalidated by all types of SQL statements.

if 'update' in sql or 'insert' in sql or 'delete' in sql \
or 'alter' in sql or 'create' in sql \
or 'drop' in sql:
if SQL_DATA_CHANGE_RE.search(sql):
Copy link
Member

Choose a reason for hiding this comment

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

One could argue that this is missing a is not None, but what you wrote is logically equivalent and marginally faster.

Capture d’écran 2022-02-24 à 14 34 15

@coveralls
Copy link

coveralls commented Feb 24, 2022

Pull Request Test Coverage Report for Build 1893193419

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.008%) to 97.198%

Totals Coverage Status
Change from base Build 1700021054: 0.008%
Covered Lines: 659
Relevant Lines: 678

💛 - Coveralls

@Andrew-Chen-Wang Andrew-Chen-Wang linked an issue Feb 24, 2022 that may be closed by this pull request
@Andrew-Chen-Wang
Copy link
Collaborator

Thank you for the PR @JanoValaska ! And thank you for catching this @BertrandBordage

@Andrew-Chen-Wang Andrew-Chen-Wang merged commit c8791fe into noripyt:master Feb 24, 2022
samwoodson pushed a commit to ccn-2m/django-cachalot that referenced this pull request Mar 7, 2022
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.

Reduce number of unnecessary table invalidations
4 participants