Add doc for TF Serving #1165

lluunn · 2018-07-10T21:46:59Z

This change is

k8s-ci-robot · 2018-07-10T21:47:00Z

@lluunn: GitHub didn't allow me to request PR reviews from the following users: ynqa.

Note that only kubeflow members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @jlewi
/cc @ynqa

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

lluunn · 2018-07-11T18:41:04Z

/retest

lluunn · 2018-07-11T19:17:38Z

Tests timeout

lluunn · 2018-07-11T19:22:28Z

#1184

lluunn · 2018-07-11T19:22:41Z

/retest

jlewi

Reviewable status: 0 of 1 files reviewed, 3 unresolved discussions (waiting on @lluunn and @jlewi)

docs_dev/tf_serving.md, line 3 at r1 (raw file):

# TF Serving in Kubeflow

Status:

add authors and list yourself

docs_dev/tf_serving.md, line 21 at r1 (raw file):

#### HTTP proxy
The [http proxy](https://github.com/kubeflow/kubeflow/tree/master/components/k8s-model-server/http-proxy)

You should explain why we have this given TF 1.8 supports it. And also mention current thinking about the plans going forward.

docs_dev/tf_serving.md, line 29 at r1 (raw file):

Check out gcr.io/kubeflow-images-public registry for other versions.

#### Istio integration

I'd start a section monitoring and put this in that section. Can you also add information about promtheus integration and liveness/healthness checks?

jlewi

Reviewable status: 0 of 1 files reviewed, 4 unresolved discussions (waiting on @lluunn and @jlewi)

docs_dev/tf_serving.md, line 35 at r1 (raw file):

See [detail](https://github.com/kubeflow/kubeflow/blob/master/components/k8s-model-server/istio-integration.md).

## Usage

Lets also add a section about Seldon. This could be as simple as an open question is how to combine TF Serving with Seldon's model management features . I think there might be some open issues about this.

lluunn

Reviewable status: 0 of 1 files reviewed, 4 unresolved discussions (waiting on @jlewi)

docs_dev/tf_serving.md, line 3 at r1 (raw file):

Previously, jlewi (Jeremy Lewi) wrote…

add authors and list yourself

Done.

docs_dev/tf_serving.md, line 21 at r1 (raw file):

Previously, jlewi (Jeremy Lewi) wrote…

You should explain why we have this given TF 1.8 supports it. And also mention current thinking about the plans going forward.

Done.

docs_dev/tf_serving.md, line 29 at r1 (raw file):

Previously, jlewi (Jeremy Lewi) wrote…

I'd start a section monitoring and put this in that section. Can you also add information about promtheus integration and liveness/healthness checks?

Done.

docs_dev/tf_serving.md, line 35 at r1 (raw file):

Previously, jlewi (Jeremy Lewi) wrote…

Lets also add a section about Seldon. This could be as simple as an open question is how to combine TF Serving with Seldon's model management features . I think there might be some open issues about this.

Done.

lluunn · 2018-07-12T18:51:39Z

Thank you, PTAL @jlewi

jlewi · 2018-07-13T13:01:16Z

docs_dev/tf_serving.md

+
+## SeldonIO
+[SeldonIO](https://github.com/SeldonIO/seldon-core) is a platform for serving ML
+models. It's an open question on how should we integrate TF Serving with


... on how should we -> Its an open question how we should

jlewi · 2018-07-13T13:01:42Z

docs_dev/tf_serving.md

+Seldon to get nice features like routing and model management.
+
+## Next steps
+- [kubeflow/kubeflow#1000](https://github.com/kubeflow/kubeflow/issues/1000): Support logging request and response


How about autoscaling?

There is a benchmark issue below, which should cover it? I added some word

jlewi · 2018-07-13T13:03:42Z

docs_dev/tf_serving.md

+TF 1.8). Going forward, we will still maintain and support this http proxy as
+it has the ability to provide additional features such as:
+- exporting metrics, [kubeflow/kubeflow#1036](https://github.com/kubeflow/kubeflow/issues/1036).
+- request logging. See


Are we planning on doing request logging in the http proxy? Why can't we do that in TF Serving?
Would we have to also proxy gRPC requests in order to get that functionality when gRPC is used?

I think it's easier to make progress if we do it in our http proxy comparing to TF serving.

We can discuss on this thread #1000

Can you add a link to the issue in the doc?

jlewi

Reviewable status: 0 of 1 files reviewed, 4 unresolved discussions (waiting on @jlewi and @lluunn)

docs_dev/tf_serving.md, line 64 at r2 (raw file):

Previously, lluunn (Lun-Kai Hsu) wrote…

There is a benchmark issue below, which should cover it? I added some word

Autoscaling seems different from benchmarking. Do we have an issue about autoscaling? Some of the questions are how do we take advantage of K8s autoscaling? Do we have the right metrics to trigger K8s autoscaling?

docs_dev/tf_serving.md, line 49 at r3 (raw file):

See [detail](https://github.com/kubeflow/kubeflow/blob/master/components/k8s-model-server/istio-integration.md).

We are working on surfacing those metrics and integrating with Prometheus

I thought we were only planning on using Prometheus for application, TF Serving specific metrics?

lluunn

Reviewable status: 0 of 1 files reviewed, 4 unresolved discussions (waiting on @jlewi and @lluunn)

docs_dev/tf_serving.md, line 64 at r2 (raw file):

Previously, jlewi (Jeremy Lewi) wrote…

Autoscaling seems different from benchmarking. Do we have an issue about autoscaling? Some of the questions are how do we take advantage of K8s autoscaling? Do we have the right metrics to trigger K8s autoscaling?

I see. Filed an issue and linked it.

docs_dev/tf_serving.md, line 49 at r3 (raw file):

Previously, jlewi (Jeremy Lewi) wrote…

I thought we were only planning on using Prometheus for application, TF Serving specific metrics?

I think we can do all, then we don't have to rely on Istio?

lluunn · 2018-07-16T16:34:24Z

@jlewi PTAL, thanks!

lluunn · 2018-07-16T17:06:11Z

Error: No module named py.

lluunn · 2018-07-16T17:06:39Z

#1218

lluunn · 2018-07-16T17:17:08Z

/retest

lluunn · 2018-07-16T19:08:49Z

/retest

jlewi

Reviewable status: 0 of 1 files reviewed, 3 unresolved discussions (waiting on @jlewi)

docs_dev/tf_serving.md, line 49 at r3 (raw file):

(https://github.com/kubeflow/kubeflow/issues/1036

Do you mean we can do that in our proxy? Why would we want to build a custom proxy to collect telemtry metrics when this is exactly what ISTIO does?

We'd like to be able to do this for all Kubeflow microservices not just TF Serving.

lluunn · 2018-07-17T22:20:33Z

/retest

lluunn · 2018-07-17T22:50:47Z

/retest

lluunn · 2018-07-18T00:03:43Z

Seems all passed: https://k8s-gubernator.appspot.com/build/kubernetes-jenkins/pr-logs/pull/kubeflow_kubeflow/1165/kubeflow-presubmit/2618/

lluunn · 2018-07-18T00:03:48Z

/retest

jlewi

Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @jlewi)

docs_dev/tf_serving.md, line 49 at r3 (raw file):

Previously, jlewi (Jeremy Lewi) wrote…

(https://github.com/kubeflow/kubeflow/issues/1036
Do you mean we can do that in our proxy? Why would we want to build a custom proxy to collect telemtry metrics when this is exactly what ISTIO does?

We'd like to be able to do this for all Kubeflow microservices not just TF Serving.

I think the text is fine but I think we should continue to discuss whether its better to use ISTIO or not.

jlewi · 2018-07-18T03:04:20Z

/lgtm
/approve

k8s-ci-robot · 2018-07-18T03:04:22Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jlewi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [jlewi]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

* tf serving md * fix * address comments * address review * comment

lluunn added 2 commits July 10, 2018 14:42

tf serving md

35628c8

fix

79a1f16

k8s-ci-robot requested a review from jlewi July 10, 2018 21:46

k8s-ci-robot added the size/M label Jul 10, 2018

jlewi suggested changes Jul 12, 2018

View reviewed changes

address comments

a219d9f

lluunn commented Jul 12, 2018

View reviewed changes

jlewi reviewed Jul 13, 2018

View reviewed changes

address review

e9a8dde

jlewi suggested changes Jul 16, 2018

View reviewed changes

comment

660432b

lluunn commented Jul 16, 2018

View reviewed changes

jlewi suggested changes Jul 17, 2018

View reviewed changes

jlewi suggested changes Jul 18, 2018

View reviewed changes

k8s-ci-robot assigned jlewi Jul 18, 2018

k8s-ci-robot added the lgtm label Jul 18, 2018

k8s-ci-robot added the approved label Jul 18, 2018

k8s-ci-robot merged commit f5801b1 into kubeflow:master Jul 18, 2018

saffaalvi pushed a commit to StatCan/kubeflow that referenced this pull request Feb 11, 2021

Add doc for TF Serving (kubeflow#1165)

5e5ef97

* tf serving md * fix * address comments * address review * comment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add doc for TF Serving #1165

Add doc for TF Serving #1165

lluunn commented Jul 10, 2018 •

edited by jlewi

k8s-ci-robot commented Jul 10, 2018

lluunn commented Jul 11, 2018

lluunn commented Jul 11, 2018

lluunn commented Jul 11, 2018

lluunn commented Jul 11, 2018

jlewi left a comment

jlewi left a comment

lluunn left a comment

lluunn commented Jul 12, 2018

jlewi Jul 13, 2018

lluunn Jul 13, 2018

jlewi Jul 13, 2018

lluunn Jul 13, 2018

jlewi Jul 13, 2018

lluunn Jul 13, 2018

jlewi Jul 16, 2018

lluunn Jul 16, 2018

jlewi left a comment

lluunn left a comment

lluunn commented Jul 16, 2018

lluunn commented Jul 16, 2018

lluunn commented Jul 16, 2018

lluunn commented Jul 16, 2018

lluunn commented Jul 16, 2018

jlewi left a comment

lluunn commented Jul 17, 2018

lluunn commented Jul 17, 2018

lluunn commented Jul 18, 2018

lluunn commented Jul 18, 2018

jlewi left a comment

jlewi commented Jul 18, 2018

k8s-ci-robot commented Jul 18, 2018

Add doc for TF Serving #1165

Add doc for TF Serving #1165

Conversation

lluunn commented Jul 10, 2018 • edited by jlewi

k8s-ci-robot commented Jul 10, 2018

lluunn commented Jul 11, 2018

lluunn commented Jul 11, 2018

lluunn commented Jul 11, 2018

lluunn commented Jul 11, 2018

jlewi left a comment

Choose a reason for hiding this comment

jlewi left a comment

Choose a reason for hiding this comment

lluunn left a comment

Choose a reason for hiding this comment

lluunn commented Jul 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlewi left a comment

Choose a reason for hiding this comment

lluunn left a comment

Choose a reason for hiding this comment

lluunn commented Jul 16, 2018

lluunn commented Jul 16, 2018

lluunn commented Jul 16, 2018

lluunn commented Jul 16, 2018

lluunn commented Jul 16, 2018

jlewi left a comment

Choose a reason for hiding this comment

lluunn commented Jul 17, 2018

lluunn commented Jul 17, 2018

lluunn commented Jul 18, 2018

lluunn commented Jul 18, 2018

jlewi left a comment

Choose a reason for hiding this comment

jlewi commented Jul 18, 2018

k8s-ci-robot commented Jul 18, 2018

lluunn commented Jul 10, 2018 •

edited by jlewi