Page MenuHomePhabricator

Change ORES rules to send all events to new "/precache" endpoint
Closed, ResolvedPublic

Description

Currently ChangeProp has a bunch of rules about which change event should be precached in ORES. Change this so all potentially relevant events are sent to /precache endpoint.

This work directly depends on getting T148714: Create generalized "precache" endpoint for ORES done and deployed.

If this deployment changes the metric names for ORES, ping @Halfak and @Ladsgroup to check on the grafana graphs after deployment.

Event Timeline

Halfak created this task.Feb 17 2017, 6:02 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 17 2017, 6:02 PM
Pchelolo moved this task from Backlog to watching on the Services board.Feb 17 2017, 7:08 PM
Pchelolo edited projects, added Services (watching); removed Services.
Halfak triaged this task as Normal priority.Feb 23 2017, 3:32 PM
Halfak moved this task from Untriaged to New development on the Scoring-platform-team board.
Pchelolo claimed this task.Feb 24 2017, 8:15 PM
Pchelolo edited projects, added Services (blocked); removed Services (watching).
Halfak updated the task description. (Show Details)Apr 18 2017, 3:00 PM
Halfak added a subscriber: Ladsgroup.

@Pchelolo: Hey, you have been unblocked for a very long time, do you want me to pick this up if you don't have time?

@Ladsgroup Oh indeed we've dropped the ball on this. I'll prepare a PR over the next couple of days

@Ladsgroup Actually, I don't think it works. Trying to POST with curl to ORES I get 500 and the following stack trace from ORES

2017-12-05 01:54:08,352 ERROR:ores.wsgi.server -- Exception on /v3/precache [POST]
Traceback (most recent call last):
  File "/srv/deployment/ores/venv/lib/python3.4/site-packages/flask/app.py", line 1817, in wsgi_app
    response = self.full_dispatch_request()
  File "/srv/deployment/ores/venv/lib/python3.4/site-packages/flask/app.py", line 1477, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/srv/deployment/ores/venv/lib/python3.4/site-packages/flask/app.py", line 1381, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "/srv/deployment/ores/venv/lib/python3.4/site-packages/flask/_compat.py", line 33, in reraise
    raise value
  File "/srv/deployment/ores/venv/lib/python3.4/site-packages/flask/app.py", line 1475, in full_dispatch_request
    rv = self.dispatch_request()
  File "/srv/deployment/ores/venv/lib/python3.4/site-packages/flask/app.py", line 1461, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "./ores/wsgi/preprocessors.py", line 27, in nocache_route
    response = make_response(route(*args, **kwargs))
  File "./ores/wsgi/preprocessors.py", line 14, in minifiable_route
    response = route(*args, **kwargs)
  File "./ores/wsgi/routes/v3/precache.py", line 33, in precache_v3
    return scores.process_score_request(score_request)
AttributeError: 'module' object has no attribute 'process_score_request'
Halfak added a comment.Dec 5 2017, 3:15 PM

Woops! I see the problem. I'm looking into it.

awight added a subscriber: awight.Feb 15 2018, 8:22 PM

The above PR has been merged.

Change 411087 had a related patch set uploaded (by Awight; owner: Awight):
[mediawiki/services/change-propagation/deploy@master] Simplify ORES precaching by using the new endpoint

https://gerrit.wikimedia.org/r/411087

The config patch above smoke tests correctly under mw-vagrant.

Pchelolo added a comment.EditedFeb 16 2018, 12:59 PM

The config patch above smoke tests correctly under mw-vagrant.

Replied in gerrit.

I don't see these rules in Vagrant configuration, I think we should add them for consistency, what do you think?

@Pchelolo

I don't see these rules in Vagrant configuration, I think we should add them for consistency, what do you think?

Thanks for the response! I agree, but pending our discussion in T187343 because the ORES service is an optional role. Is it okay if these changes are independent, so we don't block the production task? Here's a WIP patch so we don't drop the thread, https://gerrit.wikimedia.org/r/412918

Change 411087 merged by Ppchelko:
[mediawiki/services/change-propagation/deploy@master] Simplify ORES precaching by using the new endpoint

https://gerrit.wikimedia.org/r/411087

Mentioned in SAL (#wikimedia-operations) [2018-02-21T16:37:53Z] <ppchelko@tin> Started deploy [changeprop/deploy@1be63aa]: Simplify ORES precaching by using the new endpoint T158437

Mentioned in SAL (#wikimedia-operations) [2018-02-21T16:39:26Z] <ppchelko@tin> Finished deploy [changeprop/deploy@1be63aa]: Simplify ORES precaching by using the new endpoint T158437 (duration: 01m 33s)

Pchelolo closed this task as Resolved.Feb 21 2018, 4:43 PM
Pchelolo edited projects, added Services (done); removed Patch-For-Review, Services (doing).

The change has been deployed and seems to work fine. Resolving.

Change 413200 had a related patch set uploaded (by Awight; owner: Awight):
[mediawiki/services/change-propagation/deploy@master] Should use the POST method

https://gerrit.wikimedia.org/r/413200

awight reopened this task as Open.Feb 21 2018, 5:09 PM

This was failing quietly,

[2018-02-21T17:04:21] [pid: 25305] 10.2.2.10 (-) {34 vars in 674 bytes} [Wed Feb 21 17:04:21 2018] GET /v3/precache => generated 178 bytes in 1 msecs (HTTP/1.1 405) 4 headers in 135 bytes (1 switches on core 0) user agent "ChangePropagation/WMF"

Should have been using the POST method, we'll try this one-line patch. Reopening until we can confirm working.

Change 413200 merged by Ppchelko:
[mediawiki/services/change-propagation/deploy@master] Should use the POST method

https://gerrit.wikimedia.org/r/413200

Mentioned in SAL (#wikimedia-operations) [2018-02-21T17:11:22Z] <ppchelko@tin> Started deploy [changeprop/deploy@e9a6bb0]: Use post for ORES precache rules T158437

Mentioned in SAL (#wikimedia-operations) [2018-02-21T17:12:45Z] <ppchelko@tin> Finished deploy [changeprop/deploy@e9a6bb0]: Use post for ORES precache rules T158437 (duration: 01m 23s)

Pchelolo closed this task as Resolved.Feb 21 2018, 6:26 PM
Pchelolo removed a project: Patch-For-Review.

Ok, after the second deploy everything seems to be fine. Resolving. Again.