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

Setting engineResources not enabling resource requests/limits to seldon-container-engine sidecar #398

Closed
masroorhasan opened this issue Jan 20, 2019 · 2 comments · Fixed by #400
Assignees

Comments

@masroorhasan
Copy link
Contributor

masroorhasan commented Jan 20, 2019

Following this example for setting engineResources to seldon deployment YAML. It looks like seldon-container-engine sidecar is using unbounded memory resources (update JVM flags?) leading to potentially being a bad neighbor. However, it is fine as long as I can make the container's QoS to be Guaranteed instead of Burstable by setting requests = limits. Here is the YAML:

...
  predictors:
  - annotations:
      predictor_version: v1
    componentSpecs:
    - spec:
        containers:
        - env:
          - name: FLASK_ENV
            value: production
          name: [REDACTED]
          image: [REDACTED]
          resources:
            limits:
              cpu: 500m
              memory: 128Mi
            requests:
              cpu: 500m
              memory: 128Mi
        terminationGracePeriodSeconds: 10
    graph:
      children: []
      endpoint:
        type: GRPC
      name: [REDACTED]
      type: MODEL
    name: [REDACTED]
    replicas: 1
    engineResources:
      limits:
        cpu: 200m
        memory: 256Mi
      requests:
        cpu: 200m
        memory: 256Mi

Describing the pod after the deployment shows no update to seldon-container-engine's resource settings (stays with its default request of 100m cpu):

seldon-container-engine:
    Container ID:   docker://7a47037fb183361a20910f9917e9182652244609650c13102c7134fd768ca511
    Image:          seldonio/engine:0.2.6-SNAPSHOT
    Image ID:       docker-pullable://seldonio/engine@sha256:f09c50d4766af29c3d7b7dfff1235e7b1ac708d0fcc67d083d87399649fed0de
    Ports:          8000/TCP, 8082/TCP, 9090/TCP
    State:          Running
      Started:      Sun, 20 Jan 2019 10:47:01 -0800
    Ready:          True
    Restart Count:  0
    Requests:
      cpu:      100m
    Liveness:   http-get http://:admin/ready delay=20s timeout=2s period=5s #success=1 #failure=3
    Readiness:  http-get http://:admin/ready delay=20s timeout=2s period=1s #success=1 #failure=1
    Environment:
...

However, as the chart below shows, both memory and cpu are unbounded:
image

@ukclivecox ukclivecox self-assigned this Jan 20, 2019
@ukclivecox
Copy link
Contributor

EngineResources is deprecated. Though its use should work but I think there is a bug in the move to use svcOrchSpec.

Can you try using svcOrchSpec

optional k8s.io.api.core.v1.ResourceRequirements engineResources = 6 [deprecated=true]; // Optional set of resources for the Seldon engine which is added to each Predictor graph to manage the request/response flow
map<string,string> labels = 7; // labels to be attached to entry deplyment for this predictor
optional SvcOrchSpec svcOrchSpec = 8; // Service Orchestrator configuration
}
message SvcOrchSpec {
optional k8s.io.api.core.v1.ResourceRequirements resources = 1;
repeated k8s.io.api.core.v1.EnvVar env = 2;
}

The bug is in:

// Add engine resources if specified (deprecated - will be removed)
if (predictorDef.hasEngineResources())
cBuilder.setResources(predictorDef.getEngineResources());
if (predictorDef.hasSvcOrchSpec())
{
if (predictorDef.getSvcOrchSpec().hasResources())
cBuilder.setResources(predictorDef.getSvcOrchSpec().getResources());
if (predictorDef.getSvcOrchSpec().getEnvCount() > 0)
cBuilder.addAllEnv(predictorDef.getSvcOrchSpec().getEnvList());
}
else {// set default resource requests for cpu
final String DEFAULT_ENGINE_CPU_REQUEST = "0.1";
cBuilder.setResources(V1.ResourceRequirements.newBuilder().putRequests("cpu", Quantity.newBuilder().setString(DEFAULT_ENGINE_CPU_REQUEST).build()));
}

The engineResources are presently always overridden.

On a side note I think there will need some work to ensure Java respects the container env, but for now you can set JAVA_OPTS as described here as well?

@masroorhasan
Copy link
Contributor Author

Thanks for the prompt response @cliveseldon!

I was following this doc, which is missing both the deprecated=true tag and svcOrchSpec component.

Bummer about the bug but with engineResources deprecation, this feels like a documentation issue. It looks like the proto in docs is a previous revision and could do with being in sync with latest definition. I could put up a quick PR to update it unless there are issues tracking overall documentation audit/update separately?

Also thanks for the JAVA_OPTS annotations pointer, that is very helpful. My original query in the issue seems to be resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants