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

feat: Serving Source #1806

Merged
merged 27 commits into from
Jul 17, 2024
Merged

feat: Serving Source #1806

merged 27 commits into from
Jul 17, 2024

Conversation

yhl25
Copy link
Contributor

@yhl25 yhl25 commented Jul 10, 2024

closes #1813

Copy link

codecov bot commented Jul 10, 2024

Codecov Report

Attention: Patch coverage is 77.41560% with 194 lines in your changes missing coverage. Please review.

Project coverage is 53.58%. Comparing base (d620f1b) to head (abb1de6).

Files Patch % Lines
pkg/sources/jetstream/jetstream.go 59.29% 31 Missing and 15 partials ⚠️
serving/src/config.rs 30.43% 32 Missing ⚠️
serving/src/pipeline.rs 59.45% 30 Missing ⚠️
pkg/isbsvc/jetstream_service.go 0.00% 26 Missing ⚠️
serving/src/app.rs 89.47% 22 Missing ⚠️
pkg/apis/numaflow/v1alpha1/vertex_types.go 74.07% 16 Missing and 5 partials ⚠️
serving/src/app/callback/store/redisstore.rs 45.45% 6 Missing ⚠️
serving/src/app/jetstream_proxy.rs 76.92% 3 Missing ⚠️
pkg/apis/numaflow/v1alpha1/pipeline_types.go 0.00% 2 Missing ⚠️
pkg/sources/source.go 0.00% 2 Missing ⚠️
... and 4 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1806      +/-   ##
==========================================
+ Coverage   52.97%   53.58%   +0.60%     
==========================================
  Files         282      283       +1     
  Lines       27320    27986     +666     
==========================================
+ Hits        14473    14996     +523     
- Misses      11999    12110     +111     
- Partials      848      880      +32     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

vigith and others added 3 commits July 10, 2024 21:01
Signed-off-by: Vigith Maurice <[email protected]>
Signed-off-by: Vigith Maurice <[email protected]>
Signed-off-by: Yashash H L <[email protected]>
pkg/isbsvc/jetstream_service.go Show resolved Hide resolved
pkg/sources/jetstream/jetstream.go Outdated Show resolved Hide resolved
serving/Dockerfile Show resolved Hide resolved
serving/src/app.rs Outdated Show resolved Hide resolved
serving/src/app.rs Outdated Show resolved Hide resolved
serving/src/app.rs Show resolved Hide resolved
serving/src/config.rs Outdated Show resolved Hide resolved
pkg/apis/numaflow/v1alpha1/const.go Outdated Show resolved Hide resolved
examples/15-serving-source-pipeline.yaml Show resolved Hide resolved
Signed-off-by: Yashash H L <[email protected]>
Copy link
Member

@whynowy whynowy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

partial reviewed.

pkg/apis/numaflow/v1alpha1/pipeline_types.go Outdated Show resolved Hide resolved
pkg/apis/numaflow/v1alpha1/serving_source.go Outdated Show resolved Hide resolved
pkg/apis/numaflow/v1alpha1/serving_source.go Outdated Show resolved Hide resolved
pkg/apis/numaflow/v1alpha1/serving_source.go Outdated Show resolved Hide resolved
pkg/apis/numaflow/v1alpha1/serving_source.go Outdated Show resolved Hide resolved
pkg/apis/numaflow/v1alpha1/vertex_types.go Outdated Show resolved Hide resolved
@whynowy
Copy link
Member

whynowy commented Jul 14, 2024

Which one is better?

  1. Serving source orchestrated to be a JetStream source with a sidecar.
  2. Serving source orchestrated to be a JetStream source + a Deployment.

@yhl25
Copy link
Contributor Author

yhl25 commented Jul 14, 2024

Which one is better?

  1. Serving source orchestrated to be a JetStream source with a sidecar.
  2. Serving source orchestrated to be a JetStream source + a Deployment.

I prefer Jetstream source + Deployment because it's not tied to the source, but it will be tricky to manage the deployment, especially the scaling part and also we will have to make changes in the UI to monitor the deployment. For the initial phase, I think the sidecar approach should be good since we don't have to worry about the autoscaling part and also at this point serving is very much tied to the pipeline. But in the future, if we decide to support serving independently we can consider switching to a deployment.

@whynowy
Copy link
Member

whynowy commented Jul 14, 2024

Which one is better?

  1. Serving source orchestrated to be a JetStream source with a sidecar.
  2. Serving source orchestrated to be a JetStream source + a Deployment.

I prefer Jetstream source + Deployment because it's not tied to the source, but it will be tricky to manage the deployment, especially the scaling part and also we will have to make changes in the UI to monitor the deployment. For the initial phase, I think the sidecar approach should be good since we don't have to worry about the autoscaling part and also at this point serving is very much tied to the pipeline. But in the future, if we decide to support serving independently we can consider switching to a deployment.

Scaling wouldn't be a problem, instead it would be better - independent scaling mechanism, but the UI is a problem. Anyways, go ahead with what you think is better for now.

@yhl25
Copy link
Contributor Author

yhl25 commented Jul 15, 2024

Which one is better?

  1. Serving source orchestrated to be a JetStream source with a sidecar.
  2. Serving source orchestrated to be a JetStream source + a Deployment.

I prefer Jetstream source + Deployment because it's not tied to the source, but it will be tricky to manage the deployment, especially the scaling part and also we will have to make changes in the UI to monitor the deployment. For the initial phase, I think the sidecar approach should be good since we don't have to worry about the autoscaling part and also at this point serving is very much tied to the pipeline. But in the future, if we decide to support serving independently we can consider switching to a deployment.

Scaling wouldn't be a problem, instead it would be better - independent scaling mechanism, but the UI is a problem. Anyways, go ahead with what you think is better for now.

That is true, but we should have a way to allow users to control the min and max and manage the deployment. Otherwise, the scaling could be done independently.

@whynowy
Copy link
Member

whynowy commented Jul 15, 2024

Which one is better?

  1. Serving source orchestrated to be a JetStream source with a sidecar.
  1. Serving source orchestrated to be a JetStream source + a Deployment.

I prefer Jetstream source + Deployment because it's not tied to the source, but it will be tricky to manage the deployment, especially the scaling part and also we will have to make changes in the UI to monitor the deployment. For the initial phase, I think the sidecar approach should be good since we don't have to worry about the autoscaling part and also at this point serving is very much tied to the pipeline. But in the future, if we decide to support serving independently we can consider switching to a deployment.

Scaling wouldn't be a problem, instead it would be better - independent scaling mechanism, but the UI is a problem. Anyways, go ahead with what you think is better for now.

That is true, but we should have a way to allow users to control the min and max and manage the deployment. Otherwise, the scaling could be done independently.

This is an http service, so HPA....

@yhl25
Copy link
Contributor Author

yhl25 commented Jul 15, 2024

Which one is better?

  1. Serving source orchestrated to be a JetStream source with a sidecar.
  1. Serving source orchestrated to be a JetStream source + a Deployment.

I prefer Jetstream source + Deployment because it's not tied to the source, but it will be tricky to manage the deployment, especially the scaling part and also we will have to make changes in the UI to monitor the deployment. For the initial phase, I think the sidecar approach should be good since we don't have to worry about the autoscaling part and also at this point serving is very much tied to the pipeline. But in the future, if we decide to support serving independently we can consider switching to a deployment.

Scaling wouldn't be a problem, instead it would be better - independent scaling mechanism, but the UI is a problem. Anyways, go ahead with what you think is better for now.

That is true, but we should have a way to allow users to control the min and max and manage the deployment. Otherwise, the scaling could be done independently.

This is an http service, so HPA....

yes, we should allow users to control the min and max right? If it is going to be a separate deployment we can't put it inside the pipeline spec WDYT?

pkg/apis/numaflow/v1alpha1/vertex_types.go Show resolved Hide resolved
pkg/reconciler/pipeline/controller.go Outdated Show resolved Hide resolved
pkg/isbsvc/jetstream_service.go Show resolved Hide resolved
pkg/apis/numaflow/v1alpha1/pipeline_types.go Show resolved Hide resolved
pkg/isbsvc/jetstream_service.go Show resolved Hide resolved
pkg/isbsvc/jetstream_service.go Show resolved Hide resolved
serving/src/app/callback/store.rs Outdated Show resolved Hide resolved
Signed-off-by: Yashash H L <[email protected]>