Pkp-lib: Port article usage stats to master

Created on 7 May 2018  Â·  25Comments  Â·  Source: pkp/pkp-lib

This has been implemented for 3.1.2 and needs to be ported to master.

PRs for master branch:
https://github.com/pkp/ojs/pull/2381
https://github.com/pkp/omp/pull/670
https://github.com/pkp/pkp-lib/pull/4742
https://github.com/pkp/ui-library/pull/30

The original issue message is below:

Specification to deliver visualization data in the OJS/OMP framework. The goal here is to develop the foundation by which to provide reports as described in #3366, #3367, #3368, and #3369 (eventually). Data will come from the Usage Stats framework via API. Todo items:

  • [ ] Generalized Table Component
  • [ ] Generalized Table Filtering
  • [ ] Generalized Table Sorting
  • [ ] Stats Delivery via API
Enhancement Hosting Major Feature Sponsored Development

All 25 comments

Some quick notes on the actual filtering/sorting that is required for this round, before I lose them.

Filter by list:

  • Section

Filter by autosuggest:

  • Article ID
  • Issue ID
  • Author (not in current dev round, or by user ID only)
  • Context

Filter by special control:

  • Date range

Actions:

  • Export to CSV

@jmacgreg in my notes, I am missing any specifications for sorting. Were sorting options part of this round of work and do you know what was needed to sort by?

Hi Nate, sorting can be left out of this round. Likewise I think filtering by contributor instead of user ID for now.

On May 9, 2018, at 6:31 AM, Nate Wright notifications@github.com wrote:

Some quick notes on the actual filtering/sorting that is required for this round, before I lose them.

Filter by list:

Section
Filter by autosuggest:

Article ID
Issue ID
Author (not in current dev round, or by user ID only)
Context
Filter by special control:

Date range
Actions:

Export to CSV
@jmacgreg in my notes, I am missing any specifications for sorting. Were sorting options part of this round of work and do you know what was needed to sort by?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@NateWr, this one is a high priority once the settings form stuff is merged, especially if it can be back-ported easily into ojs-stable-3_1_1. As far as I know back-porting shouldn't be major -- can you confirm?

(Rescheduled tentatively so I don't forget -- not confirmed!)

I wasn't aware that this would require backporting. My plan has been to use the API/Service structure to deliver stats, which would build on the work that's going into 3.2. I'd also probably use some of the form field components. So I don't think this one would be an easy backport, but if it needs to be then we can talk about what that entails.

@bozana is doing some of the work on the API side.

@NateWr, we hadn't initially considered back-porting, it only came up recently because of the delay in getting OJS 3.2 out there. @jmacgreg, FYI, I think this one's going to need to wait for 3.2. Is that OK?

@ajnyga, could you please test the PRs with your large/big data installation? Let me then know how is the performance. Currently there is no caching, but then we could maybe think of a possible solution...
Thanks a lot!!!

Hi, thanks!

I will try during the weekend. Saw a screenshot already, looks great!

@ajnyga tested this out on his massive metrics table and identified some key issues for us to work on. I'm going to break it down into two posts. First, on performance. Some quick stats:

  • ~50 seconds to load the last 90 days
  • 54 seconds for all dates (going back to May 2014)
  • 58 seconds when using a search phrase
  • 1.82 seconds for a single submission (/api/v1/stats/publishedSubmissions/{id})
  • 1.81 seconds for a single submission when he cut PKPStatsService::getTimeSegments out of the request.
  • tens of thousands of subqueries for a request for the last 12 months (according to phpmyadmin)

@ajnyga is confident(see comment) that these times would be improved with an index on the day/month columns in the table. More discussion of that here: https://github.com/pkp/pkp-lib/issues/4130

I also feel that we're trying to deliver too much in one request. We should probably remove the timeSegments from each article returned in the collection endpoint. We're also delivering some data, like issues and galleys, that we don't need for each article. We may even want to separate articles from totals, though we may not get that in time for 3.1.2.

I also think the number of queries seems really high. I will try to do some investigating here to see if there are areas where we can cut down on requests.

Suggested steps for now:

  • [ ] Remove timeSegments from article records in /stats/publishedSubmissions and only return them for /stats/publishedSubmissions/{id}.
  • [x] Currently we use the default summary properties from SubmissionService to get info about the submission. Let's reduce that set so we don't require extra db lookups.
  • [ ] Cache the total abstract views for each context going back the last 90 days.
  • [ ] Adjust the UI to lazy load the article information. If the graph is cached, the initial page load should not take too long.
  • [ ] Make a decision about the day/month column indexes.

Suggested things to investigate:

  • [ ] Measure the impact of pulling all stats records and then slicing the array by trying out using multiple queries or other strategies for getting a max count of possible records.
  • [ ] Measure the impact of timeSegments vs totals to see if these should be split out to different API endpoints.
  • [ ] Measure the impact of splitting out article results vs total to see if these should be split out to different API endpoints.

@ajnyga also identified some bugs and things to fix.

  • [x] "a minor issue is that when you have older statistics data, it is saved without any specific date. For example in our case OJS2 was installed already back in 2007 and the old stats were migrated during some OJS2 upgrade so that all old data is saved in a single line per submission. However, when that is now visualized, all that old data is shown with timestamp 1970. 2008-2014 data is shown with timestamp 1970-01-01":

screen shot 2019-02-04 at 19 44 07

  • [x] When on the stats page, articles from all contexts were shown, even though the stats page should be limited to one context.

  • [x] Filters with long names don't wrap.
    screen shot 2019-02-04 at 22 10 23

@ajnyga is confident that these times would be improved with an index on the day/month columns in the table. More discussion of that here: #4130

Actually not that confident anymore. Seem that the queries that touch metrics table are fairly fast already. It is probably the amount of subqueries that slow down the whole thing when they are applied to 30 submissions.

Example with our 800 mb metrics table:
SELECT month, assoc_type, SUM(metric) AS metric FROM metrics WHERE assoc_type IN (1048585, 515) AND metric_type = 'ojs::counter' AND day BETWEEN '20180101' AND '20190101' GROUP BY month, assoc_type ORDER BY month DESC takes around 4 seconds (query whole site metrics)

SELECT month, assoc_type, SUM(metric) AS metric FROM metrics WHERE context_id = '37' AND assoc_type IN (1048585, 515) AND metric_type = 'ojs::counter' AND day BETWEEN '20180101' AND '20190120' GROUP BY month, assoc_type ORDER BY month DESCtakes 1.8 seconds (query context)

SELECT month, assoc_type, SUM(metric) AS metric FROM metrics WHERE context_id = '37' AND assoc_type IN (1048585, 515) AND metric_type = 'ojs::counter' AND day BETWEEN '20180101' AND '20190120' AND submission_id = '64945' GROUP BY month, assoc_type ORDER BY month DESC takes 0.006 seconds (query specific sumbission)

With @ajnyga's help, we tracked down the performance issues to a missed cache for issue lookups, which led to an extraordinary number of sql queries (80k+) for sites with a lot of issues across all contexts.

We've now got a request to /stats/publishedSubmissions on his large metrics table down to 6.85 seconds. I'm comfortable with this for 3.1.2, though we can continue to improve the speed in the UI for future versions.

This has been merged to stable. I've renamed the issue and assigned it to 3.2 for the port to master.

@asmecher I've ported the published submission usage stats page to master, with some changes based on lessons learned with our first go-around. I also added tests for the stats page, which was a good learning experience for me.

I'm getting some persistent test failures in OJS for postgres, and the test error log only goes back 200 lines, so I don't think I can see what's happening. I tried to get postgres up and running locally to work this out, but although I'm able to connect to the database from the command line, I can't seem to get the database connection right in OJS. Would you be able to look into the postgres test failure here?

https://travis-ci.org/pkp/ojs/jobs/530681298#L2914

I've added a step to the submission to publish it in the current issue, and then check that it is appearing in the issue table of contents on the frontend. The line that's failing is the line that would find it in the table of contents, so probably something is happening where the issue assignment is failing in postgres.

Otherwise, I think we're ready to go with this. The main changes for master are:

  • Now using "publication" terminology across OJS/OMP, so the endpoint is /stats/publications.
  • Split the requests for a timeline (eg - graph) and totals into two separate endpoints, and we lazy-load the publications table to make the stats page a bit quicker.
  • Refactored the StatsService methods to better reflect how metrics work, rather than try to force it into the model of other service classes that interact with entities.

PRs:
https://github.com/pkp/ojs/pull/2381
https://github.com/pkp/omp/pull/670
https://github.com/pkp/pkp-lib/pull/4742
https://github.com/pkp/ui-library/pull/30

@NateWr, looking back through the logs for "errors" (typically the last occurrence will come from PHPUnit) I get...

There was 1 error:

1) JmwandengaSubmissionTest::testSubmission
Facebook\WebDriver\Exception\NoSuchElementException: no such element: Unable to locate element: {"method":"xpath","selector":"//a[contains(text(),concat('Signalling Theory Dividends: A Review Of The Literature And Empirical Evidence',''))]"}
  (Session info: chrome=65.0.3325.181)
  (Driver info: chromedriver=2.35 (0),platform=Linux 4.4.0-101-generic x86_64)

/home/travis/build/pkp/ojs/lib/pkp/lib/vendor/facebook/webdriver/lib/Exception/WebDriverException.php:102
/home/travis/build/pkp/ojs/lib/pkp/lib/vendor/facebook/webdriver/lib/Remote/HttpCommandExecutor.php:326
/home/travis/build/pkp/ojs/lib/pkp/lib/vendor/facebook/webdriver/lib/Remote/RemoteWebDriver.php:547
/home/travis/build/pkp/ojs/lib/pkp/lib/vendor/facebook/webdriver/lib/Remote/RemoteWebDriver.php:187
/home/travis/build/pkp/ojs/lib/pkp/lib/vendor/facebook/webdriver/lib/WebDriverExpectedCondition.php:151
/home/travis/build/pkp/ojs/lib/pkp/lib/vendor/facebook/webdriver/lib/WebDriverWait.php:67
/home/travis/build/pkp/ojs/lib/pkp/tests/WebTestCase.inc.php:415
/home/travis/build/pkp/ojs/tests/ContentBaseTestCase.inc.php:75
/home/travis/build/pkp/ojs/tests/data/60-content/JmwandengaSubmissionTest.php:62

This looks to me like a race condition, and if so, if you're getting "repeating-ish" failures, you might see it vary from submission to submission on repetitions. It might be that PostgreSQL is more susceptible for some reason. I'll restart the test and see if it wanders.

Newp, looks like it's stuck firmly on the one submission. Will investigate locally.

Looks like there was a missing group-by clause that PostgreSQL was getting fussy about. Try cherry-picking https://github.com/asmecher/ojs/commit/d282469a18c57015908e3a4f2c3af9c5e2e2e808 in -- I'm running tests on this now at https://travis-ci.org/asmecher/ojs/builds/532479579.

Whoops, this was already filed and fixed at https://github.com/pkp/pkp-lib/issues/4097 but never merged. I've gone ahead and merged that. Just fetching/rebasing your OJS repo should now fix the tests.

Thanks @asmecher! Will do.

@asmecher I'm getting postgres failures in the same place after rebasing. Is it possible that it's being caused by this issue?

https://github.com/pkp/pkp-lib/issues/4599
https://github.com/pkp/pkp-lib/pull/4608

Hmm, I don't think so, but it's possible. I'll re-run the tests here locally.

Two issues were flagged as current per the tests and I guess each DBMS picks a different favourite. This should fix it (already merged to master and stable-3_1_2): https://github.com/pkp/pkp-lib/issues/4765

Merged to master (finally!)

Was this page helpful?
0 / 5 - 0 ratings