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

Log segments that are being archived #7723

Merged
merged 6 commits into from Apr 22, 2015
Merged

Log segments that are being archived #7723

merged 6 commits into from Apr 22, 2015

Conversation

mnapoli
Copy link
Contributor

@mnapoli mnapoli commented Apr 21, 2015

Fixes #7536

Example:

INFO CoreConsole[2015-04-16 23:22:25] Starting Piwik reports archiving...
INFO CoreConsole[2015-04-16 23:22:25] Will pre-process for website id = 1, day period, 2 segments
INFO CoreConsole[2015-04-16 23:22:25] - pre-processing all visits
INFO CoreConsole[2015-04-16 23:22:25] - pre-processing segment entryPageTitle=@Web%20Analytics
INFO CoreConsole[2015-04-16 23:22:26] - pre-processing segment pageTitle=@Piwik
INFO CoreConsole[2015-04-16 23:22:26] Archived website id = 1, period = day, 6 visits in last last2 days, 0 visits today, Time elapsed: 1.380s
INFO CoreConsole[2015-04-16 23:22:26] Will pre-process for website id = 1, week period, 2 segments
INFO CoreConsole[2015-04-16 23:22:26] - pre-processing all visits
INFO CoreConsole[2015-04-16 23:22:26] - pre-processing segment entryPageTitle=@Web%20Analytics
INFO CoreConsole[2015-04-16 23:22:26] - pre-processing segment pageTitle=@Piwik
INFO CoreConsole[2015-04-16 23:22:36] Archived website id = 1, period = week, 459 visits in last last3 weeks, 301 visits this week, Time elapsed: 9.440s

@mnapoli mnapoli added the Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. label Apr 21, 2015
@mnapoli mnapoli added this to the Piwik 2.13.0 milestone Apr 21, 2015
@mnapoli mnapoli added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Apr 21, 2015
@diosmosis
Copy link
Member

Can you add a unit/integration test? Maybe to CliMultiTest.

@diosmosis
Copy link
Member

Also could add an assertion for the output in ArchiveCronTest, not sure how important that is.

@mattab
Copy link
Member

mattab commented Apr 21, 2015

a subtle but big improvement to humans consuming the log!

Here is suggestion to improve output when there are 200 segments:

  • display the progress of segments eg. from:

INFO CoreConsole[2015-04-16 23:22:26] - pre-processing segment pageTitle=@Piwik

to

INFO CoreConsole[2015-04-16 23:22:26] - pre-processing segment pageTitle=@Piwik [2/200 segments done] (if there was 200 segments)

@mattab
Copy link
Member

mattab commented Apr 21, 2015

Also could add an assertion for the output in ArchiveCronTest, not sure how important that is.

+1 testing for the core:archive output has a lot of value, regressing these nice logging messages could have large impact on piwik admins

mnapoli added a commit that referenced this pull request Apr 21, 2015
@mnapoli
Copy link
Contributor Author

mnapoli commented Apr 21, 2015

I rebased on master and pushed tests for CliMulti + the cron archive output. FYI the cron archive output test takes ~ a minute (I created 1 website + 1 segment, I didn't add any visits).

@mnapoli mnapoli added the Needs Review PRs that need a code review label Apr 21, 2015
@mattab
Copy link
Member

mattab commented Apr 22, 2015

+1 - it's a real good feeling to see new tests for core:archive output :)

@diosmosis
Copy link
Member

Looks good to me, I think the tests can be sped up later so if the build passes, I will merge.

@mattab
Copy link
Member

mattab commented Apr 22, 2015

Btw the test could be made faster by appending to the command --force-periods=day which will cause archiving to only process the day (should make it maybe 3-4 times faster) - feel free to make the change after the merge if you want...

mattab pushed a commit that referenced this pull request Apr 22, 2015
Log segments that are being archived
@mattab mattab merged commit 1992f33 into master Apr 22, 2015
@mnapoli mnapoli deleted the archiver-log-segments branch April 22, 2015 04:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants