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

Remove one index from log_visit that is actually not needed. #7271

Closed
wants to merge 3 commits into from

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Feb 23, 2015

Noticed the index is quite big for log_visit table so had a look and noticed there is one index not needed when we move config_id to the end of the existing index_idsite_datetime index.

We had a quick look and it seems that we query config_id always in combination with visit_last_action_time so should be all good. Probably in the past this was not the case and therefore this index was needed.

refs #6953 this does not affect log_link_visit_action so should be ok?

When moving config_id to the end of the existing other index
@tsteur tsteur added this to the Piwik 2.12.0 milestone Feb 23, 2015
@tsteur tsteur added the Needs Review PRs that need a code review label Feb 23, 2015
@mattab
Copy link
Member

mattab commented Feb 23, 2015

Before we merge this PR we must decide if #6953 is still valid or not. Maybe we need to wait for 2.13.0

(edit March 11th - correct issue number)

@mattab mattab modified the milestones: Piwik 2.12.0, Piwik 2.13.0 Feb 24, 2015
This could otherwise lead to confusion and problems IE if the update runs again etc.
This could otherwise lead to confusion and problems IE if the update runs again etc.
@mnapoli
Copy link
Contributor

mnapoli commented Mar 10, 2015

FYI #6953 is also about log_visit

@tsteur
Copy link
Member Author

tsteur commented Mar 10, 2015

It was changed a few days ago ;) The milestone is already set to 2.13

@tsteur tsteur added the c: Performance For when we could improve the performance / speed of Matomo. label Mar 19, 2015
@tsteur
Copy link
Member Author

tsteur commented Mar 19, 2015

refs #6759

@mattab mattab modified the milestones: Short term, Piwik 2.13.0 Mar 31, 2015
@mattab
Copy link
Member

mattab commented May 27, 2015

Moving to 3.0.0

@mattab mattab modified the milestones: 3.0.0, Short term May 27, 2015
@tsteur
Copy link
Member Author

tsteur commented Jul 28, 2015

Can we merge this one in Piwik 3.0 branch already? If so, I'd create a new PR.

@@ -153,8 +153,7 @@ public function getTablesCreateSql()
config_id BINARY(8) NOT NULL,
location_ip VARBINARY(16) NOT NULL,
PRIMARY KEY(idvisit),
INDEX index_idsite_config_datetime (idsite, config_id, visit_last_action_time),
INDEX index_idsite_datetime (idsite, visit_last_action_time),
INDEX index_idsite_configid_datetime (idsite, visit_last_action_time, config_id),
Copy link
Member

Choose a reason for hiding this comment

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

let's rename the INDEX index_idsite_datetime_configid to follow index field order

@mattab mattab modified the milestones: 3.0.0-b1, 3.0.0 Jul 30, 2015
@mattab
Copy link
Member

mattab commented Jul 30, 2015

Yes we can issue DB schema change again on Piwik 3.0.0 branch. #6953 was closed

@tsteur
Copy link
Member Author

tsteur commented Jul 30, 2015

Will issue a new PR

@tsteur tsteur closed this Jul 30, 2015
@tsteur tsteur deleted the duplicated_logvisit_index branch July 30, 2015 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Performance For when we could improve the performance / speed of Matomo. Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants