Page MenuHomePhabricator

Add support for REDIS to ores-legacy
Closed, DeclinedPublic

Description

As described in T330414#8871324 we want to add support for the ORES Redis score cache to ores-legacy.

Event Timeline

Found a nice way to materialize a env variable in the ores-legacy pods via the service scaffolding secret support. We just need to add the following bits to our ores-legacy private config (in puppet):

ml-serve-eqiad:
  config:
    private:
      redis_auth: $ores-cache-password

And helmfile will create a secret called ores-legacy-main-secret-config and the following env variable config in the pod:

+             - name: REDIS_AUTH
+               valueFrom:
+                 secretKeyRef:
+                   name: ores-legacy-main-secret-config
+                   key: redis_auth

Then we just need to os.environ.get() in the code and we are done.

Checked the ores cache format:

localhost:6378> get ores:itwiki:goodfaith:133595563:0.5.0
"{\"prediction\": true, \"probability\": {\"false\": 0.02953874448878846, \"true\": 0.9704612555112115}}"

The main question mark that I have is the usage of the ores pool counters, I think it is needed (as key/lock) to avoid multiple workers computing the same things at the same time. Will investigate a little more into it as well.

Change 922561 had a related patch set uploaded (by Elukey; author: Elukey):

[machinelearning/liftwing/inference-services@main] WIP - Add read-only cache support to ores-legacy

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

Change 922809 had a related patch set uploaded (by Elukey; author: Elukey):

[machinelearning/liftwing/inference-services@main] ores-legacy: simplify test in test_liftwing.py

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

Change 922809 merged by jenkins-bot:

[machinelearning/liftwing/inference-services@main] ores-legacy: simplify test in test_liftwing.py

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

Change 922561 abandoned by Elukey:

[machinelearning/liftwing/inference-services@main] WIP - Add read-only cache support to ores-legacy

Reason:

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