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

Optimize logbook SQL query again #12881

Merged
merged 1 commit into from Mar 3, 2018

Conversation

Projects
None yet
3 participants
@amelchio
Copy link
Contributor

commented Mar 3, 2018

Description:

This brings back #12608. It was reverted in #12762 because of a report that it slowed the logbook down to be unusable. I have worked with @jjlawren to analyze this issue and it is fully fixed by the index that #12825 proposes.

Also note that reverting the optimization made the logbook similarly unusable with other workloads.

I believe the index and this optimization in tandem will avoid the pathological slowdown in both scenarios (I'm afraid to say "all scenarios" because databases are strange and we support so many of them).

The last line of the patch compares a field with NULL to include rows with no match in the outer join. I have changed this field from last_updated to state_id to make it more clear that this is about missing rows and not the value of some particular field.

Checklist:

  • The code change is tested and works locally.

If the code does not interact with devices:

  • Local tests with tox run successfully.
  • Tests have been added to verify that the new code works.
@balloob

balloob approved these changes Mar 3, 2018

@balloob balloob merged commit 54f8f12 into home-assistant:dev Mar 3, 2018

5 checks passed

WIP ready for review
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.0002%) to 94.079%
Details
hound No violations found. Woof!

@amelchio amelchio referenced this pull request Mar 4, 2018

Merged

Revert optimized logbook SQL #12762

2 of 3 tasks complete

@balloob balloob referenced this pull request Mar 9, 2018

Merged

0.65 #12995

@home-assistant home-assistant locked and limited conversation to collaborators Jul 26, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.