From 1e1abb197de50daa4ed1976bbf29aceab38ca968 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Wed, 11 Mar 2020 14:24:39 +0800 Subject: [PATCH 01/66] Added protobuf definitions for a Job Management API --- protos/feast/core/CoreService.proto | 53 ++++++++++++++++- protos/feast/core/FeatureSetReference.proto | 33 +++++++++++ protos/feast/core/IngestionJob.proto | 65 +++++++++++++++++++++ 3 files changed, 150 insertions(+), 1 deletion(-) create mode 100644 protos/feast/core/FeatureSetReference.proto create mode 100644 protos/feast/core/IngestionJob.proto diff --git a/protos/feast/core/CoreService.proto b/protos/feast/core/CoreService.proto index 35b96e17895..037bcc12ddb 100644 --- a/protos/feast/core/CoreService.proto +++ b/protos/feast/core/CoreService.proto @@ -24,6 +24,8 @@ option java_package = "feast.core"; import "feast/core/FeatureSet.proto"; import "feast/core/Store.proto"; +import "feast/core/FeatureSetReference.proto"; +import "feast/core/IngestionJob.proto"; service CoreService { // Retrieve version information about this Feast deployment @@ -73,6 +75,19 @@ service CoreService { // Lists all projects active projects. rpc ListProjects (ListProjectsRequest) returns (ListProjectsResponse); + + // List Injestion Jobs + // TODO: write docs + rpc ListIngestionJobs(ListIngestionJobsRequest) returns (ListIngestionJobsResponse); + + // Restart an Ingestion job + // TODO: write docs + rpc RestartIngestionJob(RestartIngestionJobRequest) returns (RestartIngestionJobResponse); + + // Restart an Ingestion job + // TODO: write docs + rpc StopIngestionJob(StopIngestionJobRequest) returns (StopIngestionJobResponse); + } // Request for a single feature set @@ -215,4 +230,40 @@ message ListProjectsRequest { message ListProjectsResponse { // List of project names (archived projects are filtered out) repeated string projects = 1; -} \ No newline at end of file +} + +// Request for listing ingestion jobs +message ListIngestionJobsRequest { + message Filter { + // Job ID assigned by Feast + string id = 1; + // Feature set reference + FeatureSetReference feature_set_reference = 2; + // Name of store + string store_name = 3; + } +} + +// Response from listing ingestion jobs +message ListIngestionJobsResponse { + repeated IngestionJob jobs = 1; +} + +// Request to restart ingestion job +message RestartIngestionJobRequest { + // Job ID assigned by Feast + string id = 1; +} + +// Response from restartingan injestion job +message RestartIngestionJobResponse {} + + +// Request to stop ingestion job +message StopIngestionJobRequest { + // Job ID assigned by Feast + string id = 1; +} + +// Request from stopping an ingestion job +message StopIngestionJobResponse {} diff --git a/protos/feast/core/FeatureSetReference.proto b/protos/feast/core/FeatureSetReference.proto new file mode 100644 index 00000000000..2501ec0931c --- /dev/null +++ b/protos/feast/core/FeatureSetReference.proto @@ -0,0 +1,33 @@ +// +// Copyright 2020 The Feast Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +syntax = "proto3"; + +package feast.core; + +option go_package = "github.com/gojek/feast/sdk/go/protos/feast/core"; +option java_outer_classname = "FeatureSetReferenceProto"; +option java_package = "feast.core"; + +// Defines a composite key that refers to a unique FeatureSet +message FeatureSetReference { + // Name of the project + string project = 1; + // Name of the FeatureSet + string name = 2; + // Version no. of the FeatureSet + int32 version = 3; +} diff --git a/protos/feast/core/IngestionJob.proto b/protos/feast/core/IngestionJob.proto new file mode 100644 index 00000000000..68af28c0763 --- /dev/null +++ b/protos/feast/core/IngestionJob.proto @@ -0,0 +1,65 @@ +// +// Copyright 2020 The Feast Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +syntax = "proto3"; + +package feast.core; + +option go_package = "github.com/gojek/feast/sdk/go/protos/feast/core"; +option java_outer_classname = "IngestionJobProto"; +option java_package = "feast.core"; + +import "feast/core/FeatureSet.proto"; +import "feast/core/Store.proto"; +import "feast/core/Source.proto"; + +// Represents Feast Injestion Job +message IngestionJob { + // Job ID assigned by Feast + string id = 1; + // External job ID specific to the runner. + // For DirectRunner jobs, this is identical to id. For DataflowRunner jobs, this refers to the Dataflow job ID. + string external_id = 2; + IngestionJobStatus status = 3; + // List of feature sets whose features are populated by this job. + repeated feast.core.FeatureSet feature_sets = 4; + // Source this job is reading from. + feast.core.Source source = 5; + // Store this job is writing to. + feast.core.Store store = 6; +} + +// Status of a Feast Ingestion Job +enum IngestionJobStatus { + // Job status is not known. + UNKNOWN = 0; + // Import job is submitted to runner and currently pending for executing + PENDING = 1; + // Import job is currently running in the runner + RUNNING = 2; + // Runner's reported the import job has completed (applicable to batch job) + COMPLETED = 3; + // When user sent abort command, but it's still running + ABORTING = 4; + // User initiated abort job + ABORTED = 5; + // Runner's reported that the import job failed to run or there is a failure during job + ERROR = 6; + // job has been suspended and waiting for cleanup + SUSPENDING = 7; + // job has been suspended + SUSPENDED = 8; +} From bd0407667b849dac82194cc44262348f883fd00a Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Thu, 12 Mar 2020 10:48:44 +0800 Subject: [PATCH 02/66] Added query methods to JobRepository to query jobs by store and featureset --- core/src/main/java/feast/core/dao/JobRepository.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/core/src/main/java/feast/core/dao/JobRepository.java b/core/src/main/java/feast/core/dao/JobRepository.java index 98da76912e7..61bae25cea8 100644 --- a/core/src/main/java/feast/core/dao/JobRepository.java +++ b/core/src/main/java/feast/core/dao/JobRepository.java @@ -16,6 +16,7 @@ */ package feast.core.dao; +import feast.core.model.FeatureSet; import feast.core.model.Job; import feast.core.model.JobStatus; import java.util.Collection; @@ -29,4 +30,10 @@ public interface JobRepository extends JpaRepository { List findByStatusNotIn(Collection statuses); List findBySourceIdAndStoreNameOrderByLastUpdatedDesc(String sourceId, String storeName); + + // find jobs by feast store name + List findByStoreName(String storeName); + + // find jobs by featureset + List findByFeatureSets(FeatureSet featureSet); } From 643f0429fcf156a4f21517c4e2aec1ffd59dff05 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Thu, 12 Mar 2020 11:35:21 +0800 Subject: [PATCH 03/66] Added hashCode() & equals() to Job model compare and hash jobs This would allow jobs to be used as elements in HashSets and keys in HashMaps --- core/src/main/java/feast/core/model/Job.java | 35 ++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/core/src/main/java/feast/core/model/Job.java b/core/src/main/java/feast/core/model/Job.java index 377f5f70956..c0d012f9788 100644 --- a/core/src/main/java/feast/core/model/Job.java +++ b/core/src/main/java/feast/core/model/Job.java @@ -102,4 +102,39 @@ public void updateMetrics(List newMetrics) { public String getSinkName() { return store.getName(); } + + // compute hash for a job + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + ((id == null) ? 0 : id.hashCode()); + return result; + } + + // comparing jobs - true equal, false otherwise + + @Override + public boolean equals(Object obj) { + if(this == obj) { + return true; + } + if(obj == null) { + return false; + } + if(getClass() != obj.getClass()) { + return false; + } + + Job other = (Job) obj; + if(id == null) { + if(other.id != null) { + return false; + } + } else if(!id.equals(other.id)) { + return false; + } + return true; + } + } From f4b417ec3857a133fe3e16be6161441385a2abf8 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 16 Mar 2020 10:32:38 +0800 Subject: [PATCH 04/66] Added toIngestionProto() to Job object to convert Job model to ingestion job proto --- core/src/main/java/feast/core/model/Job.java | 62 +++++++++++++++++++- 1 file changed, 60 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/feast/core/model/Job.java b/core/src/main/java/feast/core/model/Job.java index c0d012f9788..dce56e9815d 100644 --- a/core/src/main/java/feast/core/model/Job.java +++ b/core/src/main/java/feast/core/model/Job.java @@ -16,8 +16,27 @@ */ package feast.core.model; +import java.util.ArrayList; import java.util.List; -import javax.persistence.*; +import java.util.Map; +import java.util.stream.Collectors; + +import javax.persistence.CascadeType; +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.EnumType; +import javax.persistence.Enumerated; +import javax.persistence.Id; +import javax.persistence.JoinColumn; +import javax.persistence.ManyToMany; +import javax.persistence.ManyToOne; +import javax.persistence.OneToMany; +import javax.persistence.Table; + +import com.google.protobuf.InvalidProtocolBufferException; + +import feast.core.FeatureSetProto; +import feast.core.IngestionJobProto; import lombok.AllArgsConstructor; import lombok.Getter; import lombok.Setter; @@ -102,6 +121,46 @@ public void updateMetrics(List newMetrics) { public String getSinkName() { return store.getName(); } + + /** + * Convert a job model to ingestion job proto + * + * @param job job model to convert + * @return Ingestion Job proto derieved from the given job + */ + public IngestionJobProto.IngestionJob toIngestionProto() + throws InvalidProtocolBufferException { + // maps job models job status to ingestion job status + Map statusMap = Map.of( + JobStatus.UNKNOWN, IngestionJobProto.IngestionJobStatus.UNKNOWN, + JobStatus.PENDING, IngestionJobProto.IngestionJobStatus.PENDING, + JobStatus.RUNNING, IngestionJobProto.IngestionJobStatus.RUNNING, + JobStatus.COMPLETED, IngestionJobProto.IngestionJobStatus.COMPLETED, + JobStatus.ABORTING, IngestionJobProto.IngestionJobStatus.ABORTING, + JobStatus.ABORTED, IngestionJobProto.IngestionJobStatus.ABORTED, + JobStatus.ERROR, IngestionJobProto.IngestionJobStatus.ERROR, + JobStatus.SUSPENDING, IngestionJobProto.IngestionJobStatus.SUSPENDING, + JobStatus.SUSPENDED, IngestionJobProto.IngestionJobStatus.SUSPENDED + ); + + // convert featuresets of job to protos + List featureSetProtos = new ArrayList<>(); + for(FeatureSet featureSet: this.getFeatureSets()) { + featureSetProtos.add(featureSet.toProto()); + } + + // build ingestion job proto with job data + IngestionJobProto.IngestionJob ingestJob = IngestionJobProto.IngestionJob.newBuilder() + .setId(this.getId()) + .setExternalId(this.getExtId()) + .setStatus(statusMap.get(this.getStatus())) + .addAllFeatureSets(featureSetProtos) + .setSource(this.getSource().toProto()) + .setStore(this.getStore().toProto()) + .build(); + + return ingestJob; + } // compute hash for a job @Override @@ -113,7 +172,6 @@ public int hashCode() { } // comparing jobs - true equal, false otherwise - @Override public boolean equals(Object obj) { if(this == obj) { From 9d62a38b3286a3d840e9578ec622a692b340c80a Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 16 Mar 2020 11:56:34 +0800 Subject: [PATCH 05/66] Added query methods to FeatureSetRepository to query by exact (name, project) or (name,version) --- .../main/java/feast/core/dao/FeatureSetRepository.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/feast/core/dao/FeatureSetRepository.java b/core/src/main/java/feast/core/dao/FeatureSetRepository.java index 3eba2108889..718dd761a37 100644 --- a/core/src/main/java/feast/core/dao/FeatureSetRepository.java +++ b/core/src/main/java/feast/core/dao/FeatureSetRepository.java @@ -35,7 +35,13 @@ FeatureSet findFirstFeatureSetByNameLikeAndProject_NameOrderByVersionDesc( // find all feature sets and order by name and version List findAllByOrderByNameAscVersionAsc(); - + + // find all feature sets by name and project name + List findAllByNameAndProject_Name(String name, String projectName); + + // find all feature sets by name and version + List findAllByNameAndVersion(String name, Integer version); + // find all feature sets within a project and order by name and version List findAllByProject_NameOrderByNameAscVersionAsc(String project_name); From 32a0bb5709337066d3cd4f89edf652615c19f15d Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 16 Mar 2020 11:57:29 +0800 Subject: [PATCH 06/66] Added listJobs() to JobService to handle request list ingestion jobs requests --- .../feast/core/dao/FeatureSetRepository.java | 6 +- .../java/feast/core/dao/JobRepository.java | 2 +- core/src/main/java/feast/core/model/Job.java | 80 +++++------ .../java/feast/core/service/JobService.java | 135 ++++++++++++++++++ 4 files changed, 176 insertions(+), 47 deletions(-) create mode 100644 core/src/main/java/feast/core/service/JobService.java diff --git a/core/src/main/java/feast/core/dao/FeatureSetRepository.java b/core/src/main/java/feast/core/dao/FeatureSetRepository.java index 718dd761a37..0ec3bb6921c 100644 --- a/core/src/main/java/feast/core/dao/FeatureSetRepository.java +++ b/core/src/main/java/feast/core/dao/FeatureSetRepository.java @@ -35,13 +35,13 @@ FeatureSet findFirstFeatureSetByNameLikeAndProject_NameOrderByVersionDesc( // find all feature sets and order by name and version List findAllByOrderByNameAscVersionAsc(); - + // find all feature sets by name and project name List findAllByNameAndProject_Name(String name, String projectName); - + // find all feature sets by name and version List findAllByNameAndVersion(String name, Integer version); - + // find all feature sets within a project and order by name and version List findAllByProject_NameOrderByNameAscVersionAsc(String project_name); diff --git a/core/src/main/java/feast/core/dao/JobRepository.java b/core/src/main/java/feast/core/dao/JobRepository.java index 61bae25cea8..f05ca457ac8 100644 --- a/core/src/main/java/feast/core/dao/JobRepository.java +++ b/core/src/main/java/feast/core/dao/JobRepository.java @@ -35,5 +35,5 @@ public interface JobRepository extends JpaRepository { List findByStoreName(String storeName); // find jobs by featureset - List findByFeatureSets(FeatureSet featureSet); + List findByFeatureSetIn(List featureSets); } diff --git a/core/src/main/java/feast/core/model/Job.java b/core/src/main/java/feast/core/model/Job.java index dce56e9815d..3ac8e731033 100644 --- a/core/src/main/java/feast/core/model/Job.java +++ b/core/src/main/java/feast/core/model/Job.java @@ -16,11 +16,12 @@ */ package feast.core.model; +import com.google.protobuf.InvalidProtocolBufferException; +import feast.core.FeatureSetProto; +import feast.core.IngestionJobProto; import java.util.ArrayList; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; - import javax.persistence.CascadeType; import javax.persistence.Column; import javax.persistence.Entity; @@ -32,11 +33,6 @@ import javax.persistence.ManyToOne; import javax.persistence.OneToMany; import javax.persistence.Table; - -import com.google.protobuf.InvalidProtocolBufferException; - -import feast.core.FeatureSetProto; -import feast.core.IngestionJobProto; import lombok.AllArgsConstructor; import lombok.Getter; import lombok.Setter; @@ -121,44 +117,43 @@ public void updateMetrics(List newMetrics) { public String getSinkName() { return store.getName(); } - + /** * Convert a job model to ingestion job proto - * - * @param job job model to convert + * * @return Ingestion Job proto derieved from the given job - */ - public IngestionJobProto.IngestionJob toIngestionProto() - throws InvalidProtocolBufferException { + */ + public IngestionJobProto.IngestionJob toIngestionProto() throws InvalidProtocolBufferException { // maps job models job status to ingestion job status - Map statusMap = Map.of( - JobStatus.UNKNOWN, IngestionJobProto.IngestionJobStatus.UNKNOWN, - JobStatus.PENDING, IngestionJobProto.IngestionJobStatus.PENDING, - JobStatus.RUNNING, IngestionJobProto.IngestionJobStatus.RUNNING, - JobStatus.COMPLETED, IngestionJobProto.IngestionJobStatus.COMPLETED, - JobStatus.ABORTING, IngestionJobProto.IngestionJobStatus.ABORTING, - JobStatus.ABORTED, IngestionJobProto.IngestionJobStatus.ABORTED, - JobStatus.ERROR, IngestionJobProto.IngestionJobStatus.ERROR, - JobStatus.SUSPENDING, IngestionJobProto.IngestionJobStatus.SUSPENDING, - JobStatus.SUSPENDED, IngestionJobProto.IngestionJobStatus.SUSPENDED - ); - + Map statusMap = + Map.of( + JobStatus.UNKNOWN, IngestionJobProto.IngestionJobStatus.UNKNOWN, + JobStatus.PENDING, IngestionJobProto.IngestionJobStatus.PENDING, + JobStatus.RUNNING, IngestionJobProto.IngestionJobStatus.RUNNING, + JobStatus.COMPLETED, IngestionJobProto.IngestionJobStatus.COMPLETED, + JobStatus.ABORTING, IngestionJobProto.IngestionJobStatus.ABORTING, + JobStatus.ABORTED, IngestionJobProto.IngestionJobStatus.ABORTED, + JobStatus.ERROR, IngestionJobProto.IngestionJobStatus.ERROR, + JobStatus.SUSPENDING, IngestionJobProto.IngestionJobStatus.SUSPENDING, + JobStatus.SUSPENDED, IngestionJobProto.IngestionJobStatus.SUSPENDED); + // convert featuresets of job to protos List featureSetProtos = new ArrayList<>(); - for(FeatureSet featureSet: this.getFeatureSets()) { + for (FeatureSet featureSet : this.getFeatureSets()) { featureSetProtos.add(featureSet.toProto()); } - + // build ingestion job proto with job data - IngestionJobProto.IngestionJob ingestJob = IngestionJobProto.IngestionJob.newBuilder() - .setId(this.getId()) - .setExternalId(this.getExtId()) - .setStatus(statusMap.get(this.getStatus())) - .addAllFeatureSets(featureSetProtos) - .setSource(this.getSource().toProto()) - .setStore(this.getStore().toProto()) - .build(); - + IngestionJobProto.IngestionJob ingestJob = + IngestionJobProto.IngestionJob.newBuilder() + .setId(this.getId()) + .setExternalId(this.getExtId()) + .setStatus(statusMap.get(this.getStatus())) + .addAllFeatureSets(featureSetProtos) + .setSource(this.getSource().toProto()) + .setStore(this.getStore().toProto()) + .build(); + return ingestJob; } @@ -174,25 +169,24 @@ public int hashCode() { // comparing jobs - true equal, false otherwise @Override public boolean equals(Object obj) { - if(this == obj) { + if (this == obj) { return true; } - if(obj == null) { + if (obj == null) { return false; } - if(getClass() != obj.getClass()) { + if (getClass() != obj.getClass()) { return false; } Job other = (Job) obj; - if(id == null) { - if(other.id != null) { + if (id == null) { + if (other.id != null) { return false; } - } else if(!id.equals(other.id)) { + } else if (!id.equals(other.id)) { return false; } return true; } - } diff --git a/core/src/main/java/feast/core/service/JobService.java b/core/src/main/java/feast/core/service/JobService.java new file mode 100644 index 00000000000..77fe7887be2 --- /dev/null +++ b/core/src/main/java/feast/core/service/JobService.java @@ -0,0 +1,135 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright 2018-2020 The Feast Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package feast.core.service; + +import com.google.protobuf.InvalidProtocolBufferException; +import feast.core.CoreServiceProto.ListIngestionJobsRequest; +import feast.core.CoreServiceProto.ListIngestionJobsResponse; +import feast.core.FeatureSetReferenceProto.FeatureSetReference; +import feast.core.IngestionJobProto; +import feast.core.dao.FeatureSetRepository; +import feast.core.dao.JobRepository; +import feast.core.job.JobManager; +import feast.core.model.FeatureSet; +import feast.core.model.Job; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Service; + +/** Defines a Job Managemenent Service that allows users to manage feast ingestion jobs. */ +@Service +public class JobService { + private JobRepository jobRepository; + private FeatureSetRepository featureSetRepository; + private List jobManagers; + + @Autowired + public JobService( + JobRepository jobRepository, + FeatureSetRepository featureSetRepository, + List jobManagers) { + this.jobRepository = jobRepository; + this.featureSetRepository = featureSetRepository; + this.jobManagers = jobManagers; + } + + /* Service API */ + /** + * List Ingestion Jobs in feast matching the given filter - + * + * @param filter to use to filter match against ingestion jobs + * @throws UnsupportedOperationException when given filter of an unsupported + * @throws InvalidProtocolBufferException if an error occurred when constructing ingestion job + * protobuf + * @return list ingestion jobs response + */ + // TODO: @Transactional ??? + public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest.Filter filter) + throws InvalidProtocolBufferException, UnsupportedOperationException { + // filter jobs based on request filter + Set matchingJobs = new HashSet<>(); + + // for proto3, default value for missing values: + // - numeric values (ie int) is zero + // - strings is empty string + + if (filter.getId() != "") { + // get by id: no more filters required: found job + Optional job = this.jobRepository.findById(filter.getId()); + if (job.isPresent()) { + matchingJobs.add(job.get()); + } + + } else if (filter.getStoreName() != "") { + // find by name + matchingJobs.addAll(this.jobRepository.findByStoreName(filter.getStoreName())); + } else if (filter.hasFeatureSetReference()) { + // find a matching featureset for reference + FeatureSetReference fsReference = filter.getFeatureSetReference(); + List matchFeatureSets = this.findFeatureSets(fsReference); + this.jobRepository.findByFeatureSetIn(matchFeatureSets); + } + + // convert matching job models to ingestion job protos + List ingestJobs = new ArrayList<>(); + for (Job job : matchingJobs) { + ingestJobs.add(job.toIngestionProto()); + } + + // pack jobs into response + return ListIngestionJobsResponse.newBuilder().addAllJobs(ingestJobs).build(); + } + + /* Private Utility Methods */ + /** + * Finds & returns featuresets matching the given feature set refererence + * + * @param fsReference FeatureSetReference that specifies which featuresets to match + * @throws UnsupportedOperationException fsReference given is unsupported. + * @return Returns a list of matching featuresets + */ + private List findFeatureSets(FeatureSetReference fsReference) + throws UnsupportedOperationException { + + String fsName = fsReference.getName(); + String fsProject = fsReference.getProject(); + Integer fsVersion = fsReference.getVersion(); + + List featureSets = new ArrayList<>(); + if (fsName != "" && fsProject != "" && fsVersion != 0) { + featureSets.add( + this.featureSetRepository.findFeatureSetByNameAndProject_NameAndVersion( + fsName, fsProject, fsVersion)); + } else if (fsName != "" && fsProject != "") { + featureSets.addAll(this.featureSetRepository.findAllByNameAndProject_Name(fsName, fsProject)); + } else if (fsName != "" && fsVersion != 0) { + featureSets.addAll(this.featureSetRepository.findAllByNameAndVersion(fsName, fsVersion)); + } else { + throw new UnsupportedOperationException( + String.format( + "Unsupported featureset refererence configuration: " + + "(name: '%s', project: '%s', version: '%d')", + fsName, fsProject, fsVersion)); + } + + return featureSets; + } +} From 4f550123ed44de51e0b1ed19841e839fb7c69167 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Tue, 17 Mar 2020 12:57:45 +0800 Subject: [PATCH 07/66] Added missing filter field in ListIngestionJobsRequest protobuf --- protos/feast/core/CoreService.proto | 2 ++ 1 file changed, 2 insertions(+) diff --git a/protos/feast/core/CoreService.proto b/protos/feast/core/CoreService.proto index 037bcc12ddb..17cecdd765e 100644 --- a/protos/feast/core/CoreService.proto +++ b/protos/feast/core/CoreService.proto @@ -234,6 +234,8 @@ message ListProjectsResponse { // Request for listing ingestion jobs message ListIngestionJobsRequest { + Filter filter = 1; + message Filter { // Job ID assigned by Feast string id = 1; From b3aa72ab960ce5f224b77c7b1e2818a34cc5556a Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Tue, 17 Mar 2020 13:02:42 +0800 Subject: [PATCH 08/66] Added code to setup mockups for testing JobService --- .../java/feast/core/service/JobService.java | 48 +++++-- .../feast/core/service/JobServiceTest.java | 132 ++++++++++++++++++ 2 files changed, 167 insertions(+), 13 deletions(-) create mode 100644 core/src/test/java/feast/core/service/JobServiceTest.java diff --git a/core/src/main/java/feast/core/service/JobService.java b/core/src/main/java/feast/core/service/JobService.java index 77fe7887be2..0aeb2d2553f 100644 --- a/core/src/main/java/feast/core/service/JobService.java +++ b/core/src/main/java/feast/core/service/JobService.java @@ -24,11 +24,14 @@ import feast.core.dao.FeatureSetRepository; import feast.core.dao.JobRepository; import feast.core.job.JobManager; +import feast.core.job.Runner; import feast.core.model.FeatureSet; import feast.core.model.Job; import java.util.ArrayList; +import java.util.Collection; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; import org.springframework.beans.factory.annotation.Autowired; @@ -39,16 +42,19 @@ public class JobService { private JobRepository jobRepository; private FeatureSetRepository featureSetRepository; - private List jobManagers; + private Map jobManagers; @Autowired public JobService( JobRepository jobRepository, FeatureSetRepository featureSetRepository, - List jobManagers) { + List jobManagerList) { this.jobRepository = jobRepository; this.featureSetRepository = featureSetRepository; - this.jobManagers = jobManagers; + + for (JobManager manager : jobManagerList) { + this.jobManagers.put(manager.getRunnerType(), manager); + } } /* Service API */ @@ -56,12 +62,11 @@ public JobService( * List Ingestion Jobs in feast matching the given filter - * * @param filter to use to filter match against ingestion jobs - * @throws UnsupportedOperationException when given filter of an unsupported + * @throws UnsupportedOperationException when given a filter in a unsupported configuration * @throws InvalidProtocolBufferException if an error occurred when constructing ingestion job * protobuf * @return list ingestion jobs response */ - // TODO: @Transactional ??? public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest.Filter filter) throws InvalidProtocolBufferException, UnsupportedOperationException { // filter jobs based on request filter @@ -78,14 +83,20 @@ public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest.Filter filter matchingJobs.add(job.get()); } - } else if (filter.getStoreName() != "") { - // find by name - matchingJobs.addAll(this.jobRepository.findByStoreName(filter.getStoreName())); - } else if (filter.hasFeatureSetReference()) { - // find a matching featureset for reference - FeatureSetReference fsReference = filter.getFeatureSetReference(); - List matchFeatureSets = this.findFeatureSets(fsReference); - this.jobRepository.findByFeatureSetIn(matchFeatureSets); + } else { + // multiple filters can apply together in an 'and' operation + if (filter.getStoreName() != "") { + // find jobs by name + Collection jobs = this.jobRepository.findByStoreName(filter.getStoreName()); + matchingJobs = this.mergeResults(matchingJobs, jobs); + } + if (filter.hasFeatureSetReference()) { + // find a matching featureset for reference + FeatureSetReference fsReference = filter.getFeatureSetReference(); + List matchFeatureSets = this.findFeatureSets(fsReference); + Collection jobs = this.jobRepository.findByFeatureSetIn(matchFeatureSets); + matchingJobs = this.mergeResults(matchingJobs, jobs); + } } // convert matching job models to ingestion job protos @@ -132,4 +143,15 @@ private List findFeatureSets(FeatureSetReference fsReference) return featureSets; } + + private Set mergeResults(Set results, Collection newResults) { + if (results.size() <= 0) { + // no existing results: copy over new results + results.addAll(newResults); + } else { + // and operation: keep results that exist in both existing and new results + results.retainAll(newResults); + } + return results; + } } diff --git a/core/src/test/java/feast/core/service/JobServiceTest.java b/core/src/test/java/feast/core/service/JobServiceTest.java new file mode 100644 index 00000000000..f962934c9a4 --- /dev/null +++ b/core/src/test/java/feast/core/service/JobServiceTest.java @@ -0,0 +1,132 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright 2018-2020 The Feast Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package feast.core.service; + +import static org.mockito.Mockito.when; +import static org.mockito.MockitoAnnotations.initMocks; + +import feast.core.FeatureSetProto.FeatureSetStatus; +import feast.core.SourceProto.KafkaSourceConfig; +import feast.core.SourceProto.SourceType; +import feast.core.StoreProto.Store.RedisConfig; +import feast.core.StoreProto.Store.StoreType; +import feast.core.dao.FeatureSetRepository; +import feast.core.dao.JobRepository; +import feast.core.job.JobManager; +import feast.core.job.Runner; +import feast.core.model.FeatureSet; +import feast.core.model.Field; +import feast.core.model.Job; +import feast.core.model.JobStatus; +import feast.core.model.Source; +import feast.core.model.Store; +import feast.types.ValueProto.ValueType.Enum; +import java.time.Instant; +import java.util.Arrays; +import java.util.Date; +import java.util.List; +import java.util.Optional; +import org.junit.Before; +import org.mockito.Mock; + +public class JobServiceTest { + @Mock private FeatureSetRepository featureSetRepository; + @Mock private JobRepository jobRepository; + @Mock private List jobManagers; + + private Source dataSource; + private Store dataStore; + private FeatureSet featureSet; + private Job job; + /* unit test setup */ + @Before + public void setup() { + initMocks(this); + + // create mock objects for testing + // fake data source + this.dataSource = + new Source( + SourceType.KAFKA, + KafkaSourceConfig.newBuilder() + .setBootstrapServers("kafka:9092") + .setTopic("my-topic") + .build(), + true); + // fake data store + Store store = new Store(); + store.setName("feast-redis"); + store.setType(StoreType.REDIS.toString()); + store.setSubscriptions("*:*:*"); + store.setConfig(RedisConfig.newBuilder().setPort(6379).build().toByteArray()); + this.dataStore = store; + // fake featureset & job + this.featureSet = this.newDummyFeatureSet("food", 2, "hunger"); + this.job = this.newDummyJob("job", "kafka-to-redis", JobStatus.PENDING); + // setup mock repositories + this.setupFeatureSetRepository(); + this.setupJobRepository(); + } + + // setup fake feature set repository + public void setupFeatureSetRepository() { + when(this.featureSetRepository.findFeatureSetByNameAndProject_NameAndVersion( + "food", "hunger", 2)) + .thenReturn(this.featureSet); + when(this.featureSetRepository.findAllByNameAndProject_Name("food", "hunger")) + .thenReturn(Arrays.asList(featureSet)); + when(this.featureSetRepository.findAllByNameAndVersion("food", 2)) + .thenReturn(Arrays.asList(featureSet)); + } + + // setup fake job repository + public void setupJobRepository() { + when(this.jobRepository.findById("job")).thenReturn(Optional.of(this.job)); + when(this.jobRepository.findByStoreName("feast-store")).thenReturn(Arrays.asList(this.job)); + when(this.jobRepository.findByFeatureSetIn(Arrays.asList(this.featureSet))) + .thenReturn(Arrays.asList(this.job)); + } + + /* private utilities */ + private FeatureSet newDummyFeatureSet(String name, int version, String project) { + Field feature = new Field(name + "_feature", Enum.INT64); + Field entity = new Field(name + "_entity", Enum.STRING); + + FeatureSet fs = + new FeatureSet( + name, + project, + version, + 100L, + Arrays.asList(entity), + Arrays.asList(feature), + this.dataSource, + FeatureSetStatus.STATUS_READY); + fs.setCreated(Date.from(Instant.ofEpochSecond(10L))); + return fs; + } + + private Job newDummyJob(String id, String name, JobStatus status) { + return new Job( + id, + name, + Runner.DATAFLOW.getName(), + this.dataSource, + this.dataStore, + Arrays.asList(this.featureSet), status); + } +} From 06b84dfbb8a5ea83271cdfbd3b22c44aaada472c Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Tue, 17 Mar 2020 17:10:53 +0800 Subject: [PATCH 09/66] Revert "Added hashCode() & equals() to Job model compare and hash jobs" This reverts commit ba995bb712cbd38f0f4bef47efbe433d5ec07521 as it caused core tests to fail --- core/src/main/java/feast/core/model/Job.java | 33 -------------------- 1 file changed, 33 deletions(-) diff --git a/core/src/main/java/feast/core/model/Job.java b/core/src/main/java/feast/core/model/Job.java index 3ac8e731033..376e5dc694e 100644 --- a/core/src/main/java/feast/core/model/Job.java +++ b/core/src/main/java/feast/core/model/Job.java @@ -156,37 +156,4 @@ public IngestionJobProto.IngestionJob toIngestionProto() throws InvalidProtocolB return ingestJob; } - - // compute hash for a job - @Override - public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + ((id == null) ? 0 : id.hashCode()); - return result; - } - - // comparing jobs - true equal, false otherwise - @Override - public boolean equals(Object obj) { - if (this == obj) { - return true; - } - if (obj == null) { - return false; - } - if (getClass() != obj.getClass()) { - return false; - } - - Job other = (Job) obj; - if (id == null) { - if (other.id != null) { - return false; - } - } else if (!id.equals(other.id)) { - return false; - } - return true; - } } From c4b901ab212371900c85dfe4209a8342fb388a6e Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Tue, 17 Mar 2020 19:02:01 +0800 Subject: [PATCH 10/66] Added JobServiceTest unit tests for JobService's listJobs() --- core/src/main/java/feast/core/model/Job.java | 3 +- .../java/feast/core/service/JobService.java | 59 +++++++-- .../feast/core/service/JobServiceTest.java | 122 ++++++++++++++++-- 3 files changed, 159 insertions(+), 25 deletions(-) diff --git a/core/src/main/java/feast/core/model/Job.java b/core/src/main/java/feast/core/model/Job.java index 376e5dc694e..08c41aecc3c 100644 --- a/core/src/main/java/feast/core/model/Job.java +++ b/core/src/main/java/feast/core/model/Job.java @@ -123,7 +123,8 @@ public String getSinkName() { * * @return Ingestion Job proto derieved from the given job */ - public IngestionJobProto.IngestionJob toIngestionProto() throws InvalidProtocolBufferException { + public IngestionJobProto.IngestionJob toIngestionProto() + throws InvalidProtocolBufferException { // maps job models job status to ingestion job status Map statusMap = Map.of( diff --git a/core/src/main/java/feast/core/service/JobService.java b/core/src/main/java/feast/core/service/JobService.java index 0aeb2d2553f..c7bf01481b3 100644 --- a/core/src/main/java/feast/core/service/JobService.java +++ b/core/src/main/java/feast/core/service/JobService.java @@ -34,6 +34,7 @@ import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.stream.Collectors; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; @@ -59,18 +60,18 @@ public JobService( /* Service API */ /** - * List Ingestion Jobs in feast matching the given filter - + * List Ingestion Jobs in feast matching the given request * - * @param filter to use to filter match against ingestion jobs - * @throws UnsupportedOperationException when given a filter in a unsupported configuration - * @throws InvalidProtocolBufferException if an error occurred when constructing ingestion job - * protobuf + * @param request list ingestion jobs request specifying which jobs to include + * @throws UnsupportedOperationException when given filter in a unsupported configuration + * @throws InvalidProtocolBufferException on error when constructing response protobuf * @return list ingestion jobs response */ - public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest.Filter filter) - throws InvalidProtocolBufferException, UnsupportedOperationException { + public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest request) + throws UnsupportedOperationException, InvalidProtocolBufferException { // filter jobs based on request filter - Set matchingJobs = new HashSet<>(); + ListIngestionJobsRequest.Filter filter = request.getFilter(); + Set matchingJobIds = new HashSet<>(); // for proto3, default value for missing values: // - numeric values (ie int) is zero @@ -80,34 +81,66 @@ public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest.Filter filter // get by id: no more filters required: found job Optional job = this.jobRepository.findById(filter.getId()); if (job.isPresent()) { - matchingJobs.add(job.get()); + matchingJobIds.add(filter.getId()); } } else { // multiple filters can apply together in an 'and' operation if (filter.getStoreName() != "") { // find jobs by name - Collection jobs = this.jobRepository.findByStoreName(filter.getStoreName()); - matchingJobs = this.mergeResults(matchingJobs, jobs); + List jobs = this.jobRepository.findByStoreName(filter.getStoreName()); + List jobIds = + jobs.stream() + .map( + job -> { + return job.getId(); + }) + .collect(Collectors.toList()); + + matchingJobIds = this.mergeResults(matchingJobIds, jobIds); } if (filter.hasFeatureSetReference()) { // find a matching featureset for reference FeatureSetReference fsReference = filter.getFeatureSetReference(); List matchFeatureSets = this.findFeatureSets(fsReference); Collection jobs = this.jobRepository.findByFeatureSetIn(matchFeatureSets); - matchingJobs = this.mergeResults(matchingJobs, jobs); + + List jobIds = + jobs.stream() + .map( + job -> { + return job.getId(); + }) + .collect(Collectors.toList()); + matchingJobIds = this.mergeResults(matchingJobIds, jobIds); } } // convert matching job models to ingestion job protos List ingestJobs = new ArrayList<>(); - for (Job job : matchingJobs) { + for (String jobId : matchingJobIds) { + Job job = this.jobRepository.findById(jobId).get(); ingestJobs.add(job.toIngestionProto()); } // pack jobs into response return ListIngestionJobsResponse.newBuilder().addAllJobs(ingestJobs).build(); } + + //TODO: restart ingestion job + + /** + * Stops the ingestion job matching the given request + * + * @param request stop ingestion job request specifying which job to stop + * @throw InvalidProtocolBufferException whe + * */ + /* + public StopIngestionJobResponse stopJob(StopIngestionJobRequest request) + throws InvalidProtocolBufferException { + + } + */ /* Private Utility Methods */ /** diff --git a/core/src/test/java/feast/core/service/JobServiceTest.java b/core/src/test/java/feast/core/service/JobServiceTest.java index f962934c9a4..dec5c6da144 100644 --- a/core/src/test/java/feast/core/service/JobServiceTest.java +++ b/core/src/test/java/feast/core/service/JobServiceTest.java @@ -16,10 +16,17 @@ */ package feast.core.service; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.initMocks; +import com.google.protobuf.InvalidProtocolBufferException; +import feast.core.CoreServiceProto.ListIngestionJobsRequest; +import feast.core.CoreServiceProto.ListIngestionJobsResponse; import feast.core.FeatureSetProto.FeatureSetStatus; +import feast.core.FeatureSetReferenceProto.FeatureSetReference; +import feast.core.IngestionJobProto.IngestionJob; import feast.core.SourceProto.KafkaSourceConfig; import feast.core.SourceProto.SourceType; import feast.core.StoreProto.Store.RedisConfig; @@ -36,22 +43,29 @@ import feast.core.model.Store; import feast.types.ValueProto.ValueType.Enum; import java.time.Instant; +import java.util.ArrayList; import java.util.Arrays; import java.util.Date; import java.util.List; import java.util.Optional; import org.junit.Before; +import org.junit.Test; import org.mockito.Mock; public class JobServiceTest { + // mocks @Mock private FeatureSetRepository featureSetRepository; @Mock private JobRepository jobRepository; @Mock private List jobManagers; - + // fake models private Source dataSource; private Store dataStore; private FeatureSet featureSet; private Job job; + private IngestionJob ingestionJob; + // test target + public JobService jobService; + /* unit test setup */ @Before public void setup() { @@ -68,18 +82,31 @@ public void setup() { .build(), true); // fake data store - Store store = new Store(); - store.setName("feast-redis"); - store.setType(StoreType.REDIS.toString()); - store.setSubscriptions("*:*:*"); - store.setConfig(RedisConfig.newBuilder().setPort(6379).build().toByteArray()); - this.dataStore = store; + this.dataStore = + new Store( + "feast-redis", + StoreType.REDIS.toString(), + RedisConfig.newBuilder().setPort(6379).build().toByteArray(), + "*:*:*"); + // fake featureset & job this.featureSet = this.newDummyFeatureSet("food", 2, "hunger"); this.job = this.newDummyJob("job", "kafka-to-redis", JobStatus.PENDING); + try { + this.ingestionJob = this.job.toIngestionProto(); + } catch (InvalidProtocolBufferException e) { + e.printStackTrace(); + } + // setup mock repositories this.setupFeatureSetRepository(); this.setupJobRepository(); + + // TODO: init fake job managers + this.jobManagers = new ArrayList<>(); + + this.jobService = + new JobService(this.jobRepository, this.featureSetRepository, this.jobManagers); } // setup fake feature set repository @@ -95,13 +122,17 @@ public void setupFeatureSetRepository() { // setup fake job repository public void setupJobRepository() { - when(this.jobRepository.findById("job")).thenReturn(Optional.of(this.job)); - when(this.jobRepository.findByStoreName("feast-store")).thenReturn(Arrays.asList(this.job)); + when(this.jobRepository.findById(this.job.getId())).thenReturn(Optional.of(this.job)); + when(this.jobRepository.findByStoreName(this.dataStore.getName())) + .thenReturn(Arrays.asList(this.job)); when(this.jobRepository.findByFeatureSetIn(Arrays.asList(this.featureSet))) .thenReturn(Arrays.asList(this.job)); } + + // TODO: setup fake job manager - /* private utilities */ + + // dummy model constructorss private FeatureSet newDummyFeatureSet(String name, int version, String project) { Field feature = new Field(name + "_feature", Enum.INT64); Field entity = new Field(name + "_entity", Enum.STRING); @@ -127,6 +158,75 @@ private Job newDummyJob(String id, String name, JobStatus status) { Runner.DATAFLOW.getName(), this.dataSource, this.dataStore, - Arrays.asList(this.featureSet), status); + Arrays.asList(this.featureSet), + status); + } + + /* unit tests */ + private ListIngestionJobsResponse tryListJobs(ListIngestionJobsRequest request) { + ListIngestionJobsResponse response = null; + try { + response = this.jobService.listJobs(request); + } catch(InvalidProtocolBufferException e){ + e.printStackTrace(); + fail("Caught Unexpected exception"); + } + + return response; + } + + // list jobs + @Test + public void testListJobsById() { + ListIngestionJobsRequest.Filter filter = + ListIngestionJobsRequest.Filter.newBuilder().setId(this.job.getId()).build(); + ListIngestionJobsRequest request = + ListIngestionJobsRequest.newBuilder().setFilter(filter).build(); + assertEquals(this.tryListJobs(request).getJobs(0), this.ingestionJob); + } + + @Test + public void testListJobsByStoreName() { + ListIngestionJobsRequest.Filter filter = + ListIngestionJobsRequest.Filter.newBuilder().setStoreName(this.dataStore.getName()).build(); + ListIngestionJobsRequest request = + ListIngestionJobsRequest.newBuilder().setFilter(filter).build(); + assertEquals(this.tryListJobs(request).getJobs(0), this.ingestionJob); + } + + @Test + public void testListIngestionJobByFeatureSetReference() { + // list job by feature set reference: name and version and project + FeatureSetReference fsReference = + FeatureSetReference.newBuilder() + .setVersion(this.featureSet.getVersion()) + .setName(this.featureSet.getName()) + .setProject(this.featureSet.getProject().toString()) + .build(); + ListIngestionJobsRequest.Filter filter = + ListIngestionJobsRequest.Filter.newBuilder().setId(this.job.getId()).build(); + ListIngestionJobsRequest request = + ListIngestionJobsRequest.newBuilder().setFilter(filter).build(); + assertEquals(this.tryListJobs(request).getJobs(0), this.ingestionJob); + + // list job by feature set reference: name and version + fsReference = + FeatureSetReference.newBuilder() + .setName(this.featureSet.getName()) + .setProject(this.featureSet.getProject().toString()) + .build(); + filter = ListIngestionJobsRequest.Filter.newBuilder().setId(this.job.getId()).build(); + request = ListIngestionJobsRequest.newBuilder().setFilter(filter).build(); + assertEquals(this.tryListJobs(request).getJobs(0), this.ingestionJob); + + // list job by feature set reference: name and project + fsReference = + FeatureSetReference.newBuilder() + .setName(this.featureSet.getName()) + .setVersion(this.featureSet.getVersion()) + .build(); + filter = ListIngestionJobsRequest.Filter.newBuilder().setId(this.job.getId()).build(); + request = ListIngestionJobsRequest.newBuilder().setFilter(filter).build(); + assertEquals(this.tryListJobs(request).getJobs(0), this.ingestionJob); } } From cd0bde2f5399bd68ffa79596b1d1cea6bbfd9346 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Wed, 18 Mar 2020 18:27:40 +0800 Subject: [PATCH 11/66] Added stopJobs() to JobService to handle requests to stop jobs --- core/src/main/java/feast/core/model/Job.java | 3 +- .../java/feast/core/service/JobService.java | 59 ++++++++--- .../feast/core/service/JobServiceTest.java | 99 ++++++++++++++++--- 3 files changed, 135 insertions(+), 26 deletions(-) diff --git a/core/src/main/java/feast/core/model/Job.java b/core/src/main/java/feast/core/model/Job.java index 08c41aecc3c..376e5dc694e 100644 --- a/core/src/main/java/feast/core/model/Job.java +++ b/core/src/main/java/feast/core/model/Job.java @@ -123,8 +123,7 @@ public String getSinkName() { * * @return Ingestion Job proto derieved from the given job */ - public IngestionJobProto.IngestionJob toIngestionProto() - throws InvalidProtocolBufferException { + public IngestionJobProto.IngestionJob toIngestionProto() throws InvalidProtocolBufferException { // maps job models job status to ingestion job status Map statusMap = Map.of( diff --git a/core/src/main/java/feast/core/service/JobService.java b/core/src/main/java/feast/core/service/JobService.java index c7bf01481b3..3c2b904504f 100644 --- a/core/src/main/java/feast/core/service/JobService.java +++ b/core/src/main/java/feast/core/service/JobService.java @@ -19,19 +19,23 @@ import com.google.protobuf.InvalidProtocolBufferException; import feast.core.CoreServiceProto.ListIngestionJobsRequest; import feast.core.CoreServiceProto.ListIngestionJobsResponse; +import feast.core.CoreServiceProto.StopIngestionJobRequest; +import feast.core.CoreServiceProto.StopIngestionJobResponse; import feast.core.FeatureSetReferenceProto.FeatureSetReference; import feast.core.IngestionJobProto; import feast.core.dao.FeatureSetRepository; import feast.core.dao.JobRepository; import feast.core.job.JobManager; -import feast.core.job.Runner; import feast.core.model.FeatureSet; import feast.core.model.Job; +import feast.core.model.JobStatus; import java.util.ArrayList; import java.util.Collection; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.NoSuchElementException; import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -43,7 +47,7 @@ public class JobService { private JobRepository jobRepository; private FeatureSetRepository featureSetRepository; - private Map jobManagers; + private Map jobManagers; @Autowired public JobService( @@ -53,12 +57,13 @@ public JobService( this.jobRepository = jobRepository; this.featureSetRepository = featureSetRepository; + this.jobManagers = new HashMap<>(); for (JobManager manager : jobManagerList) { - this.jobManagers.put(manager.getRunnerType(), manager); + this.jobManagers.put(manager.getRunnerType().getName(), manager); } } - /* Service API */ + /* Job Service API */ /** * List Ingestion Jobs in feast matching the given request * @@ -126,21 +131,49 @@ public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest request) // pack jobs into response return ListIngestionJobsResponse.newBuilder().addAllJobs(ingestJobs).build(); } - - //TODO: restart ingestion job + + // TODO: restart ingestion job /** - * Stops the ingestion job matching the given request - * + * Stops (Aborts) the ingestion job matching the given request. Does nothing if the target job to + * be stopped is already stopped or stopping + * * @param request stop ingestion job request specifying which job to stop - * @throw InvalidProtocolBufferException whe - * */ - /* + * @throws NoSuchElementException when stop job request requests to stop a nonexistent job. + * @throws UnsupportedOperationException when job to be stopped is in an unknown status + * @throws InvalidProtocolBufferException on error when constructing response protobuf + */ public StopIngestionJobResponse stopJob(StopIngestionJobRequest request) - throws InvalidProtocolBufferException { + throws InvalidProtocolBufferException { + // check job exists + Optional getJob = this.jobRepository.findById(request.getId()); + if (getJob.isEmpty()) { + throw new NoSuchElementException( + "Attempted to stop nonexistent job with id: " + getJob.get().getId()); + } + + // check job status is valid for stopping + Job job = getJob.get(); + JobStatus status = job.getStatus(); + if (status.equals(JobStatus.ABORTED) + || status.equals(JobStatus.ABORTING) + || status.equals(JobStatus.SUSPENDED) + || status.equals(JobStatus.SUSPENDING) + || status.equals(JobStatus.COMPLETED) + || status.equals(JobStatus.ERROR)) { + // do nothing - job is already stopped or stopping + return StopIngestionJobResponse.newBuilder().build(); + } else if (status.equals(JobStatus.UNKNOWN)) { + throw new UnsupportedOperationException( + "Stopping a job with an unknown status is unsupported"); + } + + // stop job with job manager + JobManager jobManager = this.jobManagers.get(job.getRunner()); + jobManager.abortJob(job.getExtId()); + return StopIngestionJobResponse.newBuilder().build(); } - */ /* Private Utility Methods */ /** diff --git a/core/src/test/java/feast/core/service/JobServiceTest.java b/core/src/test/java/feast/core/service/JobServiceTest.java index dec5c6da144..79a062cdd33 100644 --- a/core/src/test/java/feast/core/service/JobServiceTest.java +++ b/core/src/test/java/feast/core/service/JobServiceTest.java @@ -18,12 +18,16 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.initMocks; import com.google.protobuf.InvalidProtocolBufferException; import feast.core.CoreServiceProto.ListIngestionJobsRequest; import feast.core.CoreServiceProto.ListIngestionJobsResponse; +import feast.core.CoreServiceProto.StopIngestionJobRequest; +import feast.core.CoreServiceProto.StopIngestionJobResponse; import feast.core.FeatureSetProto.FeatureSetStatus; import feast.core.FeatureSetReferenceProto.FeatureSetReference; import feast.core.IngestionJobProto.IngestionJob; @@ -43,7 +47,6 @@ import feast.core.model.Store; import feast.types.ValueProto.ValueType.Enum; import java.time.Instant; -import java.util.ArrayList; import java.util.Arrays; import java.util.Date; import java.util.List; @@ -56,7 +59,7 @@ public class JobServiceTest { // mocks @Mock private FeatureSetRepository featureSetRepository; @Mock private JobRepository jobRepository; - @Mock private List jobManagers; + @Mock private JobManager jobManager; // fake models private Source dataSource; private Store dataStore; @@ -98,15 +101,15 @@ public void setup() { e.printStackTrace(); } - // setup mock repositories + // setup mock objects this.setupFeatureSetRepository(); this.setupJobRepository(); + this.setupJobManager(); - // TODO: init fake job managers - this.jobManagers = new ArrayList<>(); - + // create test target this.jobService = - new JobService(this.jobRepository, this.featureSetRepository, this.jobManagers); + new JobService( + this.jobRepository, this.featureSetRepository, Arrays.asList(this.jobManager)); } // setup fake feature set repository @@ -128,9 +131,11 @@ public void setupJobRepository() { when(this.jobRepository.findByFeatureSetIn(Arrays.asList(this.featureSet))) .thenReturn(Arrays.asList(this.job)); } - - // TODO: setup fake job manager + // TODO: setup fake job manager + public void setupJobManager() { + when(this.jobManager.getRunnerType()).thenReturn(Runner.DATAFLOW); + } // dummy model constructorss private FeatureSet newDummyFeatureSet(String name, int version, String project) { @@ -167,11 +172,11 @@ private ListIngestionJobsResponse tryListJobs(ListIngestionJobsRequest request) ListIngestionJobsResponse response = null; try { response = this.jobService.listJobs(request); - } catch(InvalidProtocolBufferException e){ + } catch (InvalidProtocolBufferException e) { e.printStackTrace(); fail("Caught Unexpected exception"); } - + return response; } @@ -229,4 +234,76 @@ public void testListIngestionJobByFeatureSetReference() { request = ListIngestionJobsRequest.newBuilder().setFilter(filter).build(); assertEquals(this.tryListJobs(request).getJobs(0), this.ingestionJob); } + + // stop jobs + private StopIngestionJobResponse tryStopJob( + StopIngestionJobRequest request, boolean expectError) { + StopIngestionJobResponse response = null; + try { + response = this.jobService.stopJob(request); + // expected exception, but none was thrown + if (expectError) { + fail("Expected exception, but none was thrown"); + } + } catch (Exception e) { + if (expectError != true) { + // unexpected exception + e.printStackTrace(); + fail("Caught Unexpected exception"); + } + } + + return response; + } + + @Test + public void testStopJobForId() { + JobStatus prevStatus = this.job.getStatus(); + this.job.setStatus(JobStatus.RUNNING); + + StopIngestionJobRequest request = + StopIngestionJobRequest.newBuilder().setId(this.job.getId()).build(); + this.tryStopJob(request, false); + verify(this.jobManager).abortJob(this.job.getExtId()); + + this.job.setStatus(prevStatus); + } + + @Test + public void testStopAlreadyStop() { + // check that stop jobs does not trying to stop jobs that are + // not already stopped/stopping + List doNothingStatuses = + Arrays.asList( + JobStatus.SUSPENDED, JobStatus.SUSPENDING, + JobStatus.ABORTED, JobStatus.ABORTING, + JobStatus.COMPLETED, JobStatus.ABORTED); + + JobStatus prevStatus = this.job.getStatus(); + for (JobStatus status : doNothingStatuses) { + this.job.setStatus(status); + + StopIngestionJobRequest request = + StopIngestionJobRequest.newBuilder().setId(this.job.getId()).build(); + this.tryStopJob(request, false); + + verify(this.jobManager, never()).abortJob(this.job.getExtId()); + } + + this.job.setStatus(prevStatus); + } + + @Test + public void testStopUnknownJobError() { + // check for UnsupportedOperationException when trying to stop jobs are + // in an in unknown state + JobStatus prevStatus = this.job.getStatus(); + this.job.setStatus(JobStatus.UNKNOWN); + + StopIngestionJobRequest request = + StopIngestionJobRequest.newBuilder().setId(this.job.getId()).build(); + this.tryStopJob(request, true); + + this.job.setStatus(prevStatus); + } } From c59035f48f62901b0a8cc9cb8d2325ce372c2a96 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Fri, 20 Mar 2020 11:18:27 +0800 Subject: [PATCH 12/66] Added getTransitionalStates() to JobStatus to return collection of transitional states --- core/src/main/java/feast/core/model/JobStatus.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/core/src/main/java/feast/core/model/JobStatus.java b/core/src/main/java/feast/core/model/JobStatus.java index 123b57a21b1..5bea208beb6 100644 --- a/core/src/main/java/feast/core/model/JobStatus.java +++ b/core/src/main/java/feast/core/model/JobStatus.java @@ -64,4 +64,18 @@ public enum JobStatus { public static Collection getTerminalState() { return TERMINAL_STATE; } + + private static final Collection TRANSITIONAL_STATES = + Collections.unmodifiableList(Arrays.asList(PENDING, ABORTING, SUSPENDING)); + + /** + * Get Transitional Job Status states. + * Transitionals states are assigned to jobs that transitioning to a more + * stable state (ie SUSPENDED, ABORTED etc.) + * + * @return Collection of transitional Job Status states. + */ + public static final Collection getTransitionalStates() { + return TRANSITIONAL_STATES; + } } From 898045cbf7d6215170c41c96010e55561e8dd418 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Fri, 20 Mar 2020 11:19:01 +0800 Subject: [PATCH 13/66] Changed stopJobs() to throw unsupported error on transitional job statuses --- .../java/feast/core/service/JobService.java | 19 +++++------- .../feast/core/service/JobServiceTest.java | 29 ++++++++++--------- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/core/src/main/java/feast/core/service/JobService.java b/core/src/main/java/feast/core/service/JobService.java index 3c2b904504f..454bd346bc8 100644 --- a/core/src/main/java/feast/core/service/JobService.java +++ b/core/src/main/java/feast/core/service/JobService.java @@ -135,12 +135,13 @@ public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest request) // TODO: restart ingestion job /** - * Stops (Aborts) the ingestion job matching the given request. Does nothing if the target job to - * be stopped is already stopped or stopping + * Stops (Aborts) the ingestion job matching the given request. + * Does nothing if the target job if already in a terminal states + * Errors when attempting to stop a job in a transitional or unknown job * * @param request stop ingestion job request specifying which job to stop * @throws NoSuchElementException when stop job request requests to stop a nonexistent job. - * @throws UnsupportedOperationException when job to be stopped is in an unknown status + * @throws UnsupportedOperationException when job to be stopped is in an unsupported status * @throws InvalidProtocolBufferException on error when constructing response protobuf */ public StopIngestionJobResponse stopJob(StopIngestionJobRequest request) @@ -155,15 +156,11 @@ public StopIngestionJobResponse stopJob(StopIngestionJobRequest request) // check job status is valid for stopping Job job = getJob.get(); JobStatus status = job.getStatus(); - if (status.equals(JobStatus.ABORTED) - || status.equals(JobStatus.ABORTING) - || status.equals(JobStatus.SUSPENDED) - || status.equals(JobStatus.SUSPENDING) - || status.equals(JobStatus.COMPLETED) - || status.equals(JobStatus.ERROR)) { - // do nothing - job is already stopped or stopping + if(JobStatus.getTerminalState().contains(status)) { + // do nothing - job is already stoped return StopIngestionJobResponse.newBuilder().build(); - } else if (status.equals(JobStatus.UNKNOWN)) { + } else if (JobStatus.getTransitionalStates().contains(status) || + status.equals(JobStatus.UNKNOWN)) { throw new UnsupportedOperationException( "Stopping a job with an unknown status is unsupported"); } diff --git a/core/src/test/java/feast/core/service/JobServiceTest.java b/core/src/test/java/feast/core/service/JobServiceTest.java index 79a062cdd33..2e51e9be8a4 100644 --- a/core/src/test/java/feast/core/service/JobServiceTest.java +++ b/core/src/test/java/feast/core/service/JobServiceTest.java @@ -47,6 +47,7 @@ import feast.core.model.Store; import feast.types.ValueProto.ValueType.Enum; import java.time.Instant; +import java.util.ArrayList; import java.util.Arrays; import java.util.Date; import java.util.List; @@ -271,14 +272,10 @@ public void testStopJobForId() { @Test public void testStopAlreadyStop() { - // check that stop jobs does not trying to stop jobs that are - // not already stopped/stopping - List doNothingStatuses = - Arrays.asList( - JobStatus.SUSPENDED, JobStatus.SUSPENDING, - JobStatus.ABORTED, JobStatus.ABORTING, - JobStatus.COMPLETED, JobStatus.ABORTED); - + // check that stop jobs does not trying to stop jobs that are not already stopped + List doNothingStatuses = new ArrayList<>(); + doNothingStatuses.addAll(JobStatus.getTerminalState()); + JobStatus prevStatus = this.job.getStatus(); for (JobStatus status : doNothingStatuses) { this.job.setStatus(status); @@ -294,15 +291,21 @@ public void testStopAlreadyStop() { } @Test - public void testStopUnknownJobError() { + public void testUnsupportedError() { // check for UnsupportedOperationException when trying to stop jobs are - // in an in unknown state + // in an in unknown or in a transitional state JobStatus prevStatus = this.job.getStatus(); - this.job.setStatus(JobStatus.UNKNOWN); + List unsupportedStatuses = new ArrayList<>(); + unsupportedStatuses.addAll(JobStatus.getTransitionalStates()); + unsupportedStatuses.add(JobStatus.UNKNOWN); + + for (JobStatus status : unsupportedStatuses) { + this.job.setStatus(status); - StopIngestionJobRequest request = + StopIngestionJobRequest request = StopIngestionJobRequest.newBuilder().setId(this.job.getId()).build(); - this.tryStopJob(request, true); + this.tryStopJob(request, true); + } this.job.setStatus(prevStatus); } From f4711cde75d6e0ae1802bae64fb9dd6c9d4b1a24 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Fri, 20 Mar 2020 11:46:19 +0800 Subject: [PATCH 14/66] Moved conversion of JobStatus to IngestionJobStatus proto to JobStatus. --- core/src/main/java/feast/core/model/Job.java | 14 +--------- .../main/java/feast/core/model/JobStatus.java | 26 +++++++++++++++++++ 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/feast/core/model/Job.java b/core/src/main/java/feast/core/model/Job.java index 376e5dc694e..6c6f20ed72b 100644 --- a/core/src/main/java/feast/core/model/Job.java +++ b/core/src/main/java/feast/core/model/Job.java @@ -124,18 +124,6 @@ public String getSinkName() { * @return Ingestion Job proto derieved from the given job */ public IngestionJobProto.IngestionJob toIngestionProto() throws InvalidProtocolBufferException { - // maps job models job status to ingestion job status - Map statusMap = - Map.of( - JobStatus.UNKNOWN, IngestionJobProto.IngestionJobStatus.UNKNOWN, - JobStatus.PENDING, IngestionJobProto.IngestionJobStatus.PENDING, - JobStatus.RUNNING, IngestionJobProto.IngestionJobStatus.RUNNING, - JobStatus.COMPLETED, IngestionJobProto.IngestionJobStatus.COMPLETED, - JobStatus.ABORTING, IngestionJobProto.IngestionJobStatus.ABORTING, - JobStatus.ABORTED, IngestionJobProto.IngestionJobStatus.ABORTED, - JobStatus.ERROR, IngestionJobProto.IngestionJobStatus.ERROR, - JobStatus.SUSPENDING, IngestionJobProto.IngestionJobStatus.SUSPENDING, - JobStatus.SUSPENDED, IngestionJobProto.IngestionJobStatus.SUSPENDED); // convert featuresets of job to protos List featureSetProtos = new ArrayList<>(); @@ -148,7 +136,7 @@ public IngestionJobProto.IngestionJob toIngestionProto() throws InvalidProtocolB IngestionJobProto.IngestionJob.newBuilder() .setId(this.getId()) .setExternalId(this.getExtId()) - .setStatus(statusMap.get(this.getStatus())) + .setStatus(this.getStatus().toIngestionProto()) .addAllFeatureSets(featureSetProtos) .setSource(this.getSource().toProto()) .setStore(this.getStore().toProto()) diff --git a/core/src/main/java/feast/core/model/JobStatus.java b/core/src/main/java/feast/core/model/JobStatus.java index 5bea208beb6..f0575248eb7 100644 --- a/core/src/main/java/feast/core/model/JobStatus.java +++ b/core/src/main/java/feast/core/model/JobStatus.java @@ -19,6 +19,10 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.Map; + +import feast.core.IngestionJobProto; +import feast.core.IngestionJobProto.IngestionJobStatus; public enum JobStatus { /** Job status is not known. */ @@ -78,4 +82,26 @@ public static Collection getTerminalState() { public static final Collection getTransitionalStates() { return TRANSITIONAL_STATES; } + + /** + * Convert a Job Status to Ingestion Job Status proto + * + * @return IngestionJobStatus proto derieved from this job status + */ + public IngestionJobStatus toIngestionProto() { + // maps job models job status to ingestion job status + Map statusMap = + Map.of( + JobStatus.UNKNOWN, IngestionJobStatus.UNKNOWN, + JobStatus.PENDING, IngestionJobStatus.PENDING, + JobStatus.RUNNING, IngestionJobStatus.RUNNING, + JobStatus.COMPLETED, IngestionJobStatus.COMPLETED, + JobStatus.ABORTING, IngestionJobStatus.ABORTING, + JobStatus.ABORTED, IngestionJobStatus.ABORTED, + JobStatus.ERROR, IngestionJobStatus.ERROR, + JobStatus.SUSPENDING, IngestionJobStatus.SUSPENDING, + JobStatus.SUSPENDED, IngestionJobStatus.SUSPENDED); + return statusMap.get(this); + } + } From fe744eabac7d579dbac0476fe3c741d74eeb3a43 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Fri, 20 Mar 2020 12:26:57 +0800 Subject: [PATCH 15/66] Make findFeatureSet() match only one featureset. Limit findFeatureSets() as feature set references as composite keys should match one and only one featureset. --- .../java/feast/core/service/JobService.java | 42 ++++++++++++------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/feast/core/service/JobService.java b/core/src/main/java/feast/core/service/JobService.java index 454bd346bc8..346c449be8f 100644 --- a/core/src/main/java/feast/core/service/JobService.java +++ b/core/src/main/java/feast/core/service/JobService.java @@ -30,6 +30,7 @@ import feast.core.model.Job; import feast.core.model.JobStatus; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; @@ -107,11 +108,11 @@ public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest request) if (filter.hasFeatureSetReference()) { // find a matching featureset for reference FeatureSetReference fsReference = filter.getFeatureSetReference(); - List matchFeatureSets = this.findFeatureSets(fsReference); - Collection jobs = this.jobRepository.findByFeatureSetIn(matchFeatureSets); + FeatureSet featureSet = this.findFeatureSet(fsReference); + Collection matchingJobs = this.jobRepository.findByFeatureSetIn(Arrays.asList(featureSet)); List jobIds = - jobs.stream() + matchingJobs.stream() .map( job -> { return job.getId(); @@ -174,28 +175,32 @@ public StopIngestionJobResponse stopJob(StopIngestionJobRequest request) /* Private Utility Methods */ /** - * Finds & returns featuresets matching the given feature set refererence + * Finds & returns the featureset matching the given feature set refererence. + * The feature set reference provide should match one and only one featureset. * - * @param fsReference FeatureSetReference that specifies which featuresets to match - * @throws UnsupportedOperationException fsReference given is unsupported. - * @return Returns a list of matching featuresets + * @param fsReference FeatureSetReference that specifies matching criteria + * @throws NoSuchElementException when reference given matches either matches + * none of the featuresets or matches multiple featureses + * @throws UnsupportedOperationException reference given is unsupported. + * @return Returns matching featureset */ - private List findFeatureSets(FeatureSetReference fsReference) - throws UnsupportedOperationException { + private FeatureSet findFeatureSet(FeatureSetReference fsReference) + throws NoSuchElementException, UnsupportedOperationException { + // match featuresets using contents of featureset reference String fsName = fsReference.getName(); String fsProject = fsReference.getProject(); Integer fsVersion = fsReference.getVersion(); - List featureSets = new ArrayList<>(); + List matchingFeatureSets = new ArrayList<>(); if (fsName != "" && fsProject != "" && fsVersion != 0) { - featureSets.add( + matchingFeatureSets.add( this.featureSetRepository.findFeatureSetByNameAndProject_NameAndVersion( fsName, fsProject, fsVersion)); } else if (fsName != "" && fsProject != "") { - featureSets.addAll(this.featureSetRepository.findAllByNameAndProject_Name(fsName, fsProject)); + matchingFeatureSets.addAll(this.featureSetRepository.findAllByNameAndProject_Name(fsName, fsProject)); } else if (fsName != "" && fsVersion != 0) { - featureSets.addAll(this.featureSetRepository.findAllByNameAndVersion(fsName, fsVersion)); + matchingFeatureSets.addAll(this.featureSetRepository.findAllByNameAndVersion(fsName, fsVersion)); } else { throw new UnsupportedOperationException( String.format( @@ -203,8 +208,17 @@ private List findFeatureSets(FeatureSetReference fsReference) + "(name: '%s', project: '%s', version: '%d')", fsName, fsProject, fsVersion)); } + + // check no. of matching featuresets + // featureset reference should match one and only one featureset + if (matchingFeatureSets.size() != 1) { + throw new NoSuchElementException( + String.format("Featureset Reference should match only one featureset:" + + "(name: '%s', project: '%s', version: '%d')", + fsName, fsProject, fsVersion)); + } - return featureSets; + return matchingFeatureSets.get(0); } private Set mergeResults(Set results, Collection newResults) { From a79f8b4ec9024f79d966502395c3074a00c0ae10 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Fri, 20 Mar 2020 16:04:59 +0800 Subject: [PATCH 16/66] Added matchFeatureSets() to SpecService to match Feature Sets from References This commit is adds temporary support for FeatureSetReference to SpecService via listFeatureSets(). In the future, this should merged together with listFeatureSets() as their functionality is almost the same. --- .../java/feast/core/service/SpecService.java | 47 ++++++++++++++++--- .../feast/core/service/SpecServiceTest.java | 13 +++++ 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/feast/core/service/SpecService.java b/core/src/main/java/feast/core/service/SpecService.java index 8fec6ac5112..ccbaa92380b 100644 --- a/core/src/main/java/feast/core/service/SpecService.java +++ b/core/src/main/java/feast/core/service/SpecService.java @@ -19,8 +19,17 @@ import static feast.core.validators.Matchers.checkValidCharacters; import static feast.core.validators.Matchers.checkValidCharactersAllowAsterisk; +import java.util.ArrayList; +import java.util.List; + import com.google.common.collect.Ordering; import com.google.protobuf.InvalidProtocolBufferException; + +import org.apache.commons.lang3.StringUtils; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + import feast.core.CoreServiceProto.ApplyFeatureSetResponse; import feast.core.CoreServiceProto.ApplyFeatureSetResponse.Status; import feast.core.CoreServiceProto.GetFeatureSetRequest; @@ -33,6 +42,7 @@ import feast.core.CoreServiceProto.UpdateStoreRequest; import feast.core.CoreServiceProto.UpdateStoreResponse; import feast.core.FeatureSetProto; +import feast.core.FeatureSetReferenceProto.FeatureSetReference; import feast.core.SourceProto; import feast.core.StoreProto; import feast.core.StoreProto.Store.Subscription; @@ -45,13 +55,7 @@ import feast.core.model.Source; import feast.core.model.Store; import feast.core.validators.FeatureSetValidator; -import java.util.ArrayList; -import java.util.List; import lombok.extern.slf4j.Slf4j; -import org.apache.commons.lang3.StringUtils; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.stereotype.Service; -import org.springframework.transaction.annotation.Transactional; /** * Facilitates management of specs within the Feast registry. This includes getting existing specs @@ -131,6 +135,37 @@ public GetFeatureSetResponse getFeatureSet(GetFeatureSetRequest request) return GetFeatureSetResponse.newBuilder().setFeatureSet(featureSet.toProto()).build(); } + /** + * Finds & returns the featuresets matching the given feature set reference. + * TODO: merge with {@link #listFeatureSets(feast.core.CoreServiceProto.ListFeatureSetsRequest.Filter)} + * as they are very similar. + * + * @param fsReference FeatureSetReference that specifies matching criteria + * @throws UnsupportedOperationException reference given is unsupported. + * @throws InvalidProtocolBufferException on error when constructing response protobuf + * @return ListFeatureSetsRequest with the matching featuresets + */ + public ListFeatureSetsResponse matchFeatureSets(FeatureSetReference fsReference) + throws InvalidProtocolBufferException { + + // match featuresets using contents of featureset reference + String fsName = fsReference.getName(); + String fsProject = fsReference.getProject(); + Integer fsVersion = fsReference.getVersion(); + + // construct list featureset request filter using feature set reference + // for proto3, default value for missing values: + // - numeric values (ie int) is zero + // - strings is empty string + ListFeatureSetsRequest.Filter filter = ListFeatureSetsRequest.Filter.newBuilder() + .setFeatureSetName((fsName != "") ? fsName : "*") + .setProject((fsProject != "") ? fsProject : "*") + .setFeatureSetVersion((fsVersion != 0) ? fsVersion.toString() : "*") + .build(); + + return this.listFeatureSets(filter); + } + /** * Return a list of feature sets matching the feature set name, version, and project provided in * the filter. All fields are requried. Use '*' for all three arguments in order to return all diff --git a/core/src/test/java/feast/core/service/SpecServiceTest.java b/core/src/test/java/feast/core/service/SpecServiceTest.java index 43a66135dce..adc14b2cf4e 100644 --- a/core/src/test/java/feast/core/service/SpecServiceTest.java +++ b/core/src/test/java/feast/core/service/SpecServiceTest.java @@ -41,6 +41,7 @@ import feast.core.FeatureSetProto.FeatureSetSpec; import feast.core.FeatureSetProto.FeatureSetStatus; import feast.core.FeatureSetProto.FeatureSpec; +import feast.core.FeatureSetReferenceProto.FeatureSetReference; import feast.core.SourceProto.KafkaSourceConfig; import feast.core.SourceProto.SourceType; import feast.core.StoreProto; @@ -832,4 +833,16 @@ private Store newDummyStore(String name) { store.setConfig(RedisConfig.newBuilder().setPort(6379).build().toByteArray()); return store; } + + @Test + public void shouldMatchFeatureSetGivenFeatureSetReference() throws InvalidProtocolBufferException{ + FeatureSetReference fsReference = FeatureSetReference.newBuilder() + .setName("f1") + .setProject("project1") + .setVersion(1) + .build(); + ListFeatureSetsResponse response = this.specService.matchFeatureSets(fsReference); + FeatureSet featureSet = FeatureSet.fromProto(response.getFeatureSets(0)); + assertEquals(featureSet, this.featureSets.get(0)); + } } From 5b2592996902726bca85ef1ab67c603625b8a1ea Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Fri, 20 Mar 2020 16:09:08 +0800 Subject: [PATCH 17/66] Refactor JobService listJobs() to use SpecService to provide featureset matching --- .../java/feast/core/service/JobService.java | 99 ++++++------------- .../feast/core/service/JobServiceTest.java | 94 ++++++++++++------ 2 files changed, 93 insertions(+), 100 deletions(-) diff --git a/core/src/main/java/feast/core/service/JobService.java b/core/src/main/java/feast/core/service/JobService.java index 346c449be8f..5af19aabb32 100644 --- a/core/src/main/java/feast/core/service/JobService.java +++ b/core/src/main/java/feast/core/service/JobService.java @@ -16,19 +16,6 @@ */ package feast.core.service; -import com.google.protobuf.InvalidProtocolBufferException; -import feast.core.CoreServiceProto.ListIngestionJobsRequest; -import feast.core.CoreServiceProto.ListIngestionJobsResponse; -import feast.core.CoreServiceProto.StopIngestionJobRequest; -import feast.core.CoreServiceProto.StopIngestionJobResponse; -import feast.core.FeatureSetReferenceProto.FeatureSetReference; -import feast.core.IngestionJobProto; -import feast.core.dao.FeatureSetRepository; -import feast.core.dao.JobRepository; -import feast.core.job.JobManager; -import feast.core.model.FeatureSet; -import feast.core.model.Job; -import feast.core.model.JobStatus; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -40,23 +27,40 @@ import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; + +import com.google.protobuf.InvalidProtocolBufferException; + import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; +import feast.core.CoreServiceProto.GetFeatureSetResponse; +import feast.core.CoreServiceProto.ListFeatureSetsResponse; +import feast.core.CoreServiceProto.ListIngestionJobsRequest; +import feast.core.CoreServiceProto.ListIngestionJobsResponse; +import feast.core.CoreServiceProto.StopIngestionJobRequest; +import feast.core.CoreServiceProto.StopIngestionJobResponse; +import feast.core.FeatureSetReferenceProto.FeatureSetReference; +import feast.core.IngestionJobProto; +import feast.core.dao.JobRepository; +import feast.core.job.JobManager; +import feast.core.model.FeatureSet; +import feast.core.model.Job; +import feast.core.model.JobStatus; + /** Defines a Job Managemenent Service that allows users to manage feast ingestion jobs. */ @Service public class JobService { private JobRepository jobRepository; - private FeatureSetRepository featureSetRepository; + private SpecService specService; private Map jobManagers; @Autowired public JobService( JobRepository jobRepository, - FeatureSetRepository featureSetRepository, + SpecService specService, List jobManagerList) { this.jobRepository = jobRepository; - this.featureSetRepository = featureSetRepository; + this.specService = specService; this.jobManagers = new HashMap<>(); for (JobManager manager : jobManagerList) { @@ -74,7 +78,7 @@ public JobService( * @return list ingestion jobs response */ public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest request) - throws UnsupportedOperationException, InvalidProtocolBufferException { + throws InvalidProtocolBufferException { // filter jobs based on request filter ListIngestionJobsRequest.Filter filter = request.getFilter(); Set matchingJobIds = new HashSet<>(); @@ -106,11 +110,17 @@ public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest request) matchingJobIds = this.mergeResults(matchingJobIds, jobIds); } if (filter.hasFeatureSetReference()) { - // find a matching featureset for reference + // find a matching featuresets for reference FeatureSetReference fsReference = filter.getFeatureSetReference(); - FeatureSet featureSet = this.findFeatureSet(fsReference); - Collection matchingJobs = this.jobRepository.findByFeatureSetIn(Arrays.asList(featureSet)); - + ListFeatureSetsResponse response = this.specService.matchFeatureSets(fsReference); + List featureSets = response.getFeatureSetsList().stream() + .map(fsProto -> { + return FeatureSet.fromProto(fsProto); + }).collect(Collectors.toList()); + + + // find jobs for the matching featuresets + Collection matchingJobs = this.jobRepository.findByFeatureSetIn(featureSets); List jobIds = matchingJobs.stream() .map( @@ -174,53 +184,6 @@ public StopIngestionJobResponse stopJob(StopIngestionJobRequest request) } /* Private Utility Methods */ - /** - * Finds & returns the featureset matching the given feature set refererence. - * The feature set reference provide should match one and only one featureset. - * - * @param fsReference FeatureSetReference that specifies matching criteria - * @throws NoSuchElementException when reference given matches either matches - * none of the featuresets or matches multiple featureses - * @throws UnsupportedOperationException reference given is unsupported. - * @return Returns matching featureset - */ - private FeatureSet findFeatureSet(FeatureSetReference fsReference) - throws NoSuchElementException, UnsupportedOperationException { - - // match featuresets using contents of featureset reference - String fsName = fsReference.getName(); - String fsProject = fsReference.getProject(); - Integer fsVersion = fsReference.getVersion(); - - List matchingFeatureSets = new ArrayList<>(); - if (fsName != "" && fsProject != "" && fsVersion != 0) { - matchingFeatureSets.add( - this.featureSetRepository.findFeatureSetByNameAndProject_NameAndVersion( - fsName, fsProject, fsVersion)); - } else if (fsName != "" && fsProject != "") { - matchingFeatureSets.addAll(this.featureSetRepository.findAllByNameAndProject_Name(fsName, fsProject)); - } else if (fsName != "" && fsVersion != 0) { - matchingFeatureSets.addAll(this.featureSetRepository.findAllByNameAndVersion(fsName, fsVersion)); - } else { - throw new UnsupportedOperationException( - String.format( - "Unsupported featureset refererence configuration: " - + "(name: '%s', project: '%s', version: '%d')", - fsName, fsProject, fsVersion)); - } - - // check no. of matching featuresets - // featureset reference should match one and only one featureset - if (matchingFeatureSets.size() != 1) { - throw new NoSuchElementException( - String.format("Featureset Reference should match only one featureset:" - + "(name: '%s', project: '%s', version: '%d')", - fsName, fsProject, fsVersion)); - } - - return matchingFeatureSets.get(0); - } - private Set mergeResults(Set results, Collection newResults) { if (results.size() <= 0) { // no existing results: copy over new results diff --git a/core/src/test/java/feast/core/service/JobServiceTest.java b/core/src/test/java/feast/core/service/JobServiceTest.java index 2e51e9be8a4..afbaa0b5153 100644 --- a/core/src/test/java/feast/core/service/JobServiceTest.java +++ b/core/src/test/java/feast/core/service/JobServiceTest.java @@ -24,6 +24,8 @@ import static org.mockito.MockitoAnnotations.initMocks; import com.google.protobuf.InvalidProtocolBufferException; + +import feast.core.CoreServiceProto.ListFeatureSetsResponse; import feast.core.CoreServiceProto.ListIngestionJobsRequest; import feast.core.CoreServiceProto.ListIngestionJobsResponse; import feast.core.CoreServiceProto.StopIngestionJobRequest; @@ -58,13 +60,14 @@ public class JobServiceTest { // mocks - @Mock private FeatureSetRepository featureSetRepository; @Mock private JobRepository jobRepository; @Mock private JobManager jobManager; + @Mock private SpecService specService; // fake models private Source dataSource; private Store dataStore; private FeatureSet featureSet; + private List fsReferences; private Job job; private IngestionJob ingestionJob; // test target @@ -101,27 +104,39 @@ public void setup() { } catch (InvalidProtocolBufferException e) { e.printStackTrace(); } + + this.fsReferences = this.newDummyFeatureSetReferences(); // setup mock objects - this.setupFeatureSetRepository(); + this.setupSpecService(); this.setupJobRepository(); this.setupJobManager(); // create test target this.jobService = new JobService( - this.jobRepository, this.featureSetRepository, Arrays.asList(this.jobManager)); + this.jobRepository, this.specService, Arrays.asList(this.jobManager)); } - // setup fake feature set repository - public void setupFeatureSetRepository() { - when(this.featureSetRepository.findFeatureSetByNameAndProject_NameAndVersion( - "food", "hunger", 2)) - .thenReturn(this.featureSet); - when(this.featureSetRepository.findAllByNameAndProject_Name("food", "hunger")) - .thenReturn(Arrays.asList(featureSet)); - when(this.featureSetRepository.findAllByNameAndVersion("food", 2)) - .thenReturn(Arrays.asList(featureSet)); + // setup fake spec service + public void setupSpecService() { + try { + ListFeatureSetsResponse response = ListFeatureSetsResponse.newBuilder() + .addFeatureSets(this.featureSet.toProto()) + .build(); + + when(this.specService.matchFeatureSets(this.fsReferences.get(0))) + .thenReturn(response); + + when(this.specService.matchFeatureSets(this.fsReferences.get(1))) + .thenReturn(response); + + when(this.specService.matchFeatureSets(this.fsReferences.get(0))) + .thenReturn(response); + } catch(InvalidProtocolBufferException e){ + e.printStackTrace(); + fail("Unexpected exception"); + } } // setup fake job repository @@ -167,6 +182,29 @@ private Job newDummyJob(String id, String name, JobStatus status) { Arrays.asList(this.featureSet), status); } + + private List newDummyFeatureSetReferences() { + return Arrays.asList( + // all provided: name, version and project + FeatureSetReference.newBuilder() + .setVersion(this.featureSet.getVersion()) + .setName(this.featureSet.getName()) + .setProject(this.featureSet.getProject().toString()) + .build(), + + // name and project + FeatureSetReference.newBuilder() + .setName(this.featureSet.getName()) + .setProject(this.featureSet.getProject().toString()) + .build(), + + // name and version + FeatureSetReference.newBuilder() + .setName(this.featureSet.getName()) + .setVersion(this.featureSet.getVersion()) + .build() + ); + } /* unit tests */ private ListIngestionJobsResponse tryListJobs(ListIngestionJobsRequest request) { @@ -203,35 +241,27 @@ public void testListJobsByStoreName() { @Test public void testListIngestionJobByFeatureSetReference() { // list job by feature set reference: name and version and project - FeatureSetReference fsReference = - FeatureSetReference.newBuilder() - .setVersion(this.featureSet.getVersion()) - .setName(this.featureSet.getName()) - .setProject(this.featureSet.getProject().toString()) - .build(); - ListIngestionJobsRequest.Filter filter = - ListIngestionJobsRequest.Filter.newBuilder().setId(this.job.getId()).build(); + ListIngestionJobsRequest.Filter filter = ListIngestionJobsRequest.Filter.newBuilder() + .setFeatureSetReference(this.fsReferences.get(0)) + .setId(this.job.getId()) + .build(); ListIngestionJobsRequest request = ListIngestionJobsRequest.newBuilder().setFilter(filter).build(); assertEquals(this.tryListJobs(request).getJobs(0), this.ingestionJob); // list job by feature set reference: name and version - fsReference = - FeatureSetReference.newBuilder() - .setName(this.featureSet.getName()) - .setProject(this.featureSet.getProject().toString()) - .build(); - filter = ListIngestionJobsRequest.Filter.newBuilder().setId(this.job.getId()).build(); + filter = ListIngestionJobsRequest.Filter.newBuilder() + .setFeatureSetReference(this.fsReferences.get(1)) + .setId(this.job.getId()) + .build(); request = ListIngestionJobsRequest.newBuilder().setFilter(filter).build(); assertEquals(this.tryListJobs(request).getJobs(0), this.ingestionJob); // list job by feature set reference: name and project - fsReference = - FeatureSetReference.newBuilder() - .setName(this.featureSet.getName()) - .setVersion(this.featureSet.getVersion()) - .build(); - filter = ListIngestionJobsRequest.Filter.newBuilder().setId(this.job.getId()).build(); + filter = ListIngestionJobsRequest.Filter.newBuilder() + .setFeatureSetReference(this.fsReferences.get(2)) + .setId(this.job.getId()) + .build(); request = ListIngestionJobsRequest.newBuilder().setFilter(filter).build(); assertEquals(this.tryListJobs(request).getJobs(0), this.ingestionJob); } From 1997bb11bcde5434ba445e8ce0bbe71413e2e751 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Fri, 20 Mar 2020 16:20:44 +0800 Subject: [PATCH 18/66] Revert findBy methods added to FeatureSetRepository as no longer used. JobService no longer depends on FeatureSetRepository directly, instead via SpecService --- core/src/main/java/feast/core/dao/FeatureSetRepository.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/core/src/main/java/feast/core/dao/FeatureSetRepository.java b/core/src/main/java/feast/core/dao/FeatureSetRepository.java index 0ec3bb6921c..3eba2108889 100644 --- a/core/src/main/java/feast/core/dao/FeatureSetRepository.java +++ b/core/src/main/java/feast/core/dao/FeatureSetRepository.java @@ -36,12 +36,6 @@ FeatureSet findFirstFeatureSetByNameLikeAndProject_NameOrderByVersionDesc( // find all feature sets and order by name and version List findAllByOrderByNameAscVersionAsc(); - // find all feature sets by name and project name - List findAllByNameAndProject_Name(String name, String projectName); - - // find all feature sets by name and version - List findAllByNameAndVersion(String name, Integer version); - // find all feature sets within a project and order by name and version List findAllByProject_NameOrderByNameAscVersionAsc(String project_name); From f320e3898d3f0405d3212d26590eaf3220b0f414 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Fri, 20 Mar 2020 22:45:52 +0800 Subject: [PATCH 19/66] Added restartJob() to JobManagers to restart ingestion/import jobs --- .../main/java/feast/core/job/JobManager.java | 11 ++++++++++ .../core/job/dataflow/DataflowJobManager.java | 22 +++++++++++++++++++ .../job/direct/DirectRunnerJobManager.java | 21 ++++++++++++++++++ 3 files changed, 54 insertions(+) diff --git a/core/src/main/java/feast/core/job/JobManager.java b/core/src/main/java/feast/core/job/JobManager.java index 99880cdb764..9005f56bdb9 100644 --- a/core/src/main/java/feast/core/job/JobManager.java +++ b/core/src/main/java/feast/core/job/JobManager.java @@ -51,6 +51,17 @@ public interface JobManager { */ void abortJob(String extId); + /** + * Restart an job. + * If job is an terminated state, will simply start the job. + * Might cause data to be lost during when restarting running jobs + * in some implementations. Refer to on docs the specific implementation. + * + * @param job job to restart + * @return the restarted job + */ + Job restartJob(Job job); + /** * Get status of a job given runner-specific job ID. * diff --git a/core/src/main/java/feast/core/job/dataflow/DataflowJobManager.java b/core/src/main/java/feast/core/job/dataflow/DataflowJobManager.java index f4df3d352a9..bb9a6c1fac6 100644 --- a/core/src/main/java/feast/core/job/dataflow/DataflowJobManager.java +++ b/core/src/main/java/feast/core/job/dataflow/DataflowJobManager.java @@ -152,6 +152,27 @@ public void abortJob(String dataflowJobId) { } } + /** + * Restart a restart dataflow job. + * Dataflow should ensure continuity between during the restart, so no data + * should be lost during the restart operation. + * + * @param job job to restart + * @return the restarted job + */ + @Override + public Job restartJob(Job job) { + JobStatus status = job.getStatus(); + if (JobStatus.getTerminalState().contains(status)) { + // job yet not running: just start job + return this.startJob(job); + } else { + // job is running - updating the job without changing the job has + // the effect of restarting the job + return this.updateJob(job); + } + } + /** * Get status of a dataflow job with given id and try to map it into Feast's JobStatus. * @@ -258,4 +279,5 @@ private String waitForJobToRun(DataflowPipelineJob pipelineResult) Thread.sleep(2000); } } + } diff --git a/core/src/main/java/feast/core/job/direct/DirectRunnerJobManager.java b/core/src/main/java/feast/core/job/direct/DirectRunnerJobManager.java index 08aeed1cc3a..f9ccf85a72d 100644 --- a/core/src/main/java/feast/core/job/direct/DirectRunnerJobManager.java +++ b/core/src/main/java/feast/core/job/direct/DirectRunnerJobManager.java @@ -156,6 +156,27 @@ public void abortJob(String extId) { public PipelineResult runPipeline(ImportOptions pipelineOptions) throws IOException { return ImportJob.runPipeline(pipelineOptions); } + + /** + * Restart a direct runner job. + * Note that some data will be temporarily lost during when restarting running + * direct runner jobs. See {#link {@link #updateJob(Job)} for more info. + * + * @param job job to restart + * @return the restarted job + */ + @Override + public Job restartJob(Job job) { + JobStatus status = job.getStatus(); + if (JobStatus.getTerminalState().contains(status)) { + // job yet not running: just start job + return this.startJob(job); + } else { + // job is running - updating the job without changing the job has + // the effect of restarting the job. + return this.updateJob(job); + } + } /** * Gets the state of the direct runner job. Direct runner jobs only have 2 states: RUNNING and From 0ec5afbb0e4fa63219b78bbd4c30a3d2427e820e Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 23 Mar 2020 10:15:28 +0800 Subject: [PATCH 20/66] Added restartJob() to JobService to restart ingestion jobs --- .../java/feast/core/service/JobService.java | 49 ++++++++++-- .../feast/core/service/JobServiceTest.java | 80 +++++++++++++++++-- 2 files changed, 117 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/feast/core/service/JobService.java b/core/src/main/java/feast/core/service/JobService.java index 5af19aabb32..99d7bd6ff3b 100644 --- a/core/src/main/java/feast/core/service/JobService.java +++ b/core/src/main/java/feast/core/service/JobService.java @@ -37,6 +37,8 @@ import feast.core.CoreServiceProto.ListFeatureSetsResponse; import feast.core.CoreServiceProto.ListIngestionJobsRequest; import feast.core.CoreServiceProto.ListIngestionJobsResponse; +import feast.core.CoreServiceProto.RestartIngestionJobRequest; +import feast.core.CoreServiceProto.RestartIngestionJobResponse; import feast.core.CoreServiceProto.StopIngestionJobRequest; import feast.core.CoreServiceProto.StopIngestionJobResponse; import feast.core.FeatureSetReferenceProto.FeatureSetReference; @@ -93,7 +95,6 @@ public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest request) if (job.isPresent()) { matchingJobIds.add(filter.getId()); } - } else { // multiple filters can apply together in an 'and' operation if (filter.getStoreName() != "") { @@ -106,7 +107,6 @@ public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest request) return job.getId(); }) .collect(Collectors.toList()); - matchingJobIds = this.mergeResults(matchingJobIds, jobIds); } if (filter.hasFeatureSetReference()) { @@ -143,12 +143,45 @@ public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest request) return ListIngestionJobsResponse.newBuilder().addAllJobs(ingestJobs).build(); } - // TODO: restart ingestion job + /** + * Restart (Aborts) the ingestion job matching the given restart request. + * + * @param request restart ingestion job request specifying which job to stop + * @throws NoSuchElementException when restart job request requests to restart a nonexistent job. + * @throws UnsupportedOperationException when job to be restarted is in an unsupported status + * @throws InvalidProtocolBufferException on error when constructing response protobuf + */ + public RestartIngestionJobResponse restartJob(RestartIngestionJobRequest request) + throws InvalidProtocolBufferException { + // check job exists + Optional getJob = this.jobRepository.findById(request.getId()); + if (getJob.isEmpty()) { + throw new NoSuchElementException( + "Attempted to stop nonexistent job with id: " + getJob.get().getId()); + } + + // check job status is valid for restarting + Job job = getJob.get(); + JobStatus status = job.getStatus(); + if (JobStatus.getTransitionalStates().contains(status) || + status.equals(JobStatus.UNKNOWN)) { + throw new UnsupportedOperationException( + "Restarting a job with a transitional or unknown status is unsupported"); + } + + // restart job with job manager + JobManager jobManager = this.jobManagers.get(job.getRunner()); + job = jobManager.restartJob(job); + + // TODO: update restart model + + return RestartIngestionJobResponse.newBuilder().build(); + } /** - * Stops (Aborts) the ingestion job matching the given request. + * Stops (Aborts) the ingestion job matching the given stop request. * Does nothing if the target job if already in a terminal states - * Errors when attempting to stop a job in a transitional or unknown job + * Does not support stopping a job in a transitional or unknown status * * @param request stop ingestion job request specifying which job to stop * @throws NoSuchElementException when stop job request requests to stop a nonexistent job. @@ -168,17 +201,19 @@ public StopIngestionJobResponse stopJob(StopIngestionJobRequest request) Job job = getJob.get(); JobStatus status = job.getStatus(); if(JobStatus.getTerminalState().contains(status)) { - // do nothing - job is already stoped + // do nothing - job is already stopped return StopIngestionJobResponse.newBuilder().build(); } else if (JobStatus.getTransitionalStates().contains(status) || status.equals(JobStatus.UNKNOWN)) { throw new UnsupportedOperationException( - "Stopping a job with an unknown status is unsupported"); + "Stopping a job with a transitional or unknown status is unsupported"); } // stop job with job manager JobManager jobManager = this.jobManagers.get(job.getRunner()); jobManager.abortJob(job.getExtId()); + + // TODO: update stop model return StopIngestionJobResponse.newBuilder().build(); } diff --git a/core/src/test/java/feast/core/service/JobServiceTest.java b/core/src/test/java/feast/core/service/JobServiceTest.java index afbaa0b5153..32edd2a8f33 100644 --- a/core/src/test/java/feast/core/service/JobServiceTest.java +++ b/core/src/test/java/feast/core/service/JobServiceTest.java @@ -19,6 +19,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.initMocks; @@ -28,6 +29,8 @@ import feast.core.CoreServiceProto.ListFeatureSetsResponse; import feast.core.CoreServiceProto.ListIngestionJobsRequest; import feast.core.CoreServiceProto.ListIngestionJobsResponse; +import feast.core.CoreServiceProto.RestartIngestionJobRequest; +import feast.core.CoreServiceProto.RestartIngestionJobResponse; import feast.core.CoreServiceProto.StopIngestionJobRequest; import feast.core.CoreServiceProto.StopIngestionJobResponse; import feast.core.FeatureSetProto.FeatureSetStatus; @@ -98,7 +101,7 @@ public void setup() { // fake featureset & job this.featureSet = this.newDummyFeatureSet("food", 2, "hunger"); - this.job = this.newDummyJob("job", "kafka-to-redis", JobStatus.PENDING); + this.job = this.newDummyJob("kafka-to-redis", "job-1111", JobStatus.PENDING); try { this.ingestionJob = this.job.toIngestionProto(); } catch (InvalidProtocolBufferException e) { @@ -151,6 +154,8 @@ public void setupJobRepository() { // TODO: setup fake job manager public void setupJobManager() { when(this.jobManager.getRunnerType()).thenReturn(Runner.DATAFLOW); + when(this.jobManager.restartJob(this.job)).thenReturn( + this.newDummyJob(this.job.getId(), this.job.getExtId(), JobStatus.PENDING)); } // dummy model constructorss @@ -172,10 +177,10 @@ private FeatureSet newDummyFeatureSet(String name, int version, String project) return fs; } - private Job newDummyJob(String id, String name, JobStatus status) { + private Job newDummyJob(String id, String extId, JobStatus status) { return new Job( id, - name, + extId, Runner.DATAFLOW.getName(), this.dataSource, this.dataStore, @@ -280,10 +285,11 @@ private StopIngestionJobResponse tryStopJob( if (expectError != true) { // unexpected exception e.printStackTrace(); - fail("Caught Unexpected exception"); + fail("Caught Unexpected exception trying to restart job"); } } + return response; } @@ -297,6 +303,8 @@ public void testStopJobForId() { this.tryStopJob(request, false); verify(this.jobManager).abortJob(this.job.getExtId()); + // TODO: check that for job status change in featureset source + this.job.setStatus(prevStatus); } @@ -321,7 +329,7 @@ public void testStopAlreadyStop() { } @Test - public void testUnsupportedError() { + public void testStopUnsupportedError() { // check for UnsupportedOperationException when trying to stop jobs are // in an in unknown or in a transitional state JobStatus prevStatus = this.job.getStatus(); @@ -339,4 +347,66 @@ public void testUnsupportedError() { this.job.setStatus(prevStatus); } + + // restart jobs + private RestartIngestionJobResponse tryRestartJob(RestartIngestionJobRequest request, boolean expectError) { + RestartIngestionJobResponse response = null; + try { + response = this.jobService.restartJob(request); + // expected exception, but none was thrown + if (expectError) { + fail("Expected exception, but none was thrown"); + } + } catch (Exception e) { + if (expectError != true) { + // unexpected exception + e.printStackTrace(); + fail("Caught Unexpected exception trying to stop job"); + } + } + + + return response; + } + + @Test + public void testRestartJobForId() { + JobStatus prevStatus = this.job.getStatus(); + + // restart running job + this.job.setStatus(JobStatus.RUNNING); + RestartIngestionJobRequest request = + RestartIngestionJobRequest.newBuilder().setId(this.job.getId()).build(); + this.tryRestartJob(request, false); + + // restart terminated job + this.job.setStatus(JobStatus.SUSPENDED); + request = RestartIngestionJobRequest.newBuilder().setId(this.job.getId()).build(); + this.tryRestartJob(request, false); + + verify(this.jobManager, times(2)).restartJob(this.job); + // TODO: check that for job status change in featureset source + + this.job.setStatus(prevStatus); + } + + @Test + public void testRestartUnsupportedError() { + // check for UnsupportedOperationException when trying to restart jobs are + // in an in unknown or in a transitional state + JobStatus prevStatus = this.job.getStatus(); + List unsupportedStatuses = new ArrayList<>(); + unsupportedStatuses.addAll(JobStatus.getTransitionalStates()); + unsupportedStatuses.add(JobStatus.UNKNOWN); + + for (JobStatus status : unsupportedStatuses) { + this.job.setStatus(status); + + RestartIngestionJobRequest request = + RestartIngestionJobRequest.newBuilder().setId(this.job.getId()).build(); + this.tryRestartJob(request, true); + } + + this.job.setStatus(prevStatus); + } } From 83cc4c7e8a66e0e2b4291e1890da1315b1b54f94 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 23 Mar 2020 10:25:06 +0800 Subject: [PATCH 21/66] Update job model (due to new extId) when restartJob() in JobService --- core/src/main/java/feast/core/service/JobService.java | 9 ++++++--- .../src/test/java/feast/core/service/JobServiceTest.java | 4 ++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/feast/core/service/JobService.java b/core/src/main/java/feast/core/service/JobService.java index 99d7bd6ff3b..06174b16a2e 100644 --- a/core/src/main/java/feast/core/service/JobService.java +++ b/core/src/main/java/feast/core/service/JobService.java @@ -28,6 +28,8 @@ import java.util.Set; import java.util.stream.Collectors; +import javax.transaction.Transactional; + import com.google.protobuf.InvalidProtocolBufferException; import org.springframework.beans.factory.annotation.Autowired; @@ -151,6 +153,7 @@ public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest request) * @throws UnsupportedOperationException when job to be restarted is in an unsupported status * @throws InvalidProtocolBufferException on error when constructing response protobuf */ + @Transactional public RestartIngestionJobResponse restartJob(RestartIngestionJobRequest request) throws InvalidProtocolBufferException { // check job exists @@ -173,7 +176,8 @@ public RestartIngestionJobResponse restartJob(RestartIngestionJobRequest request JobManager jobManager = this.jobManagers.get(job.getRunner()); job = jobManager.restartJob(job); - // TODO: update restart model + // update job model in job repository + this.jobRepository.saveAndFlush(job); return RestartIngestionJobResponse.newBuilder().build(); } @@ -188,6 +192,7 @@ public RestartIngestionJobResponse restartJob(RestartIngestionJobRequest request * @throws UnsupportedOperationException when job to be stopped is in an unsupported status * @throws InvalidProtocolBufferException on error when constructing response protobuf */ + @Transactional public StopIngestionJobResponse stopJob(StopIngestionJobRequest request) throws InvalidProtocolBufferException { // check job exists @@ -212,8 +217,6 @@ public StopIngestionJobResponse stopJob(StopIngestionJobRequest request) // stop job with job manager JobManager jobManager = this.jobManagers.get(job.getRunner()); jobManager.abortJob(job.getExtId()); - - // TODO: update stop model return StopIngestionJobResponse.newBuilder().build(); } diff --git a/core/src/test/java/feast/core/service/JobServiceTest.java b/core/src/test/java/feast/core/service/JobServiceTest.java index 32edd2a8f33..eb4a52220ac 100644 --- a/core/src/test/java/feast/core/service/JobServiceTest.java +++ b/core/src/test/java/feast/core/service/JobServiceTest.java @@ -385,8 +385,8 @@ public void testRestartJobForId() { this.tryRestartJob(request, false); verify(this.jobManager, times(2)).restartJob(this.job); - // TODO: check that for job status change in featureset source - + verify(this.jobRepository, times(2)).saveAndFlush(this.job); + this.job.setStatus(prevStatus); } From cf6a9c472dfddaade724ed8f2920ca5f311469e1 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 23 Mar 2020 11:01:22 +0800 Subject: [PATCH 22/66] Use assertThat() & equalTo() instead of assertEquals() in JobService --- .../main/java/feast/core/job/JobManager.java | 9 +- .../core/job/dataflow/DataflowJobManager.java | 10 +- .../job/direct/DirectRunnerJobManager.java | 13 ++- core/src/main/java/feast/core/model/Job.java | 1 - .../main/java/feast/core/model/JobStatus.java | 26 +++-- .../java/feast/core/service/JobService.java | 79 +++++++-------- .../java/feast/core/service/SpecService.java | 34 +++---- .../feast/core/service/JobServiceTest.java | 99 +++++++++---------- .../feast/core/service/SpecServiceTest.java | 10 +- 9 files changed, 128 insertions(+), 153 deletions(-) diff --git a/core/src/main/java/feast/core/job/JobManager.java b/core/src/main/java/feast/core/job/JobManager.java index 9005f56bdb9..eda211b7574 100644 --- a/core/src/main/java/feast/core/job/JobManager.java +++ b/core/src/main/java/feast/core/job/JobManager.java @@ -52,14 +52,13 @@ public interface JobManager { void abortJob(String extId); /** - * Restart an job. - * If job is an terminated state, will simply start the job. - * Might cause data to be lost during when restarting running jobs - * in some implementations. Refer to on docs the specific implementation. + * Restart an job. If job is an terminated state, will simply start the job. Might cause data to + * be lost during when restarting running jobs in some implementations. Refer to on docs the + * specific implementation. * * @param job job to restart * @return the restarted job - */ + */ Job restartJob(Job job); /** diff --git a/core/src/main/java/feast/core/job/dataflow/DataflowJobManager.java b/core/src/main/java/feast/core/job/dataflow/DataflowJobManager.java index bb9a6c1fac6..c2313d75ecc 100644 --- a/core/src/main/java/feast/core/job/dataflow/DataflowJobManager.java +++ b/core/src/main/java/feast/core/job/dataflow/DataflowJobManager.java @@ -153,9 +153,8 @@ public void abortJob(String dataflowJobId) { } /** - * Restart a restart dataflow job. - * Dataflow should ensure continuity between during the restart, so no data - * should be lost during the restart operation. + * Restart a restart dataflow job. Dataflow should ensure continuity between during the restart, + * so no data should be lost during the restart operation. * * @param job job to restart * @return the restarted job @@ -166,8 +165,8 @@ public Job restartJob(Job job) { if (JobStatus.getTerminalState().contains(status)) { // job yet not running: just start job return this.startJob(job); - } else { - // job is running - updating the job without changing the job has + } else { + // job is running - updating the job without changing the job has // the effect of restarting the job return this.updateJob(job); } @@ -279,5 +278,4 @@ private String waitForJobToRun(DataflowPipelineJob pipelineResult) Thread.sleep(2000); } } - } diff --git a/core/src/main/java/feast/core/job/direct/DirectRunnerJobManager.java b/core/src/main/java/feast/core/job/direct/DirectRunnerJobManager.java index f9ccf85a72d..9b3a8473e47 100644 --- a/core/src/main/java/feast/core/job/direct/DirectRunnerJobManager.java +++ b/core/src/main/java/feast/core/job/direct/DirectRunnerJobManager.java @@ -156,23 +156,22 @@ public void abortJob(String extId) { public PipelineResult runPipeline(ImportOptions pipelineOptions) throws IOException { return ImportJob.runPipeline(pipelineOptions); } - + /** - * Restart a direct runner job. - * Note that some data will be temporarily lost during when restarting running - * direct runner jobs. See {#link {@link #updateJob(Job)} for more info. + * Restart a direct runner job. Note that some data will be temporarily lost during when + * restarting running direct runner jobs. See {#link {@link #updateJob(Job)} for more info. * * @param job job to restart * @return the restarted job - */ + */ @Override public Job restartJob(Job job) { JobStatus status = job.getStatus(); if (JobStatus.getTerminalState().contains(status)) { // job yet not running: just start job return this.startJob(job); - } else { - // job is running - updating the job without changing the job has + } else { + // job is running - updating the job without changing the job has // the effect of restarting the job. return this.updateJob(job); } diff --git a/core/src/main/java/feast/core/model/Job.java b/core/src/main/java/feast/core/model/Job.java index 6c6f20ed72b..87e9ae4c286 100644 --- a/core/src/main/java/feast/core/model/Job.java +++ b/core/src/main/java/feast/core/model/Job.java @@ -21,7 +21,6 @@ import feast.core.IngestionJobProto; import java.util.ArrayList; import java.util.List; -import java.util.Map; import javax.persistence.CascadeType; import javax.persistence.Column; import javax.persistence.Entity; diff --git a/core/src/main/java/feast/core/model/JobStatus.java b/core/src/main/java/feast/core/model/JobStatus.java index f0575248eb7..38d6f819068 100644 --- a/core/src/main/java/feast/core/model/JobStatus.java +++ b/core/src/main/java/feast/core/model/JobStatus.java @@ -16,14 +16,12 @@ */ package feast.core.model; +import feast.core.IngestionJobProto.IngestionJobStatus; import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Map; -import feast.core.IngestionJobProto; -import feast.core.IngestionJobProto.IngestionJobStatus; - public enum JobStatus { /** Job status is not known. */ UNKNOWN, @@ -68,26 +66,25 @@ public enum JobStatus { public static Collection getTerminalState() { return TERMINAL_STATE; } - + private static final Collection TRANSITIONAL_STATES = Collections.unmodifiableList(Arrays.asList(PENDING, ABORTING, SUSPENDING)); /** - * Get Transitional Job Status states. - * Transitionals states are assigned to jobs that transitioning to a more - * stable state (ie SUSPENDED, ABORTED etc.) - * + * Get Transitional Job Status states. Transitionals states are assigned to jobs that + * transitioning to a more stable state (ie SUSPENDED, ABORTED etc.) + * * @return Collection of transitional Job Status states. - */ + */ public static final Collection getTransitionalStates() { return TRANSITIONAL_STATES; } - - /** - * Convert a Job Status to Ingestion Job Status proto - * + + /** + * Convert a Job Status to Ingestion Job Status proto + * * @return IngestionJobStatus proto derieved from this job status - */ + */ public IngestionJobStatus toIngestionProto() { // maps job models job status to ingestion job status Map statusMap = @@ -103,5 +100,4 @@ public IngestionJobStatus toIngestionProto() { JobStatus.SUSPENDED, IngestionJobStatus.SUSPENDED); return statusMap.get(this); } - } diff --git a/core/src/main/java/feast/core/service/JobService.java b/core/src/main/java/feast/core/service/JobService.java index 06174b16a2e..9d39902009a 100644 --- a/core/src/main/java/feast/core/service/JobService.java +++ b/core/src/main/java/feast/core/service/JobService.java @@ -16,26 +16,7 @@ */ package feast.core.service; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.NoSuchElementException; -import java.util.Optional; -import java.util.Set; -import java.util.stream.Collectors; - -import javax.transaction.Transactional; - import com.google.protobuf.InvalidProtocolBufferException; - -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.stereotype.Service; - -import feast.core.CoreServiceProto.GetFeatureSetResponse; import feast.core.CoreServiceProto.ListFeatureSetsResponse; import feast.core.CoreServiceProto.ListIngestionJobsRequest; import feast.core.CoreServiceProto.ListIngestionJobsResponse; @@ -50,6 +31,19 @@ import feast.core.model.FeatureSet; import feast.core.model.Job; import feast.core.model.JobStatus; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.NoSuchElementException; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; +import javax.transaction.Transactional; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Service; /** Defines a Job Managemenent Service that allows users to manage feast ingestion jobs. */ @Service @@ -60,11 +54,9 @@ public class JobService { @Autowired public JobService( - JobRepository jobRepository, - SpecService specService, - List jobManagerList) { + JobRepository jobRepository, SpecService specService, List jobManagerList) { this.jobRepository = jobRepository; - this.specService = specService; + this.specService = specService; this.jobManagers = new HashMap<>(); for (JobManager manager : jobManagerList) { @@ -115,16 +107,18 @@ public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest request) // find a matching featuresets for reference FeatureSetReference fsReference = filter.getFeatureSetReference(); ListFeatureSetsResponse response = this.specService.matchFeatureSets(fsReference); - List featureSets = response.getFeatureSetsList().stream() - .map(fsProto -> { - return FeatureSet.fromProto(fsProto); - }).collect(Collectors.toList()); - - + List featureSets = + response.getFeatureSetsList().stream() + .map( + fsProto -> { + return FeatureSet.fromProto(fsProto); + }) + .collect(Collectors.toList()); + // find jobs for the matching featuresets Collection matchingJobs = this.jobRepository.findByFeatureSetIn(featureSets); List jobIds = - matchingJobs.stream() + matchingJobs.stream() .map( job -> { return job.getId(); @@ -145,29 +139,28 @@ public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest request) return ListIngestionJobsResponse.newBuilder().addAllJobs(ingestJobs).build(); } - /** + /** * Restart (Aborts) the ingestion job matching the given restart request. * * @param request restart ingestion job request specifying which job to stop * @throws NoSuchElementException when restart job request requests to restart a nonexistent job. * @throws UnsupportedOperationException when job to be restarted is in an unsupported status * @throws InvalidProtocolBufferException on error when constructing response protobuf - */ + */ @Transactional public RestartIngestionJobResponse restartJob(RestartIngestionJobRequest request) - throws InvalidProtocolBufferException { + throws InvalidProtocolBufferException { // check job exists Optional getJob = this.jobRepository.findById(request.getId()); if (getJob.isEmpty()) { throw new NoSuchElementException( "Attempted to stop nonexistent job with id: " + getJob.get().getId()); } - + // check job status is valid for restarting Job job = getJob.get(); JobStatus status = job.getStatus(); - if (JobStatus.getTransitionalStates().contains(status) || - status.equals(JobStatus.UNKNOWN)) { + if (JobStatus.getTransitionalStates().contains(status) || status.equals(JobStatus.UNKNOWN)) { throw new UnsupportedOperationException( "Restarting a job with a transitional or unknown status is unsupported"); } @@ -175,7 +168,7 @@ public RestartIngestionJobResponse restartJob(RestartIngestionJobRequest request // restart job with job manager JobManager jobManager = this.jobManagers.get(job.getRunner()); job = jobManager.restartJob(job); - + // update job model in job repository this.jobRepository.saveAndFlush(job); @@ -183,9 +176,9 @@ public RestartIngestionJobResponse restartJob(RestartIngestionJobRequest request } /** - * Stops (Aborts) the ingestion job matching the given stop request. - * Does nothing if the target job if already in a terminal states - * Does not support stopping a job in a transitional or unknown status + * Stops (Aborts) the ingestion job matching the given stop request. Does nothing if the target + * job if already in a terminal states Does not support stopping a job in a transitional or + * unknown status * * @param request stop ingestion job request specifying which job to stop * @throws NoSuchElementException when stop job request requests to stop a nonexistent job. @@ -205,11 +198,11 @@ public StopIngestionJobResponse stopJob(StopIngestionJobRequest request) // check job status is valid for stopping Job job = getJob.get(); JobStatus status = job.getStatus(); - if(JobStatus.getTerminalState().contains(status)) { + if (JobStatus.getTerminalState().contains(status)) { // do nothing - job is already stopped return StopIngestionJobResponse.newBuilder().build(); - } else if (JobStatus.getTransitionalStates().contains(status) || - status.equals(JobStatus.UNKNOWN)) { + } else if (JobStatus.getTransitionalStates().contains(status) + || status.equals(JobStatus.UNKNOWN)) { throw new UnsupportedOperationException( "Stopping a job with a transitional or unknown status is unsupported"); } diff --git a/core/src/main/java/feast/core/service/SpecService.java b/core/src/main/java/feast/core/service/SpecService.java index ccbaa92380b..d211ac24f9a 100644 --- a/core/src/main/java/feast/core/service/SpecService.java +++ b/core/src/main/java/feast/core/service/SpecService.java @@ -19,17 +19,8 @@ import static feast.core.validators.Matchers.checkValidCharacters; import static feast.core.validators.Matchers.checkValidCharactersAllowAsterisk; -import java.util.ArrayList; -import java.util.List; - import com.google.common.collect.Ordering; import com.google.protobuf.InvalidProtocolBufferException; - -import org.apache.commons.lang3.StringUtils; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.stereotype.Service; -import org.springframework.transaction.annotation.Transactional; - import feast.core.CoreServiceProto.ApplyFeatureSetResponse; import feast.core.CoreServiceProto.ApplyFeatureSetResponse.Status; import feast.core.CoreServiceProto.GetFeatureSetRequest; @@ -55,7 +46,13 @@ import feast.core.model.Source; import feast.core.model.Store; import feast.core.validators.FeatureSetValidator; +import java.util.ArrayList; +import java.util.List; import lombok.extern.slf4j.Slf4j; +import org.apache.commons.lang3.StringUtils; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; /** * Facilitates management of specs within the Feast registry. This includes getting existing specs @@ -136,9 +133,9 @@ public GetFeatureSetResponse getFeatureSet(GetFeatureSetRequest request) } /** - * Finds & returns the featuresets matching the given feature set reference. - * TODO: merge with {@link #listFeatureSets(feast.core.CoreServiceProto.ListFeatureSetsRequest.Filter)} - * as they are very similar. + * Finds & returns the featuresets matching the given feature set reference. TODO: merge with + * {@link #listFeatureSets(feast.core.CoreServiceProto.ListFeatureSetsRequest.Filter)} as they are + * very similar. * * @param fsReference FeatureSetReference that specifies matching criteria * @throws UnsupportedOperationException reference given is unsupported. @@ -152,16 +149,17 @@ public ListFeatureSetsResponse matchFeatureSets(FeatureSetReference fsReference) String fsName = fsReference.getName(); String fsProject = fsReference.getProject(); Integer fsVersion = fsReference.getVersion(); - + // construct list featureset request filter using feature set reference // for proto3, default value for missing values: // - numeric values (ie int) is zero // - strings is empty string - ListFeatureSetsRequest.Filter filter = ListFeatureSetsRequest.Filter.newBuilder() - .setFeatureSetName((fsName != "") ? fsName : "*") - .setProject((fsProject != "") ? fsProject : "*") - .setFeatureSetVersion((fsVersion != 0) ? fsVersion.toString() : "*") - .build(); + ListFeatureSetsRequest.Filter filter = + ListFeatureSetsRequest.Filter.newBuilder() + .setFeatureSetName((fsName != "") ? fsName : "*") + .setProject((fsProject != "") ? fsProject : "*") + .setFeatureSetVersion((fsVersion != 0) ? fsVersion.toString() : "*") + .build(); return this.listFeatureSets(filter); } diff --git a/core/src/test/java/feast/core/service/JobServiceTest.java b/core/src/test/java/feast/core/service/JobServiceTest.java index eb4a52220ac..260455381e1 100644 --- a/core/src/test/java/feast/core/service/JobServiceTest.java +++ b/core/src/test/java/feast/core/service/JobServiceTest.java @@ -16,7 +16,8 @@ */ package feast.core.service; -import static org.junit.Assert.assertEquals; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.fail; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -25,7 +26,6 @@ import static org.mockito.MockitoAnnotations.initMocks; import com.google.protobuf.InvalidProtocolBufferException; - import feast.core.CoreServiceProto.ListFeatureSetsResponse; import feast.core.CoreServiceProto.ListIngestionJobsRequest; import feast.core.CoreServiceProto.ListIngestionJobsResponse; @@ -40,7 +40,6 @@ import feast.core.SourceProto.SourceType; import feast.core.StoreProto.Store.RedisConfig; import feast.core.StoreProto.Store.StoreType; -import feast.core.dao.FeatureSetRepository; import feast.core.dao.JobRepository; import feast.core.job.JobManager; import feast.core.job.Runner; @@ -107,7 +106,7 @@ public void setup() { } catch (InvalidProtocolBufferException e) { e.printStackTrace(); } - + this.fsReferences = this.newDummyFeatureSetReferences(); // setup mock objects @@ -117,26 +116,21 @@ public void setup() { // create test target this.jobService = - new JobService( - this.jobRepository, this.specService, Arrays.asList(this.jobManager)); + new JobService(this.jobRepository, this.specService, Arrays.asList(this.jobManager)); } // setup fake spec service public void setupSpecService() { try { - ListFeatureSetsResponse response = ListFeatureSetsResponse.newBuilder() - .addFeatureSets(this.featureSet.toProto()) - .build(); + ListFeatureSetsResponse response = + ListFeatureSetsResponse.newBuilder().addFeatureSets(this.featureSet.toProto()).build(); - when(this.specService.matchFeatureSets(this.fsReferences.get(0))) - .thenReturn(response); + when(this.specService.matchFeatureSets(this.fsReferences.get(0))).thenReturn(response); - when(this.specService.matchFeatureSets(this.fsReferences.get(1))) - .thenReturn(response); + when(this.specService.matchFeatureSets(this.fsReferences.get(1))).thenReturn(response); - when(this.specService.matchFeatureSets(this.fsReferences.get(0))) - .thenReturn(response); - } catch(InvalidProtocolBufferException e){ + when(this.specService.matchFeatureSets(this.fsReferences.get(0))).thenReturn(response); + } catch (InvalidProtocolBufferException e) { e.printStackTrace(); fail("Unexpected exception"); } @@ -154,8 +148,8 @@ public void setupJobRepository() { // TODO: setup fake job manager public void setupJobManager() { when(this.jobManager.getRunnerType()).thenReturn(Runner.DATAFLOW); - when(this.jobManager.restartJob(this.job)).thenReturn( - this.newDummyJob(this.job.getId(), this.job.getExtId(), JobStatus.PENDING)); + when(this.jobManager.restartJob(this.job)) + .thenReturn(this.newDummyJob(this.job.getId(), this.job.getExtId(), JobStatus.PENDING)); } // dummy model constructorss @@ -187,7 +181,7 @@ private Job newDummyJob(String id, String extId, JobStatus status) { Arrays.asList(this.featureSet), status); } - + private List newDummyFeatureSetReferences() { return Arrays.asList( // all provided: name, version and project @@ -207,8 +201,7 @@ private List newDummyFeatureSetReferences() { FeatureSetReference.newBuilder() .setName(this.featureSet.getName()) .setVersion(this.featureSet.getVersion()) - .build() - ); + .build()); } /* unit tests */ @@ -231,7 +224,7 @@ public void testListJobsById() { ListIngestionJobsRequest.Filter.newBuilder().setId(this.job.getId()).build(); ListIngestionJobsRequest request = ListIngestionJobsRequest.newBuilder().setFilter(filter).build(); - assertEquals(this.tryListJobs(request).getJobs(0), this.ingestionJob); + assertThat(this.tryListJobs(request).getJobs(0), equalTo(this.ingestionJob)); } @Test @@ -240,35 +233,38 @@ public void testListJobsByStoreName() { ListIngestionJobsRequest.Filter.newBuilder().setStoreName(this.dataStore.getName()).build(); ListIngestionJobsRequest request = ListIngestionJobsRequest.newBuilder().setFilter(filter).build(); - assertEquals(this.tryListJobs(request).getJobs(0), this.ingestionJob); + assertThat(this.tryListJobs(request).getJobs(0), equalTo(this.ingestionJob)); } @Test public void testListIngestionJobByFeatureSetReference() { // list job by feature set reference: name and version and project - ListIngestionJobsRequest.Filter filter = ListIngestionJobsRequest.Filter.newBuilder() - .setFeatureSetReference(this.fsReferences.get(0)) - .setId(this.job.getId()) - .build(); + ListIngestionJobsRequest.Filter filter = + ListIngestionJobsRequest.Filter.newBuilder() + .setFeatureSetReference(this.fsReferences.get(0)) + .setId(this.job.getId()) + .build(); ListIngestionJobsRequest request = ListIngestionJobsRequest.newBuilder().setFilter(filter).build(); - assertEquals(this.tryListJobs(request).getJobs(0), this.ingestionJob); + assertThat(this.tryListJobs(request).getJobs(0), equalTo(this.ingestionJob)); // list job by feature set reference: name and version - filter = ListIngestionJobsRequest.Filter.newBuilder() - .setFeatureSetReference(this.fsReferences.get(1)) - .setId(this.job.getId()) - .build(); + filter = + ListIngestionJobsRequest.Filter.newBuilder() + .setFeatureSetReference(this.fsReferences.get(1)) + .setId(this.job.getId()) + .build(); request = ListIngestionJobsRequest.newBuilder().setFilter(filter).build(); - assertEquals(this.tryListJobs(request).getJobs(0), this.ingestionJob); + assertThat(this.tryListJobs(request).getJobs(0), equalTo(this.ingestionJob)); // list job by feature set reference: name and project - filter = ListIngestionJobsRequest.Filter.newBuilder() - .setFeatureSetReference(this.fsReferences.get(2)) - .setId(this.job.getId()) - .build(); + filter = + ListIngestionJobsRequest.Filter.newBuilder() + .setFeatureSetReference(this.fsReferences.get(2)) + .setId(this.job.getId()) + .build(); request = ListIngestionJobsRequest.newBuilder().setFilter(filter).build(); - assertEquals(this.tryListJobs(request).getJobs(0), this.ingestionJob); + assertThat(this.tryListJobs(request).getJobs(0), equalTo(this.ingestionJob)); } // stop jobs @@ -289,7 +285,6 @@ private StopIngestionJobResponse tryStopJob( } } - return response; } @@ -303,8 +298,8 @@ public void testStopJobForId() { this.tryStopJob(request, false); verify(this.jobManager).abortJob(this.job.getExtId()); - // TODO: check that for job status change in featureset source - + // TODO: check that for job status change in featureset source + this.job.setStatus(prevStatus); } @@ -313,7 +308,7 @@ public void testStopAlreadyStop() { // check that stop jobs does not trying to stop jobs that are not already stopped List doNothingStatuses = new ArrayList<>(); doNothingStatuses.addAll(JobStatus.getTerminalState()); - + JobStatus prevStatus = this.job.getStatus(); for (JobStatus status : doNothingStatuses) { this.job.setStatus(status); @@ -336,20 +331,21 @@ public void testStopUnsupportedError() { List unsupportedStatuses = new ArrayList<>(); unsupportedStatuses.addAll(JobStatus.getTransitionalStates()); unsupportedStatuses.add(JobStatus.UNKNOWN); - + for (JobStatus status : unsupportedStatuses) { this.job.setStatus(status); StopIngestionJobRequest request = - StopIngestionJobRequest.newBuilder().setId(this.job.getId()).build(); + StopIngestionJobRequest.newBuilder().setId(this.job.getId()).build(); this.tryStopJob(request, true); } this.job.setStatus(prevStatus); } - + // restart jobs - private RestartIngestionJobResponse tryRestartJob(RestartIngestionJobRequest request, boolean expectError) { + private RestartIngestionJobResponse tryRestartJob( + RestartIngestionJobRequest request, boolean expectError) { RestartIngestionJobResponse response = null; try { response = this.jobService.restartJob(request); @@ -365,10 +361,9 @@ private RestartIngestionJobResponse tryRestartJob(RestartIngestionJobRequest req } } - return response; } - + @Test public void testRestartJobForId() { JobStatus prevStatus = this.job.getStatus(); @@ -378,7 +373,7 @@ public void testRestartJobForId() { RestartIngestionJobRequest request = RestartIngestionJobRequest.newBuilder().setId(this.job.getId()).build(); this.tryRestartJob(request, false); - + // restart terminated job this.job.setStatus(JobStatus.SUSPENDED); request = RestartIngestionJobRequest.newBuilder().setId(this.job.getId()).build(); @@ -389,7 +384,7 @@ public void testRestartJobForId() { this.job.setStatus(prevStatus); } - + @Test public void testRestartUnsupportedError() { // check for UnsupportedOperationException when trying to restart jobs are @@ -398,12 +393,12 @@ public void testRestartUnsupportedError() { List unsupportedStatuses = new ArrayList<>(); unsupportedStatuses.addAll(JobStatus.getTransitionalStates()); unsupportedStatuses.add(JobStatus.UNKNOWN); - + for (JobStatus status : unsupportedStatuses) { this.job.setStatus(status); RestartIngestionJobRequest request = - RestartIngestionJobRequest.newBuilder().setId(this.job.getId()).build(); + RestartIngestionJobRequest.newBuilder().setId(this.job.getId()).build(); this.tryRestartJob(request, true); } diff --git a/core/src/test/java/feast/core/service/SpecServiceTest.java b/core/src/test/java/feast/core/service/SpecServiceTest.java index adc14b2cf4e..18d2f6adf3c 100644 --- a/core/src/test/java/feast/core/service/SpecServiceTest.java +++ b/core/src/test/java/feast/core/service/SpecServiceTest.java @@ -835,12 +835,10 @@ private Store newDummyStore(String name) { } @Test - public void shouldMatchFeatureSetGivenFeatureSetReference() throws InvalidProtocolBufferException{ - FeatureSetReference fsReference = FeatureSetReference.newBuilder() - .setName("f1") - .setProject("project1") - .setVersion(1) - .build(); + public void shouldMatchFeatureSetGivenFeatureSetReference() + throws InvalidProtocolBufferException { + FeatureSetReference fsReference = + FeatureSetReference.newBuilder().setName("f1").setProject("project1").setVersion(1).build(); ListFeatureSetsResponse response = this.specService.matchFeatureSets(fsReference); FeatureSet featureSet = FeatureSet.fromProto(response.getFeatureSets(0)); assertEquals(featureSet, this.featureSets.get(0)); From 8088b33e0d8f5ba4dc61e7cdab9e0f7d31e760ce Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 23 Mar 2020 11:27:12 +0800 Subject: [PATCH 23/66] Revert "Use assertThat() & equalTo() instead of assertEquals() in JobService" due to failed tests This reverts commit bb3cf0ae1e7cfbde7a2f997cacc661d040c0a35f. --- .../java/feast/core/service/JobServiceTest.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/core/src/test/java/feast/core/service/JobServiceTest.java b/core/src/test/java/feast/core/service/JobServiceTest.java index 260455381e1..6ef22ad0501 100644 --- a/core/src/test/java/feast/core/service/JobServiceTest.java +++ b/core/src/test/java/feast/core/service/JobServiceTest.java @@ -16,8 +16,7 @@ */ package feast.core.service; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -224,7 +223,7 @@ public void testListJobsById() { ListIngestionJobsRequest.Filter.newBuilder().setId(this.job.getId()).build(); ListIngestionJobsRequest request = ListIngestionJobsRequest.newBuilder().setFilter(filter).build(); - assertThat(this.tryListJobs(request).getJobs(0), equalTo(this.ingestionJob)); + assertEquals(this.tryListJobs(request).getJobs(0), this.ingestionJob); } @Test @@ -233,7 +232,7 @@ public void testListJobsByStoreName() { ListIngestionJobsRequest.Filter.newBuilder().setStoreName(this.dataStore.getName()).build(); ListIngestionJobsRequest request = ListIngestionJobsRequest.newBuilder().setFilter(filter).build(); - assertThat(this.tryListJobs(request).getJobs(0), equalTo(this.ingestionJob)); + assertEquals(this.tryListJobs(request).getJobs(0), this.ingestionJob); } @Test @@ -246,7 +245,7 @@ public void testListIngestionJobByFeatureSetReference() { .build(); ListIngestionJobsRequest request = ListIngestionJobsRequest.newBuilder().setFilter(filter).build(); - assertThat(this.tryListJobs(request).getJobs(0), equalTo(this.ingestionJob)); + assertEquals(this.tryListJobs(request).getJobs(0), this.ingestionJob); // list job by feature set reference: name and version filter = @@ -255,7 +254,7 @@ public void testListIngestionJobByFeatureSetReference() { .setId(this.job.getId()) .build(); request = ListIngestionJobsRequest.newBuilder().setFilter(filter).build(); - assertThat(this.tryListJobs(request).getJobs(0), equalTo(this.ingestionJob)); + assertEquals(this.tryListJobs(request).getJobs(0), this.ingestionJob); // list job by feature set reference: name and project filter = @@ -264,7 +263,7 @@ public void testListIngestionJobByFeatureSetReference() { .setId(this.job.getId()) .build(); request = ListIngestionJobsRequest.newBuilder().setFilter(filter).build(); - assertThat(this.tryListJobs(request).getJobs(0), equalTo(this.ingestionJob)); + assertEquals(this.tryListJobs(request).getJobs(0), this.ingestionJob); } // stop jobs From bdc2803a8b2c8c0ddde37511e6d24d444e22d9eb Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 23 Mar 2020 11:45:58 +0800 Subject: [PATCH 24/66] =?UTF-8?q?Use=20hamcrest=E2=80=99s=20assertThat()?= =?UTF-8?q?=20and=20equalTo()=20instead=20of=20Junit's=20assertEquals?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../java/feast/core/service/JobServiceTest.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/core/src/test/java/feast/core/service/JobServiceTest.java b/core/src/test/java/feast/core/service/JobServiceTest.java index 6ef22ad0501..0644b55b1da 100644 --- a/core/src/test/java/feast/core/service/JobServiceTest.java +++ b/core/src/test/java/feast/core/service/JobServiceTest.java @@ -16,7 +16,8 @@ */ package feast.core.service; -import static org.junit.Assert.assertEquals; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.fail; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -223,7 +224,7 @@ public void testListJobsById() { ListIngestionJobsRequest.Filter.newBuilder().setId(this.job.getId()).build(); ListIngestionJobsRequest request = ListIngestionJobsRequest.newBuilder().setFilter(filter).build(); - assertEquals(this.tryListJobs(request).getJobs(0), this.ingestionJob); + assertThat(this.tryListJobs(request).getJobs(0), equalTo(this.ingestionJob)); } @Test @@ -232,7 +233,7 @@ public void testListJobsByStoreName() { ListIngestionJobsRequest.Filter.newBuilder().setStoreName(this.dataStore.getName()).build(); ListIngestionJobsRequest request = ListIngestionJobsRequest.newBuilder().setFilter(filter).build(); - assertEquals(this.tryListJobs(request).getJobs(0), this.ingestionJob); + assertThat(this.tryListJobs(request).getJobs(0), equalTo(this.ingestionJob)); } @Test @@ -245,7 +246,7 @@ public void testListIngestionJobByFeatureSetReference() { .build(); ListIngestionJobsRequest request = ListIngestionJobsRequest.newBuilder().setFilter(filter).build(); - assertEquals(this.tryListJobs(request).getJobs(0), this.ingestionJob); + assertThat(this.tryListJobs(request).getJobs(0), equalTo(this.ingestionJob)); // list job by feature set reference: name and version filter = @@ -254,7 +255,7 @@ public void testListIngestionJobByFeatureSetReference() { .setId(this.job.getId()) .build(); request = ListIngestionJobsRequest.newBuilder().setFilter(filter).build(); - assertEquals(this.tryListJobs(request).getJobs(0), this.ingestionJob); + assertThat(this.tryListJobs(request).getJobs(0), equalTo(this.ingestionJob)); // list job by feature set reference: name and project filter = @@ -263,7 +264,7 @@ public void testListIngestionJobByFeatureSetReference() { .setId(this.job.getId()) .build(); request = ListIngestionJobsRequest.newBuilder().setFilter(filter).build(); - assertEquals(this.tryListJobs(request).getJobs(0), this.ingestionJob); + assertThat(this.tryListJobs(request).getJobs(0), equalTo(this.ingestionJob)); } // stop jobs From fafcd11bc258d61779daaeb94ed115527231c710 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 23 Mar 2020 13:10:00 +0800 Subject: [PATCH 25/66] Hook up JobService list, stop, restart job methods to CoreServiceImpl. --- .../java/feast/core/grpc/CoreServiceImpl.java | 80 ++++++++++++++++++- 1 file changed, 79 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/feast/core/grpc/CoreServiceImpl.java b/core/src/main/java/feast/core/grpc/CoreServiceImpl.java index 661bbe24039..eebc6055473 100644 --- a/core/src/main/java/feast/core/grpc/CoreServiceImpl.java +++ b/core/src/main/java/feast/core/grpc/CoreServiceImpl.java @@ -30,21 +30,29 @@ import feast.core.CoreServiceProto.GetFeatureSetResponse; import feast.core.CoreServiceProto.ListFeatureSetsRequest; import feast.core.CoreServiceProto.ListFeatureSetsResponse; +import feast.core.CoreServiceProto.ListIngestionJobsRequest; +import feast.core.CoreServiceProto.ListIngestionJobsResponse; import feast.core.CoreServiceProto.ListProjectsRequest; import feast.core.CoreServiceProto.ListProjectsResponse; import feast.core.CoreServiceProto.ListStoresRequest; import feast.core.CoreServiceProto.ListStoresResponse; +import feast.core.CoreServiceProto.RestartIngestionJobRequest; +import feast.core.CoreServiceProto.RestartIngestionJobResponse; +import feast.core.CoreServiceProto.StopIngestionJobRequest; +import feast.core.CoreServiceProto.StopIngestionJobResponse; import feast.core.CoreServiceProto.UpdateStoreRequest; import feast.core.CoreServiceProto.UpdateStoreResponse; import feast.core.exception.RetrievalException; import feast.core.grpc.interceptors.MonitoringInterceptor; import feast.core.model.Project; import feast.core.service.AccessManagementService; +import feast.core.service.JobService; import feast.core.service.SpecService; import io.grpc.Status; import io.grpc.StatusRuntimeException; import io.grpc.stub.StreamObserver; import java.util.List; +import java.util.NoSuchElementException; import java.util.stream.Collectors; import lombok.extern.slf4j.Slf4j; import org.lognet.springboot.grpc.GRpcService; @@ -57,11 +65,16 @@ public class CoreServiceImpl extends CoreServiceImplBase { private SpecService specService; private AccessManagementService accessManagementService; + private JobService jobService; @Autowired - public CoreServiceImpl(SpecService specService, AccessManagementService accessManagementService) { + public CoreServiceImpl( + SpecService specService, + AccessManagementService accessManagementService, + JobService jobService) { this.specService = specService; this.accessManagementService = accessManagementService; + this.jobService = jobService; } @Override @@ -192,4 +205,69 @@ public void listProjects( Status.INTERNAL.withDescription(e.getMessage()).withCause(e).asRuntimeException()); } } + + @Override + public void listIngestionJobs( + ListIngestionJobsRequest request, + StreamObserver responseObserver) { + try { + ListIngestionJobsResponse response = this.jobService.listJobs(request); + responseObserver.onNext(response); + responseObserver.onCompleted(); + } catch (UnsupportedOperationException e) { // TODO: change to InvalidArgumentException + log.error("Recieved an invalid request on calling listIngestionJobs method:", e); + responseObserver.onError( + Status.INVALID_ARGUMENT.withDescription(e.getMessage()).withCause(e).asException()); + } catch (Exception e) { + log.error("Unexpected exception on calling listIngestionJobs method:", e); + responseObserver.onError( + Status.INTERNAL.withDescription(e.getMessage()).withCause(e).asRuntimeException()); + } + } + + @Override + public void restartIngestionJob( + RestartIngestionJobRequest request, + StreamObserver responseObserver) { + try { + RestartIngestionJobResponse response = this.jobService.restartJob(request); + responseObserver.onNext(response); + responseObserver.onCompleted(); + } catch (NoSuchElementException e) { + log.error( + "Attempted to restart an nonexistent job on calling restartIngestionJob method:", e); + responseObserver.onError( + Status.NOT_FOUND.withDescription(e.getMessage()).withCause(e).asException()); + } catch (UnsupportedOperationException e) { + log.error("Recieved an unsupported request on calling restartIngestionJob method:", e); + responseObserver.onError( + Status.FAILED_PRECONDITION.withDescription(e.getMessage()).withCause(e).asException()); + } catch (Exception e) { + log.error("Unexpected exception on calling restartIngestionJob method:", e); + responseObserver.onError( + Status.INTERNAL.withDescription(e.getMessage()).withCause(e).asRuntimeException()); + } + } + + @Override + public void stopIngestionJob( + StopIngestionJobRequest request, StreamObserver responseObserver) { + try { + StopIngestionJobResponse response = this.jobService.stopJob(request); + responseObserver.onNext(response); + responseObserver.onCompleted(); + } catch (NoSuchElementException e) { + log.error("Attempted to stop an nonexistent job on calling stopIngestionJob method:", e); + responseObserver.onError( + Status.NOT_FOUND.withDescription(e.getMessage()).withCause(e).asException()); + } catch (UnsupportedOperationException e) { + log.error("Recieved an unsupported request on calling stopIngestionJob method:", e); + responseObserver.onError( + Status.FAILED_PRECONDITION.withDescription(e.getMessage()).withCause(e).asException()); + } catch (Exception e) { + log.error("Unexpected exception on calling stopIngestionJob method:", e); + responseObserver.onError( + Status.INTERNAL.withDescription(e.getMessage()).withCause(e).asRuntimeException()); + } + } } From 7a97e1f6cb4e999b231d484fb6afb95be75ce0f7 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 23 Mar 2020 13:20:59 +0800 Subject: [PATCH 26/66] Throw InvalidArgumentException instead when calling listJobs() with invalid FeatureSetReference Throw InvalidArgumentException instead of UnsupportedOperationException for better semantics: UnsupportedOperationException should be reserved for operations that fail due to failed preconditions --- core/src/main/java/feast/core/grpc/CoreServiceImpl.java | 3 ++- core/src/main/java/feast/core/service/JobService.java | 2 +- core/src/main/java/feast/core/service/SpecService.java | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/feast/core/grpc/CoreServiceImpl.java b/core/src/main/java/feast/core/grpc/CoreServiceImpl.java index eebc6055473..42bc0ba23de 100644 --- a/core/src/main/java/feast/core/grpc/CoreServiceImpl.java +++ b/core/src/main/java/feast/core/grpc/CoreServiceImpl.java @@ -16,6 +16,7 @@ */ package feast.core.grpc; +import com.google.api.gax.rpc.InvalidArgumentException; import com.google.protobuf.InvalidProtocolBufferException; import feast.core.CoreServiceGrpc.CoreServiceImplBase; import feast.core.CoreServiceProto.ApplyFeatureSetRequest; @@ -214,7 +215,7 @@ public void listIngestionJobs( ListIngestionJobsResponse response = this.jobService.listJobs(request); responseObserver.onNext(response); responseObserver.onCompleted(); - } catch (UnsupportedOperationException e) { // TODO: change to InvalidArgumentException + } catch (InvalidArgumentException e) { log.error("Recieved an invalid request on calling listIngestionJobs method:", e); responseObserver.onError( Status.INVALID_ARGUMENT.withDescription(e.getMessage()).withCause(e).asException()); diff --git a/core/src/main/java/feast/core/service/JobService.java b/core/src/main/java/feast/core/service/JobService.java index 9d39902009a..acbefab6689 100644 --- a/core/src/main/java/feast/core/service/JobService.java +++ b/core/src/main/java/feast/core/service/JobService.java @@ -69,7 +69,7 @@ public JobService( * List Ingestion Jobs in feast matching the given request * * @param request list ingestion jobs request specifying which jobs to include - * @throws UnsupportedOperationException when given filter in a unsupported configuration + * @throws InvalidArgumentException when given filter in a unsupported configuration * @throws InvalidProtocolBufferException on error when constructing response protobuf * @return list ingestion jobs response */ diff --git a/core/src/main/java/feast/core/service/SpecService.java b/core/src/main/java/feast/core/service/SpecService.java index d211ac24f9a..ed9245ae359 100644 --- a/core/src/main/java/feast/core/service/SpecService.java +++ b/core/src/main/java/feast/core/service/SpecService.java @@ -138,7 +138,7 @@ public GetFeatureSetResponse getFeatureSet(GetFeatureSetRequest request) * very similar. * * @param fsReference FeatureSetReference that specifies matching criteria - * @throws UnsupportedOperationException reference given is unsupported. + * @throws InvalidArgumentException reference given is unsupported. * @throws InvalidProtocolBufferException on error when constructing response protobuf * @return ListFeatureSetsRequest with the matching featuresets */ From d277f931b928a0037fe39cecadd2599846c9a8b7 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 23 Mar 2020 16:24:32 +0800 Subject: [PATCH 27/66] Fixed typos in javadocs: InvalidArgumentException should be IllegalArgumentException --- core/src/main/java/feast/core/service/JobService.java | 2 +- core/src/main/java/feast/core/service/SpecService.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/feast/core/service/JobService.java b/core/src/main/java/feast/core/service/JobService.java index acbefab6689..eba902b14fc 100644 --- a/core/src/main/java/feast/core/service/JobService.java +++ b/core/src/main/java/feast/core/service/JobService.java @@ -69,7 +69,7 @@ public JobService( * List Ingestion Jobs in feast matching the given request * * @param request list ingestion jobs request specifying which jobs to include - * @throws InvalidArgumentException when given filter in a unsupported configuration + * @throws IllegalArgumentException when given filter in a unsupported configuration * @throws InvalidProtocolBufferException on error when constructing response protobuf * @return list ingestion jobs response */ diff --git a/core/src/main/java/feast/core/service/SpecService.java b/core/src/main/java/feast/core/service/SpecService.java index ed9245ae359..6d8e02fb695 100644 --- a/core/src/main/java/feast/core/service/SpecService.java +++ b/core/src/main/java/feast/core/service/SpecService.java @@ -138,7 +138,7 @@ public GetFeatureSetResponse getFeatureSet(GetFeatureSetRequest request) * very similar. * * @param fsReference FeatureSetReference that specifies matching criteria - * @throws InvalidArgumentException reference given is unsupported. + * @throws IllegalArgumentException reference given is unsupported. * @throws InvalidProtocolBufferException on error when constructing response protobuf * @return ListFeatureSetsRequest with the matching featuresets */ From 255946b4b04b5c84dcf4f1e69a18a29af5295645 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 23 Mar 2020 17:18:41 +0800 Subject: [PATCH 28/66] Fixed in findByFeatureSetsIn() query in JobRepository --- core/src/main/java/feast/core/dao/JobRepository.java | 2 +- core/src/main/java/feast/core/service/JobService.java | 2 +- core/src/test/java/feast/core/service/JobServiceTest.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/feast/core/dao/JobRepository.java b/core/src/main/java/feast/core/dao/JobRepository.java index f05ca457ac8..c61f3eacc05 100644 --- a/core/src/main/java/feast/core/dao/JobRepository.java +++ b/core/src/main/java/feast/core/dao/JobRepository.java @@ -35,5 +35,5 @@ public interface JobRepository extends JpaRepository { List findByStoreName(String storeName); // find jobs by featureset - List findByFeatureSetIn(List featureSets); + List findByFeatureSetsIn(List featureSets); } diff --git a/core/src/main/java/feast/core/service/JobService.java b/core/src/main/java/feast/core/service/JobService.java index eba902b14fc..6aa816cb08c 100644 --- a/core/src/main/java/feast/core/service/JobService.java +++ b/core/src/main/java/feast/core/service/JobService.java @@ -116,7 +116,7 @@ public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest request) .collect(Collectors.toList()); // find jobs for the matching featuresets - Collection matchingJobs = this.jobRepository.findByFeatureSetIn(featureSets); + Collection matchingJobs = this.jobRepository.findByFeatureSetsIn(featureSets); List jobIds = matchingJobs.stream() .map( diff --git a/core/src/test/java/feast/core/service/JobServiceTest.java b/core/src/test/java/feast/core/service/JobServiceTest.java index 0644b55b1da..7189dc90d4d 100644 --- a/core/src/test/java/feast/core/service/JobServiceTest.java +++ b/core/src/test/java/feast/core/service/JobServiceTest.java @@ -141,7 +141,7 @@ public void setupJobRepository() { when(this.jobRepository.findById(this.job.getId())).thenReturn(Optional.of(this.job)); when(this.jobRepository.findByStoreName(this.dataStore.getName())) .thenReturn(Arrays.asList(this.job)); - when(this.jobRepository.findByFeatureSetIn(Arrays.asList(this.featureSet))) + when(this.jobRepository.findByFeatureSetsIn(Arrays.asList(this.featureSet))) .thenReturn(Arrays.asList(this.job)); } From 09e9ddcaece474f3161c143957a0dd5c335de90b Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 23 Mar 2020 17:34:21 +0800 Subject: [PATCH 29/66] Renamed toIngestionProto() methods to toProto() to follow code convention --- core/src/main/java/feast/core/model/Job.java | 4 ++-- core/src/main/java/feast/core/model/JobStatus.java | 2 +- core/src/main/java/feast/core/service/JobService.java | 2 +- core/src/test/java/feast/core/service/JobServiceTest.java | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/feast/core/model/Job.java b/core/src/main/java/feast/core/model/Job.java index 87e9ae4c286..40b60026ebb 100644 --- a/core/src/main/java/feast/core/model/Job.java +++ b/core/src/main/java/feast/core/model/Job.java @@ -122,7 +122,7 @@ public String getSinkName() { * * @return Ingestion Job proto derieved from the given job */ - public IngestionJobProto.IngestionJob toIngestionProto() throws InvalidProtocolBufferException { + public IngestionJobProto.IngestionJob toProto() throws InvalidProtocolBufferException { // convert featuresets of job to protos List featureSetProtos = new ArrayList<>(); @@ -135,7 +135,7 @@ public IngestionJobProto.IngestionJob toIngestionProto() throws InvalidProtocolB IngestionJobProto.IngestionJob.newBuilder() .setId(this.getId()) .setExternalId(this.getExtId()) - .setStatus(this.getStatus().toIngestionProto()) + .setStatus(this.getStatus().toProto()) .addAllFeatureSets(featureSetProtos) .setSource(this.getSource().toProto()) .setStore(this.getStore().toProto()) diff --git a/core/src/main/java/feast/core/model/JobStatus.java b/core/src/main/java/feast/core/model/JobStatus.java index 38d6f819068..277a1870811 100644 --- a/core/src/main/java/feast/core/model/JobStatus.java +++ b/core/src/main/java/feast/core/model/JobStatus.java @@ -85,7 +85,7 @@ public static final Collection getTransitionalStates() { * * @return IngestionJobStatus proto derieved from this job status */ - public IngestionJobStatus toIngestionProto() { + public IngestionJobStatus toProto() { // maps job models job status to ingestion job status Map statusMap = Map.of( diff --git a/core/src/main/java/feast/core/service/JobService.java b/core/src/main/java/feast/core/service/JobService.java index 6aa816cb08c..0f1d703ea91 100644 --- a/core/src/main/java/feast/core/service/JobService.java +++ b/core/src/main/java/feast/core/service/JobService.java @@ -132,7 +132,7 @@ public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest request) List ingestJobs = new ArrayList<>(); for (String jobId : matchingJobIds) { Job job = this.jobRepository.findById(jobId).get(); - ingestJobs.add(job.toIngestionProto()); + ingestJobs.add(job.toProto()); } // pack jobs into response diff --git a/core/src/test/java/feast/core/service/JobServiceTest.java b/core/src/test/java/feast/core/service/JobServiceTest.java index 7189dc90d4d..47db0bd03c0 100644 --- a/core/src/test/java/feast/core/service/JobServiceTest.java +++ b/core/src/test/java/feast/core/service/JobServiceTest.java @@ -102,7 +102,7 @@ public void setup() { this.featureSet = this.newDummyFeatureSet("food", 2, "hunger"); this.job = this.newDummyJob("kafka-to-redis", "job-1111", JobStatus.PENDING); try { - this.ingestionJob = this.job.toIngestionProto(); + this.ingestionJob = this.job.toProto(); } catch (InvalidProtocolBufferException e) { e.printStackTrace(); } From c85da173ac559fedd3d59a5ce0ab2d8023b44cca Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 23 Mar 2020 17:51:48 +0800 Subject: [PATCH 30/66] Make JobService's listJobs() a transaction to prevent DB data race conditions --- .../java/feast/core/service/JobService.java | 24 ++++--------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/core/src/main/java/feast/core/service/JobService.java b/core/src/main/java/feast/core/service/JobService.java index 0f1d703ea91..d2446b0bf37 100644 --- a/core/src/main/java/feast/core/service/JobService.java +++ b/core/src/main/java/feast/core/service/JobService.java @@ -41,9 +41,9 @@ import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; -import javax.transaction.Transactional; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; /** Defines a Job Managemenent Service that allows users to manage feast ingestion jobs. */ @Service @@ -73,6 +73,7 @@ public JobService( * @throws InvalidProtocolBufferException on error when constructing response protobuf * @return list ingestion jobs response */ + @Transactional(readOnly = true) public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest request) throws InvalidProtocolBufferException { // filter jobs based on request filter @@ -94,13 +95,7 @@ public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest request) if (filter.getStoreName() != "") { // find jobs by name List jobs = this.jobRepository.findByStoreName(filter.getStoreName()); - List jobIds = - jobs.stream() - .map( - job -> { - return job.getId(); - }) - .collect(Collectors.toList()); + List jobIds = jobs.stream().map(Job::getId).collect(Collectors.toList()); matchingJobIds = this.mergeResults(matchingJobIds, jobIds); } if (filter.hasFeatureSetReference()) { @@ -109,21 +104,12 @@ public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest request) ListFeatureSetsResponse response = this.specService.matchFeatureSets(fsReference); List featureSets = response.getFeatureSetsList().stream() - .map( - fsProto -> { - return FeatureSet.fromProto(fsProto); - }) + .map(FeatureSet::fromProto) .collect(Collectors.toList()); // find jobs for the matching featuresets Collection matchingJobs = this.jobRepository.findByFeatureSetsIn(featureSets); - List jobIds = - matchingJobs.stream() - .map( - job -> { - return job.getId(); - }) - .collect(Collectors.toList()); + List jobIds = matchingJobs.stream().map(Job::getId).collect(Collectors.toList()); matchingJobIds = this.mergeResults(matchingJobIds, jobIds); } } From c876e4017291aae51d98dc4b1303eb5319143ef8 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 23 Mar 2020 21:32:03 +0800 Subject: [PATCH 31/66] Moved matchFeatureSets() in SpecService to private method in JobService --- .../java/feast/core/service/JobService.java | 25 +++++++++++++- .../java/feast/core/service/SpecService.java | 33 ------------------- .../feast/core/service/JobServiceTest.java | 33 +++++++++++++++++-- .../feast/core/service/SpecServiceTest.java | 11 ------- 4 files changed, 54 insertions(+), 48 deletions(-) diff --git a/core/src/main/java/feast/core/service/JobService.java b/core/src/main/java/feast/core/service/JobService.java index d2446b0bf37..0e007b368c5 100644 --- a/core/src/main/java/feast/core/service/JobService.java +++ b/core/src/main/java/feast/core/service/JobService.java @@ -17,6 +17,7 @@ package feast.core.service; import com.google.protobuf.InvalidProtocolBufferException; +import feast.core.CoreServiceProto.ListFeatureSetsRequest; import feast.core.CoreServiceProto.ListFeatureSetsResponse; import feast.core.CoreServiceProto.ListIngestionJobsRequest; import feast.core.CoreServiceProto.ListIngestionJobsResponse; @@ -101,7 +102,8 @@ public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest request) if (filter.hasFeatureSetReference()) { // find a matching featuresets for reference FeatureSetReference fsReference = filter.getFeatureSetReference(); - ListFeatureSetsResponse response = this.specService.matchFeatureSets(fsReference); + ListFeatureSetsResponse response = + this.specService.listFeatureSets(this.toListFeatureSetFilter(fsReference)); List featureSets = response.getFeatureSetsList().stream() .map(FeatureSet::fromProto) @@ -211,4 +213,25 @@ private Set mergeResults(Set results, Collection newResults) { } return results; } + + // converts feature set reference to a list feature set filter + private ListFeatureSetsRequest.Filter toListFeatureSetFilter(FeatureSetReference fsReference) { + // match featuresets using contents of featureset reference + String fsName = fsReference.getName(); + String fsProject = fsReference.getProject(); + Integer fsVersion = fsReference.getVersion(); + + // construct list featureset request filter using feature set reference + // for proto3, default value for missing values: + // - numeric values (ie int) is zero + // - strings is empty string + ListFeatureSetsRequest.Filter filter = + ListFeatureSetsRequest.Filter.newBuilder() + .setFeatureSetName((fsName != "") ? fsName : "*") + .setProject((fsProject != "") ? fsProject : "*") + .setFeatureSetVersion((fsVersion != 0) ? fsVersion.toString() : "*") + .build(); + + return filter; + } } diff --git a/core/src/main/java/feast/core/service/SpecService.java b/core/src/main/java/feast/core/service/SpecService.java index 6d8e02fb695..8fec6ac5112 100644 --- a/core/src/main/java/feast/core/service/SpecService.java +++ b/core/src/main/java/feast/core/service/SpecService.java @@ -33,7 +33,6 @@ import feast.core.CoreServiceProto.UpdateStoreRequest; import feast.core.CoreServiceProto.UpdateStoreResponse; import feast.core.FeatureSetProto; -import feast.core.FeatureSetReferenceProto.FeatureSetReference; import feast.core.SourceProto; import feast.core.StoreProto; import feast.core.StoreProto.Store.Subscription; @@ -132,38 +131,6 @@ public GetFeatureSetResponse getFeatureSet(GetFeatureSetRequest request) return GetFeatureSetResponse.newBuilder().setFeatureSet(featureSet.toProto()).build(); } - /** - * Finds & returns the featuresets matching the given feature set reference. TODO: merge with - * {@link #listFeatureSets(feast.core.CoreServiceProto.ListFeatureSetsRequest.Filter)} as they are - * very similar. - * - * @param fsReference FeatureSetReference that specifies matching criteria - * @throws IllegalArgumentException reference given is unsupported. - * @throws InvalidProtocolBufferException on error when constructing response protobuf - * @return ListFeatureSetsRequest with the matching featuresets - */ - public ListFeatureSetsResponse matchFeatureSets(FeatureSetReference fsReference) - throws InvalidProtocolBufferException { - - // match featuresets using contents of featureset reference - String fsName = fsReference.getName(); - String fsProject = fsReference.getProject(); - Integer fsVersion = fsReference.getVersion(); - - // construct list featureset request filter using feature set reference - // for proto3, default value for missing values: - // - numeric values (ie int) is zero - // - strings is empty string - ListFeatureSetsRequest.Filter filter = - ListFeatureSetsRequest.Filter.newBuilder() - .setFeatureSetName((fsName != "") ? fsName : "*") - .setProject((fsProject != "") ? fsProject : "*") - .setFeatureSetVersion((fsVersion != 0) ? fsVersion.toString() : "*") - .build(); - - return this.listFeatureSets(filter); - } - /** * Return a list of feature sets matching the feature set name, version, and project provided in * the filter. All fields are requried. Use '*' for all three arguments in order to return all diff --git a/core/src/test/java/feast/core/service/JobServiceTest.java b/core/src/test/java/feast/core/service/JobServiceTest.java index 47db0bd03c0..e798e6a94ec 100644 --- a/core/src/test/java/feast/core/service/JobServiceTest.java +++ b/core/src/test/java/feast/core/service/JobServiceTest.java @@ -26,6 +26,7 @@ import static org.mockito.MockitoAnnotations.initMocks; import com.google.protobuf.InvalidProtocolBufferException; +import feast.core.CoreServiceProto.ListFeatureSetsRequest; import feast.core.CoreServiceProto.ListFeatureSetsResponse; import feast.core.CoreServiceProto.ListIngestionJobsRequest; import feast.core.CoreServiceProto.ListIngestionJobsResponse; @@ -70,6 +71,7 @@ public class JobServiceTest { private Store dataStore; private FeatureSet featureSet; private List fsReferences; + private List listFilters; private Job job; private IngestionJob ingestionJob; // test target @@ -108,6 +110,7 @@ public void setup() { } this.fsReferences = this.newDummyFeatureSetReferences(); + this.listFilters = this.newDummyListRequestFilters(); // setup mock objects this.setupSpecService(); @@ -125,11 +128,11 @@ public void setupSpecService() { ListFeatureSetsResponse response = ListFeatureSetsResponse.newBuilder().addFeatureSets(this.featureSet.toProto()).build(); - when(this.specService.matchFeatureSets(this.fsReferences.get(0))).thenReturn(response); + when(this.specService.listFeatureSets(this.listFilters.get(0))).thenReturn(response); - when(this.specService.matchFeatureSets(this.fsReferences.get(1))).thenReturn(response); + when(this.specService.listFeatureSets(this.listFilters.get(1))).thenReturn(response); - when(this.specService.matchFeatureSets(this.fsReferences.get(0))).thenReturn(response); + when(this.specService.listFeatureSets(this.listFilters.get(2))).thenReturn(response); } catch (InvalidProtocolBufferException e) { e.printStackTrace(); fail("Unexpected exception"); @@ -204,6 +207,30 @@ private List newDummyFeatureSetReferences() { .build()); } + private List newDummyListRequestFilters() { + return Arrays.asList( + // all provided: name, version and project + ListFeatureSetsRequest.Filter.newBuilder() + .setFeatureSetName(this.featureSet.getName()) + .setProject(this.featureSet.getProject().toString()) + .setFeatureSetVersion(String.valueOf(this.featureSet.getVersion())) + .build(), + + // name and project + ListFeatureSetsRequest.Filter.newBuilder() + .setFeatureSetName(this.featureSet.getName()) + .setProject(this.featureSet.getProject().toString()) + .setFeatureSetVersion("*") + .build(), + + // name and project + ListFeatureSetsRequest.Filter.newBuilder() + .setFeatureSetName(this.featureSet.getName()) + .setProject("*") + .setFeatureSetVersion(String.valueOf(this.featureSet.getVersion())) + .build()); + } + /* unit tests */ private ListIngestionJobsResponse tryListJobs(ListIngestionJobsRequest request) { ListIngestionJobsResponse response = null; diff --git a/core/src/test/java/feast/core/service/SpecServiceTest.java b/core/src/test/java/feast/core/service/SpecServiceTest.java index 18d2f6adf3c..43a66135dce 100644 --- a/core/src/test/java/feast/core/service/SpecServiceTest.java +++ b/core/src/test/java/feast/core/service/SpecServiceTest.java @@ -41,7 +41,6 @@ import feast.core.FeatureSetProto.FeatureSetSpec; import feast.core.FeatureSetProto.FeatureSetStatus; import feast.core.FeatureSetProto.FeatureSpec; -import feast.core.FeatureSetReferenceProto.FeatureSetReference; import feast.core.SourceProto.KafkaSourceConfig; import feast.core.SourceProto.SourceType; import feast.core.StoreProto; @@ -833,14 +832,4 @@ private Store newDummyStore(String name) { store.setConfig(RedisConfig.newBuilder().setPort(6379).build().toByteArray()); return store; } - - @Test - public void shouldMatchFeatureSetGivenFeatureSetReference() - throws InvalidProtocolBufferException { - FeatureSetReference fsReference = - FeatureSetReference.newBuilder().setName("f1").setProject("project1").setVersion(1).build(); - ListFeatureSetsResponse response = this.specService.matchFeatureSets(fsReference); - FeatureSet featureSet = FeatureSet.fromProto(response.getFeatureSets(0)); - assertEquals(featureSet, this.featureSets.get(0)); - } } From 1799c3ab6d4548956ab4f7d3e5d294e2d3a69ee0 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 23 Mar 2020 21:39:47 +0800 Subject: [PATCH 32/66] Make the map that maps between JobStatus and IngestionJobStatus static --- .../main/java/feast/core/model/JobStatus.java | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/feast/core/model/JobStatus.java b/core/src/main/java/feast/core/model/JobStatus.java index 277a1870811..86aa512933c 100644 --- a/core/src/main/java/feast/core/model/JobStatus.java +++ b/core/src/main/java/feast/core/model/JobStatus.java @@ -80,6 +80,18 @@ public static final Collection getTransitionalStates() { return TRANSITIONAL_STATES; } + private static final Map INGESTION_JOB_STATUS_MAP = + Map.of( + JobStatus.UNKNOWN, IngestionJobStatus.UNKNOWN, + JobStatus.PENDING, IngestionJobStatus.PENDING, + JobStatus.RUNNING, IngestionJobStatus.RUNNING, + JobStatus.COMPLETED, IngestionJobStatus.COMPLETED, + JobStatus.ABORTING, IngestionJobStatus.ABORTING, + JobStatus.ABORTED, IngestionJobStatus.ABORTED, + JobStatus.ERROR, IngestionJobStatus.ERROR, + JobStatus.SUSPENDING, IngestionJobStatus.SUSPENDING, + JobStatus.SUSPENDED, IngestionJobStatus.SUSPENDED); + /** * Convert a Job Status to Ingestion Job Status proto * @@ -87,17 +99,6 @@ public static final Collection getTransitionalStates() { */ public IngestionJobStatus toProto() { // maps job models job status to ingestion job status - Map statusMap = - Map.of( - JobStatus.UNKNOWN, IngestionJobStatus.UNKNOWN, - JobStatus.PENDING, IngestionJobStatus.PENDING, - JobStatus.RUNNING, IngestionJobStatus.RUNNING, - JobStatus.COMPLETED, IngestionJobStatus.COMPLETED, - JobStatus.ABORTING, IngestionJobStatus.ABORTING, - JobStatus.ABORTED, IngestionJobStatus.ABORTED, - JobStatus.ERROR, IngestionJobStatus.ERROR, - JobStatus.SUSPENDING, IngestionJobStatus.SUSPENDING, - JobStatus.SUSPENDED, IngestionJobStatus.SUSPENDED); - return statusMap.get(this); + return INGESTION_JOB_STATUS_MAP.get(this); } } From 1bead61c4550604fd427b8b255a42c2735156306 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Wed, 25 Mar 2020 14:09:23 +0800 Subject: [PATCH 33/66] Make JobService's listJobs() to return all ingestion jobs on empty filter --- .../java/feast/core/service/JobService.java | 76 ++++++++++--------- protos/feast/core/CoreService.proto | 4 +- 2 files changed, 44 insertions(+), 36 deletions(-) diff --git a/core/src/main/java/feast/core/service/JobService.java b/core/src/main/java/feast/core/service/JobService.java index 0e007b368c5..ec2b09110ee 100644 --- a/core/src/main/java/feast/core/service/JobService.java +++ b/core/src/main/java/feast/core/service/JobService.java @@ -67,7 +67,8 @@ public JobService( /* Job Service API */ /** - * List Ingestion Jobs in feast matching the given request + * List Ingestion Jobs in feast matching the given request. See CoreService protobuf documentation + * on the request * * @param request list ingestion jobs request specifying which jobs to include * @throws IllegalArgumentException when given filter in a unsupported configuration @@ -77,43 +78,48 @@ public JobService( @Transactional(readOnly = true) public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest request) throws InvalidProtocolBufferException { - // filter jobs based on request filter - ListIngestionJobsRequest.Filter filter = request.getFilter(); Set matchingJobIds = new HashSet<>(); - - // for proto3, default value for missing values: - // - numeric values (ie int) is zero - // - strings is empty string - - if (filter.getId() != "") { - // get by id: no more filters required: found job - Optional job = this.jobRepository.findById(filter.getId()); - if (job.isPresent()) { - matchingJobIds.add(filter.getId()); + if (request.hasFilter()) { + // filter jobs based on request filter + ListIngestionJobsRequest.Filter filter = request.getFilter(); + + // for proto3, default value for missing values: + // - numeric values (ie int) is zero + // - strings is empty string + if (filter.getId() != "") { + // get by id: no more filters required: found job + Optional job = this.jobRepository.findById(filter.getId()); + if (job.isPresent()) { + matchingJobIds.add(filter.getId()); + } + } else { + // multiple filters can apply together in an 'and' operation + if (filter.getStoreName() != "") { + // find jobs by name + List jobs = this.jobRepository.findByStoreName(filter.getStoreName()); + Set jobIds = jobs.stream().map(Job::getId).collect(Collectors.toSet()); + matchingJobIds = this.mergeResults(matchingJobIds, jobIds); + } + if (filter.hasFeatureSetReference()) { + // find a matching featuresets for reference + FeatureSetReference fsReference = filter.getFeatureSetReference(); + ListFeatureSetsResponse response = + this.specService.listFeatureSets(this.toListFeatureSetFilter(fsReference)); + List featureSets = + response.getFeatureSetsList().stream() + .map(FeatureSet::fromProto) + .collect(Collectors.toList()); + + // find jobs for the matching featuresets + Collection matchingJobs = this.jobRepository.findByFeatureSetsIn(featureSets); + List jobIds = matchingJobs.stream().map(Job::getId).collect(Collectors.toList()); + matchingJobIds = this.mergeResults(matchingJobIds, jobIds); + } } } else { - // multiple filters can apply together in an 'and' operation - if (filter.getStoreName() != "") { - // find jobs by name - List jobs = this.jobRepository.findByStoreName(filter.getStoreName()); - List jobIds = jobs.stream().map(Job::getId).collect(Collectors.toList()); - matchingJobIds = this.mergeResults(matchingJobIds, jobIds); - } - if (filter.hasFeatureSetReference()) { - // find a matching featuresets for reference - FeatureSetReference fsReference = filter.getFeatureSetReference(); - ListFeatureSetsResponse response = - this.specService.listFeatureSets(this.toListFeatureSetFilter(fsReference)); - List featureSets = - response.getFeatureSetsList().stream() - .map(FeatureSet::fromProto) - .collect(Collectors.toList()); - - // find jobs for the matching featuresets - Collection matchingJobs = this.jobRepository.findByFeatureSetsIn(featureSets); - List jobIds = matchingJobs.stream().map(Job::getId).collect(Collectors.toList()); - matchingJobIds = this.mergeResults(matchingJobIds, jobIds); - } + // no filter: match all jobs + matchingJobIds = + this.jobRepository.findAll().stream().map(Job::getId).collect(Collectors.toSet()); } // convert matching job models to ingestion job protos diff --git a/protos/feast/core/CoreService.proto b/protos/feast/core/CoreService.proto index 17cecdd765e..02e6117f27d 100644 --- a/protos/feast/core/CoreService.proto +++ b/protos/feast/core/CoreService.proto @@ -77,7 +77,9 @@ service CoreService { rpc ListProjects (ListProjectsRequest) returns (ListProjectsResponse); // List Injestion Jobs - // TODO: write docs + // Returns allow ingestions matching the given request filter. + // Returns all ingestion jobs If no is provided. + // Returns an empty list if no ingestion jobs match the filter. rpc ListIngestionJobs(ListIngestionJobsRequest) returns (ListIngestionJobsResponse); // Restart an Ingestion job From 19994ea227568837ca072e17884641cdb2386769 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Wed, 25 Mar 2020 16:42:51 +0800 Subject: [PATCH 34/66] Fixed issue where the jobManager map that JobService built used wrong keys --- core/src/main/java/feast/core/model/Job.java | 2 ++ core/src/main/java/feast/core/service/JobService.java | 4 +++- core/src/test/java/feast/core/service/JobServiceTest.java | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/feast/core/model/Job.java b/core/src/main/java/feast/core/model/Job.java index 40b60026ebb..738a16db2d1 100644 --- a/core/src/main/java/feast/core/model/Job.java +++ b/core/src/main/java/feast/core/model/Job.java @@ -27,7 +27,9 @@ import javax.persistence.EnumType; import javax.persistence.Enumerated; import javax.persistence.Id; +import javax.persistence.Index; import javax.persistence.JoinColumn; +import javax.persistence.JoinTable; import javax.persistence.ManyToMany; import javax.persistence.ManyToOne; import javax.persistence.OneToMany; diff --git a/core/src/main/java/feast/core/service/JobService.java b/core/src/main/java/feast/core/service/JobService.java index ec2b09110ee..8114164df99 100644 --- a/core/src/main/java/feast/core/service/JobService.java +++ b/core/src/main/java/feast/core/service/JobService.java @@ -33,6 +33,7 @@ import feast.core.model.Job; import feast.core.model.JobStatus; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; @@ -61,7 +62,7 @@ public JobService( this.jobManagers = new HashMap<>(); for (JobManager manager : jobManagerList) { - this.jobManagers.put(manager.getRunnerType().getName(), manager); + this.jobManagers.put(manager.getRunnerType().toString(), manager); } } @@ -202,6 +203,7 @@ public StopIngestionJobResponse stopJob(StopIngestionJobRequest request) } // stop job with job manager + System.out.println("Status: " + job.getStatus().toString()); JobManager jobManager = this.jobManagers.get(job.getRunner()); jobManager.abortJob(job.getExtId()); diff --git a/core/src/test/java/feast/core/service/JobServiceTest.java b/core/src/test/java/feast/core/service/JobServiceTest.java index e798e6a94ec..4bd2b28e678 100644 --- a/core/src/test/java/feast/core/service/JobServiceTest.java +++ b/core/src/test/java/feast/core/service/JobServiceTest.java @@ -178,7 +178,7 @@ private Job newDummyJob(String id, String extId, JobStatus status) { return new Job( id, extId, - Runner.DATAFLOW.getName(), + Runner.DATAFLOW.toString(), this.dataSource, this.dataStore, Arrays.asList(this.featureSet), From ff73b5fe2a3a2e29a3d369846fc66b93b5f8057b Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Thu, 26 Mar 2020 15:31:21 +0800 Subject: [PATCH 35/66] Fix issue where actual Job Status is not synced with database. Issue occurs when the job is aborted/restarted, but the JobStatus has not yet been updated by JobUpdateTask. Hence another call to abortJob() & restartJob() that should be rejected due invalid status is allowed through --- core/src/main/java/feast/core/service/JobService.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/feast/core/service/JobService.java b/core/src/main/java/feast/core/service/JobService.java index 8114164df99..2f632849660 100644 --- a/core/src/main/java/feast/core/service/JobService.java +++ b/core/src/main/java/feast/core/service/JobService.java @@ -33,7 +33,6 @@ import feast.core.model.Job; import feast.core.model.JobStatus; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; @@ -163,8 +162,8 @@ public RestartIngestionJobResponse restartJob(RestartIngestionJobRequest request // restart job with job manager JobManager jobManager = this.jobManagers.get(job.getRunner()); job = jobManager.restartJob(job); - - // update job model in job repository + // sync job status & update job model in job repository + job.setStatus(jobManager.getJobStatus(job)); this.jobRepository.saveAndFlush(job); return RestartIngestionJobResponse.newBuilder().build(); @@ -203,9 +202,11 @@ public StopIngestionJobResponse stopJob(StopIngestionJobRequest request) } // stop job with job manager - System.out.println("Status: " + job.getStatus().toString()); JobManager jobManager = this.jobManagers.get(job.getRunner()); jobManager.abortJob(job.getExtId()); + // sync job status & update job model in job repository + job.setStatus(jobManager.getJobStatus(job)); + this.jobRepository.saveAndFlush(job); return StopIngestionJobResponse.newBuilder().build(); } From c207f1ac9c13a0b1f64b877cf8feb462fcad5393 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Thu, 26 Mar 2020 15:59:28 +0800 Subject: [PATCH 36/66] Log stopJob() & restartJob() operations to make debugging easier --- .../java/feast/core/service/JobService.java | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/feast/core/service/JobService.java b/core/src/main/java/feast/core/service/JobService.java index 2f632849660..89a54f0d579 100644 --- a/core/src/main/java/feast/core/service/JobService.java +++ b/core/src/main/java/feast/core/service/JobService.java @@ -29,9 +29,14 @@ import feast.core.IngestionJobProto; import feast.core.dao.JobRepository; import feast.core.job.JobManager; +import feast.core.log.Action; +import feast.core.log.AuditLogger; +import feast.core.log.Resource; import feast.core.model.FeatureSet; import feast.core.model.Job; import feast.core.model.JobStatus; +import lombok.extern.slf4j.Slf4j; + import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; @@ -47,6 +52,7 @@ import org.springframework.transaction.annotation.Transactional; /** Defines a Job Managemenent Service that allows users to manage feast ingestion jobs. */ +@Slf4j @Service public class JobService { private JobRepository jobRepository; @@ -162,10 +168,12 @@ public RestartIngestionJobResponse restartJob(RestartIngestionJobRequest request // restart job with job manager JobManager jobManager = this.jobManagers.get(job.getRunner()); job = jobManager.restartJob(job); + log.info(String.format("Restarted job (id: %s, extId: %s runner: %s)", + job.getId(), job.getExtId(), job.getRunner())); // sync job status & update job model in job repository - job.setStatus(jobManager.getJobStatus(job)); + job = this.syncJobStatus(jobManager, job); this.jobRepository.saveAndFlush(job); - + return RestartIngestionJobResponse.newBuilder().build(); } @@ -204,8 +212,11 @@ public StopIngestionJobResponse stopJob(StopIngestionJobRequest request) // stop job with job manager JobManager jobManager = this.jobManagers.get(job.getRunner()); jobManager.abortJob(job.getExtId()); + log.info(String.format("Restarted job (id: %s, extId: %s runner: %s)", + job.getId(), job.getExtId(), job.getRunner())); + // sync job status & update job model in job repository - job.setStatus(jobManager.getJobStatus(job)); + job = this.syncJobStatus(jobManager, job); this.jobRepository.saveAndFlush(job); return StopIngestionJobResponse.newBuilder().build(); @@ -243,4 +254,18 @@ private ListFeatureSetsRequest.Filter toListFeatureSetFilter(FeatureSetReference return filter; } + + // sync job status using job manager + private Job syncJobStatus(JobManager jobManager, Job job) { + JobStatus newStatus = jobManager.getJobStatus(job); + // log job status transition + if(!newStatus.equals(job.getStatus())) { + job.setStatus(newStatus); + AuditLogger.log(Resource.JOB, job.getId(), Action.STATUS_CHANGE, + "Job status transition: changed from %s to %s", + job.getStatus(), newStatus); + } + return job; + } + } From c64039ac63ef6ad1ecc8e3ae00386ce85933c1aa Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Fri, 27 Mar 2020 16:03:18 +0800 Subject: [PATCH 37/66] Use Runner.name() instead of runner.toString() to build JobManager map --- .../java/feast/core/service/JobService.java | 34 +++++++++++-------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/feast/core/service/JobService.java b/core/src/main/java/feast/core/service/JobService.java index 89a54f0d579..c34e324c7ea 100644 --- a/core/src/main/java/feast/core/service/JobService.java +++ b/core/src/main/java/feast/core/service/JobService.java @@ -35,8 +35,6 @@ import feast.core.model.FeatureSet; import feast.core.model.Job; import feast.core.model.JobStatus; -import lombok.extern.slf4j.Slf4j; - import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; @@ -47,6 +45,7 @@ import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; +import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; @@ -67,7 +66,7 @@ public JobService( this.jobManagers = new HashMap<>(); for (JobManager manager : jobManagerList) { - this.jobManagers.put(manager.getRunnerType().toString(), manager); + this.jobManagers.put(manager.getRunnerType().name(), manager); } } @@ -168,12 +167,14 @@ public RestartIngestionJobResponse restartJob(RestartIngestionJobRequest request // restart job with job manager JobManager jobManager = this.jobManagers.get(job.getRunner()); job = jobManager.restartJob(job); - log.info(String.format("Restarted job (id: %s, extId: %s runner: %s)", - job.getId(), job.getExtId(), job.getRunner())); + log.info( + String.format( + "Restarted job (id: %s, extId: %s runner: %s)", + job.getId(), job.getExtId(), job.getRunner())); // sync job status & update job model in job repository job = this.syncJobStatus(jobManager, job); this.jobRepository.saveAndFlush(job); - + return RestartIngestionJobResponse.newBuilder().build(); } @@ -212,9 +213,11 @@ public StopIngestionJobResponse stopJob(StopIngestionJobRequest request) // stop job with job manager JobManager jobManager = this.jobManagers.get(job.getRunner()); jobManager.abortJob(job.getExtId()); - log.info(String.format("Restarted job (id: %s, extId: %s runner: %s)", - job.getId(), job.getExtId(), job.getRunner())); - + log.info( + String.format( + "Restarted job (id: %s, extId: %s runner: %s)", + job.getId(), job.getExtId(), job.getRunner())); + // sync job status & update job model in job repository job = this.syncJobStatus(jobManager, job); this.jobRepository.saveAndFlush(job); @@ -259,13 +262,16 @@ private ListFeatureSetsRequest.Filter toListFeatureSetFilter(FeatureSetReference private Job syncJobStatus(JobManager jobManager, Job job) { JobStatus newStatus = jobManager.getJobStatus(job); // log job status transition - if(!newStatus.equals(job.getStatus())) { - job.setStatus(newStatus); - AuditLogger.log(Resource.JOB, job.getId(), Action.STATUS_CHANGE, + if (newStatus != job.getStatus()) { + AuditLogger.log( + Resource.JOB, + job.getId(), + Action.STATUS_CHANGE, "Job status transition: changed from %s to %s", - job.getStatus(), newStatus); + job.getStatus(), + newStatus); + job.setStatus(newStatus); } return job; } - } From ed4dae9d9b407484226879e97c8a25a75757b185 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Sat, 28 Mar 2020 17:02:41 +0800 Subject: [PATCH 38/66] Move documentation on JobService operations to CoreService protobuf definition --- .../main/java/feast/core/service/JobService.java | 10 +++++----- protos/feast/core/CoreService.proto | 15 +++++++++------ 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/feast/core/service/JobService.java b/core/src/main/java/feast/core/service/JobService.java index c34e324c7ea..a9c71755110 100644 --- a/core/src/main/java/feast/core/service/JobService.java +++ b/core/src/main/java/feast/core/service/JobService.java @@ -73,7 +73,7 @@ public JobService( /* Job Service API */ /** * List Ingestion Jobs in feast matching the given request. See CoreService protobuf documentation - * on the request + * for more detailed documentation. * * @param request list ingestion jobs request specifying which jobs to include * @throws IllegalArgumentException when given filter in a unsupported configuration @@ -139,7 +139,8 @@ public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest request) } /** - * Restart (Aborts) the ingestion job matching the given restart request. + * Restart (Aborts) the ingestion job matching the given restart request. See CoreService protobuf + * documentation for more detailed documentation. * * @param request restart ingestion job request specifying which job to stop * @throws NoSuchElementException when restart job request requests to restart a nonexistent job. @@ -179,9 +180,8 @@ public RestartIngestionJobResponse restartJob(RestartIngestionJobRequest request } /** - * Stops (Aborts) the ingestion job matching the given stop request. Does nothing if the target - * job if already in a terminal states Does not support stopping a job in a transitional or - * unknown status + * Stops (Aborts) the ingestion job matching the given stop request. See CoreService protobuf + * documentation for more detailed documentation. * * @param request stop ingestion job request specifying which job to stop * @throws NoSuchElementException when stop job request requests to stop a nonexistent job. diff --git a/protos/feast/core/CoreService.proto b/protos/feast/core/CoreService.proto index 02e6117f27d..ea3e7bf8a7a 100644 --- a/protos/feast/core/CoreService.proto +++ b/protos/feast/core/CoreService.proto @@ -76,18 +76,21 @@ service CoreService { // Lists all projects active projects. rpc ListProjects (ListProjectsRequest) returns (ListProjectsResponse); - // List Injestion Jobs + // List Ingestion Jobs given an optional filter. // Returns allow ingestions matching the given request filter. - // Returns all ingestion jobs If no is provided. + // Returns all ingestion jobs if no filter is provided. // Returns an empty list if no ingestion jobs match the filter. rpc ListIngestionJobs(ListIngestionJobsRequest) returns (ListIngestionJobsResponse); - // Restart an Ingestion job - // TODO: write docs + // Restart an Ingestion Job. Restarts the ingestion job with the given job id. + // NOTE: Data might be lost during the restart for some job runners. + // Just starts the job if the job is a terminal state (ie suspended or aborted). + // Does not support restarts a job in a transitional (ie pending, suspending, aborting) or unknown status rpc RestartIngestionJob(RestartIngestionJobRequest) returns (RestartIngestionJobResponse); - // Restart an Ingestion job - // TODO: write docs + // Stop an Ingestion Job. Stop (Aborts) the ingestion job with the given job id. + // Does nothing if the target job if already in a terminal state (ie suspended or aborted). + // Does not support stopping a job in a transitional (ie pending, suspending, aborting) or unknown status rpc StopIngestionJob(StopIngestionJobRequest) returns (StopIngestionJobResponse); } From 24c09be529e44cc4bb940ccfb4ec80cf04c24fae Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 30 Mar 2020 11:53:39 +0800 Subject: [PATCH 39/66] Added IngestJob to python sdk as native representation of IngestionJob proto --- sdk/python/feast/job.py | 62 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/sdk/python/feast/job.py b/sdk/python/feast/job.py index ab65da74459..e12f496f78a 100644 --- a/sdk/python/feast/job.py +++ b/sdk/python/feast/job.py @@ -7,6 +7,8 @@ import pandas as pd from google.cloud import storage +from feast.feature_set import FeatureSet +from feast.source import Source from feast.serving.ServingService_pb2 import ( DATA_FORMAT_AVRO, JOB_STATUS_DONE, @@ -14,6 +16,7 @@ ) from feast.serving.ServingService_pb2 import Job as JobProto from feast.serving.ServingService_pb2_grpc import ServingServiceStub +from feast.core.IngestionJob_pb2 import IngestionJob as IngestJobProto # Maximum no of seconds to wait until the jobs status is DONE in Feast # Currently set to the maximum query execution time limit in BigQuery @@ -187,3 +190,62 @@ def to_chunked_dataframe( def __iter__(self): return iter(self.result()) + + +class IngestJob: + """ + Defines a job that ingests feature data into feast. + """ + + def __init__(self, job_proto: IngestJobProto): + """ + Construct a native ingest job from its protobuf version. + + Args: + job_proto: Job proto object to construct from. + """ + self.proto = job_proto + + @property + def id(self): + """ + Getter for IngestJob's job id. + """ + return self.proto.id + + @property + def external_id(self): + """ + Getter for IngestJob's external job id. + """ + return self.proto.id + + @property + def status(self): + """ + Getter for IngestJob's status + """ + # TODO: refresh current status from core service + return self.proto.status + + @property + def feature_sets(self): + """ + Getter for the IngestJob's feature sets + """ + # convert featureset protos to native objects + return [FeatureSet.from_proto(fs) for fs in self.proto.feature_sets] + + @property + def source(self): + """ + Getter for the IngestJob's data source + """ + return Source.from_proto(self.proto.source) + + @property + def store(self): + """ + Getter for the IngestJob's target feast store. + """ + return self.proto.source From 29303888f6915a74832c5c0fe290aeb029559510 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 30 Mar 2020 13:22:01 +0800 Subject: [PATCH 40/66] Make empty filter on JobService's listJobs() select all ingestion jobs --- .../src/main/java/feast/core/service/JobService.java | 9 +++++++-- .../test/java/feast/core/service/JobServiceTest.java | 12 +++++++++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/feast/core/service/JobService.java b/core/src/main/java/feast/core/service/JobService.java index a9c71755110..fe668aedfba 100644 --- a/core/src/main/java/feast/core/service/JobService.java +++ b/core/src/main/java/feast/core/service/JobService.java @@ -84,7 +84,12 @@ public JobService( public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest request) throws InvalidProtocolBufferException { Set matchingJobIds = new HashSet<>(); - if (request.hasFilter()) { + + // check that filter specified and not empty + if (request.hasFilter() + && !(request.getFilter().getId() == "" + && request.getFilter().getStoreName() == "" + && request.getFilter().hasFeatureSetReference() == false)) { // filter jobs based on request filter ListIngestionJobsRequest.Filter filter = request.getFilter(); @@ -122,7 +127,7 @@ public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest request) } } } else { - // no filter: match all jobs + // no or empty filter: match all jobs matchingJobIds = this.jobRepository.findAll().stream().map(Job::getId).collect(Collectors.toSet()); } diff --git a/core/src/test/java/feast/core/service/JobServiceTest.java b/core/src/test/java/feast/core/service/JobServiceTest.java index 4bd2b28e678..c0e90ca43f4 100644 --- a/core/src/test/java/feast/core/service/JobServiceTest.java +++ b/core/src/test/java/feast/core/service/JobServiceTest.java @@ -146,6 +146,7 @@ public void setupJobRepository() { .thenReturn(Arrays.asList(this.job)); when(this.jobRepository.findByFeatureSetsIn(Arrays.asList(this.featureSet))) .thenReturn(Arrays.asList(this.job)); + when(this.jobRepository.findAll()).thenReturn(Arrays.asList(this.job)); } // TODO: setup fake job manager @@ -178,7 +179,7 @@ private Job newDummyJob(String id, String extId, JobStatus status) { return new Job( id, extId, - Runner.DATAFLOW.toString(), + Runner.DATAFLOW.name(), this.dataSource, this.dataStore, Arrays.asList(this.featureSet), @@ -252,6 +253,15 @@ public void testListJobsById() { ListIngestionJobsRequest request = ListIngestionJobsRequest.newBuilder().setFilter(filter).build(); assertThat(this.tryListJobs(request).getJobs(0), equalTo(this.ingestionJob)); + + // list with no filter + request = ListIngestionJobsRequest.newBuilder().build(); + assertThat(this.tryListJobs(request).getJobs(0), equalTo(this.ingestionJob)); + + // list with empty filter + filter = ListIngestionJobsRequest.Filter.newBuilder().build(); + request = ListIngestionJobsRequest.newBuilder().setFilter(filter).build(); + assertThat(this.tryListJobs(request).getJobs(0), equalTo(this.ingestionJob)); } @Test From 5cc8abe8d147efa7429523e379bd2a37028a7e83 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 30 Mar 2020 14:41:18 +0800 Subject: [PATCH 41/66] Added bindings for Job management API to python sdk client. --- sdk/python/feast/client.py | 67 ++++++++++++++++++++++++++++++++++++-- sdk/python/feast/job.py | 15 +++++---- 2 files changed, 74 insertions(+), 8 deletions(-) diff --git a/sdk/python/feast/client.py b/sdk/python/feast/client.py index 2a0b636b373..112d8db50a1 100644 --- a/sdk/python/feast/client.py +++ b/sdk/python/feast/client.py @@ -50,11 +50,15 @@ ListFeatureSetsResponse, ListProjectsRequest, ListProjectsResponse, + ListIngestionJobsRequest, + RestartIngestionJobRequest, + StopIngestionJobRequest, ) from feast.core.CoreService_pb2_grpc import CoreServiceStub from feast.core.FeatureSet_pb2 import FeatureSetStatus +from feast.core.FeatureSetReference_pb2 import FeatureSetReference from feast.feature_set import Entity, FeatureSet -from feast.job import Job +from feast.job import Job, IngestJob from feast.loaders.abstract_producer import get_producer from feast.loaders.file import export_source_to_staging_location from feast.loaders.ingest import KAFKA_CHUNK_PRODUCTION_TIMEOUT, get_feature_row_chunks @@ -416,7 +420,7 @@ def list_feature_sets( Args: project: Filter feature sets based on project name name: Filter feature sets based on feature set name - version: Filter feature sets based on version number + version: Filter feature sets based on version numbf, Returns: List of feature sets @@ -648,6 +652,65 @@ def get_online_features( ) ) + def list_ingest_jobs( + self, job_id: str = None, feature_set: FeatureSet = None, store_name: str = None + ): + """ + List the ingestion jobs currently registered in Feast, with optional filters. + Provides detailed metadata about each ingestion job. + + Args: + job_id: Select specific ingestion job with the given job_id + feature_set: Filter ingestion jobs by those tied to the given feature set. + store_name: Filter ingestion jobs by target feast store's name + + Returns: + List of IngestJobs matching the given filters + """ + # construct list request + feature_set_ref = None + if feature_set is not None: + feature_set_ref = FeatureSetReference( + name=feature_set.name, + project=feature_set.project, + version=feature_set.version, + ) + list_filter = ListIngestionJobsRequest.Filter( + id=job_id, feature_set_reference=feature_set_ref, store_name=store_name + ) + request = ListIngestionJobsRequest(filter=list_filter) + # make list request & unpack response + response = self._core_service_stub.ListIngestionJobs(request) + ingest_jobs = [IngestJob(proto) for proto in response.jobs] + return ingest_jobs + + def restart_ingest_job(self, job: IngestJob): + """ + Restart ingestion job currently registered in Feast. + NOTE: Data might be lost during the restart for some job runners. + Just starts the job if the job is a terminal state (ie suspended or aborted). + Does not support restarts a job in a transitional (ie pending, suspending, aborting) + or in a unknown status + + Args: + job: IngestJob to restart + """ + request = RestartIngestionJobRequest(id=job.id) + self._core_service_stub.RestartIngestionJob(request) + + def stop_ingest_job(self, job: IngestJob): + """ + Stop ingestion job currently resgistered in Feast + Does nothing if the target job if already in a terminal state (ie suspended or aborted). + Does not support stopping a job in a transitional (ie pending, suspending, aborting) + or in a unknown status + + Args: + job: IngestJob to restart + """ + request = StopIngestionJobRequest(id=job.id) + self._core_service_stub.StopIngestionJob(request) + def ingest( self, feature_set: Union[str, FeatureSet], diff --git a/sdk/python/feast/job.py b/sdk/python/feast/job.py index e12f496f78a..554a3a17d63 100644 --- a/sdk/python/feast/job.py +++ b/sdk/python/feast/job.py @@ -2,6 +2,7 @@ import time from datetime import datetime, timedelta from urllib.parse import urlparse +from typing import List import fastavro import pandas as pd @@ -16,7 +17,9 @@ ) from feast.serving.ServingService_pb2 import Job as JobProto from feast.serving.ServingService_pb2_grpc import ServingServiceStub +from feast.core.Store_pb2 import Store from feast.core.IngestionJob_pb2 import IngestionJob as IngestJobProto +from feast.core.IngestionJob_pb2 import IngestionJobStatus # Maximum no of seconds to wait until the jobs status is DONE in Feast # Currently set to the maximum query execution time limit in BigQuery @@ -207,21 +210,21 @@ def __init__(self, job_proto: IngestJobProto): self.proto = job_proto @property - def id(self): + def id(self) -> str: """ Getter for IngestJob's job id. """ return self.proto.id @property - def external_id(self): + def external_id(self) -> str: """ Getter for IngestJob's external job id. """ return self.proto.id @property - def status(self): + def status(self) -> IngestionJobStatus: """ Getter for IngestJob's status """ @@ -229,7 +232,7 @@ def status(self): return self.proto.status @property - def feature_sets(self): + def feature_sets(self) -> List[FeatureSet]: """ Getter for the IngestJob's feature sets """ @@ -237,14 +240,14 @@ def feature_sets(self): return [FeatureSet.from_proto(fs) for fs in self.proto.feature_sets] @property - def source(self): + def source(self) -> Source: """ Getter for the IngestJob's data source """ return Source.from_proto(self.proto.source) @property - def store(self): + def store(self) -> Store: """ Getter for the IngestJob's target feast store. """ From d86cb66d77749eb9d8ba43899c1f54e29fce7961 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 30 Mar 2020 15:13:20 +0800 Subject: [PATCH 42/66] Fixed __connect_core() to connect to Feast CoreService when calling on Job API calls --- sdk/python/feast/client.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/sdk/python/feast/client.py b/sdk/python/feast/client.py index 112d8db50a1..fdc5dd208a0 100644 --- a/sdk/python/feast/client.py +++ b/sdk/python/feast/client.py @@ -667,6 +667,7 @@ def list_ingest_jobs( Returns: List of IngestJobs matching the given filters """ + self._connect_core() # construct list request feature_set_ref = None if feature_set is not None: @@ -681,7 +682,9 @@ def list_ingest_jobs( request = ListIngestionJobsRequest(filter=list_filter) # make list request & unpack response response = self._core_service_stub.ListIngestionJobs(request) - ingest_jobs = [IngestJob(proto) for proto in response.jobs] + ingest_jobs = [ + IngestJob(proto, self._core_service_stub) for proto in response.jobs + ] return ingest_jobs def restart_ingest_job(self, job: IngestJob): @@ -695,6 +698,7 @@ def restart_ingest_job(self, job: IngestJob): Args: job: IngestJob to restart """ + self._connect_core() request = RestartIngestionJobRequest(id=job.id) self._core_service_stub.RestartIngestionJob(request) @@ -708,6 +712,7 @@ def stop_ingest_job(self, job: IngestJob): Args: job: IngestJob to restart """ + self._connect_core() request = StopIngestionJobRequest(id=job.id) self._core_service_stub.StopIngestionJob(request) From d9874f292387c2eec52df291386af67ce277d132 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 30 Mar 2020 15:14:21 +0800 Subject: [PATCH 43/66] Auto reload IngestJob.status and IngestJob.external_id on get property. --- sdk/python/feast/job.py | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/sdk/python/feast/job.py b/sdk/python/feast/job.py index 554a3a17d63..b2768a29762 100644 --- a/sdk/python/feast/job.py +++ b/sdk/python/feast/job.py @@ -20,6 +20,8 @@ from feast.core.Store_pb2 import Store from feast.core.IngestionJob_pb2 import IngestionJob as IngestJobProto from feast.core.IngestionJob_pb2 import IngestionJobStatus +from feast.core.CoreService_pb2_grpc import CoreServiceStub +from feast.core.CoreService_pb2 import ListIngestionJobsRequest # Maximum no of seconds to wait until the jobs status is DONE in Feast # Currently set to the maximum query execution time limit in BigQuery @@ -39,7 +41,6 @@ def __init__(self, job_proto: JobProto, serving_stub: ServingServiceStub): Args: job_proto: Job proto object (wrapped by this job object) serving_stub: Stub for Feast serving service - storage_client: Google Cloud Storage client """ self.job_proto = job_proto self.serving_stub = serving_stub @@ -200,14 +201,16 @@ class IngestJob: Defines a job that ingests feature data into feast. """ - def __init__(self, job_proto: IngestJobProto): + def __init__(self, job_proto: IngestJobProto, core_stub: CoreServiceStub): """ Construct a native ingest job from its protobuf version. Args: job_proto: Job proto object to construct from. + core_stub: stub for Feast CoreService """ self.proto = job_proto + self.core_svc = core_stub @property def id(self) -> str: @@ -221,14 +224,15 @@ def external_id(self) -> str: """ Getter for IngestJob's external job id. """ - return self.proto.id + self.reload() + return self.proto.external_id @property def status(self) -> IngestionJobStatus: """ Getter for IngestJob's status """ - # TODO: refresh current status from core service + self.reload() return self.proto.status @property @@ -252,3 +256,13 @@ def store(self) -> Store: Getter for the IngestJob's target feast store. """ return self.proto.source + + def reload(self): + """ + Update this IngestJob with the latest info from Feast + """ + # pull latest proto from feast core + response = self.core_svc.ListIngestionJobs( + ListIngestionJobsRequest(filter=ListIngestionJobsRequest.Filter(id=self.id)) + ) + self.proto = response.jobs[0] From 19620b2622465bf91ee7e603aceafb25d40cc4dc Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 30 Mar 2020 15:55:53 +0800 Subject: [PATCH 44/66] Added IngestJob.wait() to wait for job status to transtion --- sdk/python/feast/job.py | 41 ++++++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/sdk/python/feast/job.py b/sdk/python/feast/job.py index b2768a29762..76783ea4e36 100644 --- a/sdk/python/feast/job.py +++ b/sdk/python/feast/job.py @@ -212,6 +212,16 @@ def __init__(self, job_proto: IngestJobProto, core_stub: CoreServiceStub): self.proto = job_proto self.core_svc = core_stub + def reload(self): + """ + Update this IngestJob with the latest info from Feast + """ + # pull latest proto from feast core + response = self.core_svc.ListIngestionJobs( + ListIngestionJobsRequest(filter=ListIngestionJobsRequest.Filter(id=self.id)) + ) + self.proto = response.jobs[0] + @property def id(self) -> str: """ @@ -246,7 +256,7 @@ def feature_sets(self) -> List[FeatureSet]: @property def source(self) -> Source: """ - Getter for the IngestJob's data source + Getter for the IngestJob's data source. """ return Source.from_proto(self.proto.source) @@ -257,12 +267,25 @@ def store(self) -> Store: """ return self.proto.source - def reload(self): - """ - Update this IngestJob with the latest info from Feast + def wait( + self, status: IngestionJobStatus, timeout: float = 300, interval: float = 5 + ): """ - # pull latest proto from feast core - response = self.core_svc.ListIngestionJobs( - ListIngestionJobsRequest(filter=ListIngestionJobsRequest.Filter(id=self.id)) - ) - self.proto = response.jobs[0] + Wait for this IngestJob to transtion to the given status. + Raises TimeoutError if the wait operation times out. + + Args: + status: The IngestionJobStatus to wait for. + timeout: Maximum seconds to wait before timing out. + interval: The interval to wait between checking the IngestJob status. + """ + # poll & wait for job status to transition + wait_begin = time.time() + elapsed = 0 + while self.status != status and elapsed <= timeout: + time.sleep(interval) + elapsed = time.time() - wait_begin + + # raise error if timeout + if elapsed > timeout: + raise TimeoutError("Wait for IngestJob's status to transition timed out") From 6b47ecbf36ba9d42bd6c70840432e6d32d2a24f0 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 30 Mar 2020 16:16:44 +0800 Subject: [PATCH 45/66] Added basic job api e2e test to exercise job api --- tests/e2e/basic-ingest-redis-serving.py | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/tests/e2e/basic-ingest-redis-serving.py b/tests/e2e/basic-ingest-redis-serving.py index 1aeccfa5a3a..0df3fd31884 100644 --- a/tests/e2e/basic-ingest-redis-serving.py +++ b/tests/e2e/basic-ingest-redis-serving.py @@ -7,6 +7,7 @@ GetOnlineFeaturesRequest, GetOnlineFeaturesResponse, ) +from feast.core.IngestionJob_pb2 import IngestionJobStatus from feast.types.Value_pb2 import Value as Value from feast.client import Client from feast.feature_set import FeatureSet @@ -108,7 +109,6 @@ def test_basic_ingest_success(client, basic_dataframe): client.ingest(cust_trans_fs, basic_dataframe) time.sleep(5) - @pytest.mark.timeout(45) @pytest.mark.run(order=12) def test_basic_retrieve_online_success(client, basic_dataframe): @@ -152,6 +152,27 @@ def test_basic_retrieve_online_success(client, basic_dataframe): ): break +@pytest.mark.timeout(900) +@pytest.mark.run(order=19) +def test_basic_ingest_jobs(client, basic_dataframe): + # list ingestion jobs given featureset + cust_trans_fs = client.get_feature_set(name="customer_transactions") + jobs = client.list_ingest_jobs(feature_set=cust_trans_fs) + assert len(jobs) >= 1 + job = jobs[0] + job.wait(IngestionJobStatus.RUNNING) + assert job.status == IngestionJobStatus.RUNNING + + # restart ingestion job + client.restart_ingest_job(job) + job.wait(IngestionJobStatus.RUNNING) + assert job.status == IngestionJobStatus.RUNNING + + # stop ingestion job + client.stop_ingest_job(job) + job.wait(IngestionJobStatus.ABORTED) + assert job.status == IngestionJobStatus.ABORTED + @pytest.fixture(scope='module') def all_types_dataframe(): From f3471acae92173a43198b27aede1766ec051dc40 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 30 Mar 2020 16:47:29 +0800 Subject: [PATCH 46/66] Reorder the operations e2etest to make sure that jobs are running after test --- tests/e2e/basic-ingest-redis-serving.py | 27 +++++++++++++------------ 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/tests/e2e/basic-ingest-redis-serving.py b/tests/e2e/basic-ingest-redis-serving.py index 0df3fd31884..5a70f8a8fb7 100644 --- a/tests/e2e/basic-ingest-redis-serving.py +++ b/tests/e2e/basic-ingest-redis-serving.py @@ -159,19 +159,20 @@ def test_basic_ingest_jobs(client, basic_dataframe): cust_trans_fs = client.get_feature_set(name="customer_transactions") jobs = client.list_ingest_jobs(feature_set=cust_trans_fs) assert len(jobs) >= 1 - job = jobs[0] - job.wait(IngestionJobStatus.RUNNING) - assert job.status == IngestionJobStatus.RUNNING - - # restart ingestion job - client.restart_ingest_job(job) - job.wait(IngestionJobStatus.RUNNING) - assert job.status == IngestionJobStatus.RUNNING - - # stop ingestion job - client.stop_ingest_job(job) - job.wait(IngestionJobStatus.ABORTED) - assert job.status == IngestionJobStatus.ABORTED + + for job in jobs: + job.wait(IngestionJobStatus.RUNNING) + assert job.status == IngestionJobStatus.RUNNING + + # stop ingestion job + client.stop_ingest_job(job) + job.wait(IngestionJobStatus.ABORTED) + assert job.status == IngestionJobStatus.ABORTED + + # restart ingestion job + client.restart_ingest_job(job) + job.wait(IngestionJobStatus.RUNNING) + assert job.status == IngestionJobStatus.RUNNING @pytest.fixture(scope='module') From 170740156bcaa5e927714142536662250d1ee96d Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Tue, 31 Mar 2020 10:10:06 +0800 Subject: [PATCH 47/66] Added e2e test for all types exercising job api --- tests/e2e/basic-ingest-redis-serving.py | 52 +++++++++++++++++-------- 1 file changed, 36 insertions(+), 16 deletions(-) diff --git a/tests/e2e/basic-ingest-redis-serving.py b/tests/e2e/basic-ingest-redis-serving.py index 5a70f8a8fb7..f63393aa4a4 100644 --- a/tests/e2e/basic-ingest-redis-serving.py +++ b/tests/e2e/basic-ingest-redis-serving.py @@ -152,27 +152,27 @@ def test_basic_retrieve_online_success(client, basic_dataframe): ): break -@pytest.mark.timeout(900) -@pytest.mark.run(order=19) +@pytest.mark.timeout(300) +@pytest.mark.run(order=14) def test_basic_ingest_jobs(client, basic_dataframe): # list ingestion jobs given featureset cust_trans_fs = client.get_feature_set(name="customer_transactions") - jobs = client.list_ingest_jobs(feature_set=cust_trans_fs) - assert len(jobs) >= 1 + ingest_jobs = client.list_ingest_jobs(feature_set=cust_trans_fs) + assert len(ingest_jobs) >= 1 - for job in jobs: - job.wait(IngestionJobStatus.RUNNING) - assert job.status == IngestionJobStatus.RUNNING + for ingest_job in ingest_jobs: + ingest_job.wait(IngestionJobStatus.RUNNING) + assert ingest_job.status == IngestionJobStatus.RUNNING - # stop ingestion job - client.stop_ingest_job(job) - job.wait(IngestionJobStatus.ABORTED) - assert job.status == IngestionJobStatus.ABORTED + # stop ingestion ingest_job + client.stop_ingest_job(ingest_job) + ingest_job.wait(IngestionJobStatus.ABORTED) + assert ingest_job.status == IngestionJobStatus.ABORTED - # restart ingestion job - client.restart_ingest_job(job) - job.wait(IngestionJobStatus.RUNNING) - assert job.status == IngestionJobStatus.RUNNING + # restart ingestion ingest_job + client.restart_ingest_job(ingest_job) + ingest_job.wait(IngestionJobStatus.RUNNING) + assert ingest_job.status == IngestionJobStatus.RUNNING @pytest.fixture(scope='module') @@ -333,6 +333,27 @@ def test_all_types_retrieve_online_success(client, all_types_dataframe): ): break +@pytest.mark.timeout(300) +@pytest.mark.run(order=23) +def test_all_types_ingest_jobs(client, basic_dataframe): + # list ingestion jobs given featureset + all_types_fs = client.get_feature_set(name="all_types") + ingest_jobs = client.list_ingest_jobs(feature_set=all_types_fs) + assert len(ingest_jobs) >= 1 + + for ingest_job in ingest_jobs: + ingest_job.wait(IngestionJobStatus.RUNNING) + assert ingest_job.status == IngestionJobStatus.RUNNING + + # stop ingestion ingest_job + client.stop_ingest_job(ingest_job) + ingest_job.wait(IngestionJobStatus.ABORTED) + assert ingest_job.status == IngestionJobStatus.ABORTED + + # restart ingestion ingest_job + client.restart_ingest_job(ingest_job) + ingest_job.wait(IngestionJobStatus.RUNNING) + assert ingest_job.status == IngestionJobStatus.RUNNING @pytest.fixture(scope='module') def large_volume_dataframe(): @@ -488,7 +509,6 @@ def all_types_parquet_file(): df.to_parquet(file_path, allow_truncated_timestamps=True) return file_path - @pytest.mark.timeout(300) @pytest.mark.run(order=40) def test_all_types_parquet_register_feature_set_success(client): From 9999db5246b4e988d859bbac8d67d67ef2929df0 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Tue, 31 Mar 2020 10:37:17 +0800 Subject: [PATCH 48/66] Fixed typo in function arguments --- tests/e2e/basic-ingest-redis-serving.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/e2e/basic-ingest-redis-serving.py b/tests/e2e/basic-ingest-redis-serving.py index f63393aa4a4..a55d572f3da 100644 --- a/tests/e2e/basic-ingest-redis-serving.py +++ b/tests/e2e/basic-ingest-redis-serving.py @@ -335,7 +335,7 @@ def test_all_types_retrieve_online_success(client, all_types_dataframe): @pytest.mark.timeout(300) @pytest.mark.run(order=23) -def test_all_types_ingest_jobs(client, basic_dataframe): +def test_all_types_ingest_jobs(client, all_types_dataframe): # list ingestion jobs given featureset all_types_fs = client.get_feature_set(name="all_types") ingest_jobs = client.list_ingest_jobs(feature_set=all_types_fs) From f01262303edc5e11cfc75d5ac80fceaa50c51132 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Tue, 31 Mar 2020 12:29:44 +0800 Subject: [PATCH 49/66] Added unit tests for Ingestion Job API additions in python sdk --- sdk/python/tests/test_client.py | 105 +++++++++++++++++++++++++++++++- 1 file changed, 104 insertions(+), 1 deletion(-) diff --git a/sdk/python/tests/test_client.py b/sdk/python/tests/test_client.py index 3c1e8bef0f0..535fce439a0 100644 --- a/sdk/python/tests/test_client.py +++ b/sdk/python/tests/test_client.py @@ -29,6 +29,12 @@ from feast.core.CoreService_pb2 import ( GetFeastCoreVersionResponse, GetFeatureSetResponse, + ListIngestionJobsResponse, +) +from feast.core.Store_pb2 import Store +from feast.core.IngestionJob_pb2 import ( + IngestionJob as IngestJobProto, + IngestionJobStatus, ) from feast.core.FeatureSet_pb2 import EntitySpec as EntitySpecProto from feast.core.FeatureSet_pb2 import FeatureSet as FeatureSetProto @@ -39,6 +45,7 @@ from feast.core.Source_pb2 import KafkaSourceConfig, Source, SourceType from feast.entity import Entity from feast.feature_set import Feature, FeatureSet +from feast.job import IngestJob from feast.serving.ServingService_pb2 import ( GetFeastServingInfoResponse, GetOnlineFeaturesRequest, @@ -295,7 +302,103 @@ def test_get_feature_set(self, mocked_client, mocker): and len(feature_set.entities) == 1 ) - # @pytest.mark.parametrize( + @pytest.mark.parametrize( + "mocked_client", + [pytest.lazy_fixture("mock_client"), pytest.lazy_fixture("secure_mock_client")], + ) + def test_list_ingest_jobs(self, mocked_client, mocker): + mocker.patch.object( + mocked_client, + "_core_service_stub", + return_value=Core.CoreServiceStub(grpc.insecure_channel("")), + ) + mocker.patch.object( + mocked_client._core_service_stub, + "ListIngestionJobs", + return_value=ListIngestionJobsResponse( + jobs=[ + IngestJobProto( + id="kafka-to-redis", + external_id="job-2222", + status=IngestionJobStatus.RUNNING, + feature_sets=[ + FeatureSetProto( + spec=FeatureSetSpecProto( + name="driver", max_age=Duration(seconds=3600), + ) + ) + ], + source=Source( + type=SourceType.KAFKA, + kafka_source_config=KafkaSourceConfig( + bootstrap_servers="localhost:9092", topic="topic" + ), + ), + store=Store(name="redis"), + ) + ] + ), + ) + + ingest_jobs = mocked_client.list_ingest_jobs(job_id="kafka-to-redis") + assert len(ingest_jobs) >= 1 + + ingest_job = ingest_jobs[0] + assert ( + ingest_job.status == IngestionJobStatus.RUNNING + and ingest_job.id == "kafka-to-redis" + and ingest_job.external_id == "job-2222" + and ingest_job.feature_sets[0].name == "driver" + and ingest_job.source.source_type == "Kafka" + ) + + @pytest.mark.parametrize( + "mocked_client", + [pytest.lazy_fixture("mock_client"), pytest.lazy_fixture("secure_mock_client")], + ) + def test_restart_ingestion_job(self, mocked_client, mocker): + mocker.patch.object( + mocked_client, + "_core_service_stub", + return_value=Core.CoreServiceStub(grpc.insecure_channel("")), + ) + + ingest_job = IngestJob( + job_proto=IngestJobProto( + id="kafka-to-redis", + external_id="job#2222", + status=IngestionJobStatus.ERROR, + ), + core_stub=mocked_client._core_service_stub, + ) + + mocked_client.restart_ingest_job(ingest_job) + assert mocked_client._core_service_stub.RestartIngestionJob.called + + @pytest.mark.parametrize( + "mocked_client", + [pytest.lazy_fixture("mock_client"), pytest.lazy_fixture("secure_mock_client")], + ) + def test_stop_ingestion_job(self, mocked_client, mocker): + mocker.patch.object( + mocked_client, + "_core_service_stub", + return_value=Core.CoreServiceStub(grpc.insecure_channel("")), + ) + + ingest_job = IngestJob( + job_proto=IngestJobProto( + id="kafka-to-redis", + external_id="job#2222", + status=IngestionJobStatus.RUNNING, + ), + core_stub=mocked_client._core_service_stub, + ) + + mocked_client.stop_ingest_job(ingest_job) + assert mocked_client._core_service_stub.StopIngestionJob.called + + # @pytest.mark.parametrize # "mocked_client", # [pytest.lazy_fixture("mock_client"), pytest.lazy_fixture("secure_mock_client")], # ) From 2e819b2971e3e95c7eae894047c7a6609ccd8faf Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Tue, 31 Mar 2020 12:45:35 +0800 Subject: [PATCH 50/66] Rename "ingestion" to "ingest" for more consistent naming --- sdk/python/tests/test_client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/python/tests/test_client.py b/sdk/python/tests/test_client.py index 535fce439a0..e317b39dd79 100644 --- a/sdk/python/tests/test_client.py +++ b/sdk/python/tests/test_client.py @@ -356,7 +356,7 @@ def test_list_ingest_jobs(self, mocked_client, mocker): "mocked_client", [pytest.lazy_fixture("mock_client"), pytest.lazy_fixture("secure_mock_client")], ) - def test_restart_ingestion_job(self, mocked_client, mocker): + def test_restart_ingest_job(self, mocked_client, mocker): mocker.patch.object( mocked_client, "_core_service_stub", @@ -379,7 +379,7 @@ def test_restart_ingestion_job(self, mocked_client, mocker): "mocked_client", [pytest.lazy_fixture("mock_client"), pytest.lazy_fixture("secure_mock_client")], ) - def test_stop_ingestion_job(self, mocked_client, mocker): + def test_stop_ingest_job(self, mocked_client, mocker): mocker.patch.object( mocked_client, "_core_service_stub", From ba08c8a759f5fe1d8c08339380388161f2791b35 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Fri, 3 Apr 2020 11:50:11 +0800 Subject: [PATCH 51/66] Disable support for restarting Job in a terminal state due to possible race conditions --- core/src/main/java/feast/core/service/JobService.java | 6 ++++-- protos/feast/core/CoreService.proto | 4 ++-- sdk/python/feast/client.py | 5 ++--- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/feast/core/service/JobService.java b/core/src/main/java/feast/core/service/JobService.java index fe668aedfba..bf74b90e80c 100644 --- a/core/src/main/java/feast/core/service/JobService.java +++ b/core/src/main/java/feast/core/service/JobService.java @@ -165,9 +165,11 @@ public RestartIngestionJobResponse restartJob(RestartIngestionJobRequest request // check job status is valid for restarting Job job = getJob.get(); JobStatus status = job.getStatus(); - if (JobStatus.getTransitionalStates().contains(status) || status.equals(JobStatus.UNKNOWN)) { + if (JobStatus.getTransitionalStates().contains(status) + || JobStatus.getTerminalState().contains(status) + || status.equals(JobStatus.UNKNOWN)) { throw new UnsupportedOperationException( - "Restarting a job with a transitional or unknown status is unsupported"); + "Restarting a job with a transitional, terminal or unknown status is unsupported"); } // restart job with job manager diff --git a/protos/feast/core/CoreService.proto b/protos/feast/core/CoreService.proto index ea3e7bf8a7a..73165676645 100644 --- a/protos/feast/core/CoreService.proto +++ b/protos/feast/core/CoreService.proto @@ -84,8 +84,8 @@ service CoreService { // Restart an Ingestion Job. Restarts the ingestion job with the given job id. // NOTE: Data might be lost during the restart for some job runners. - // Just starts the job if the job is a terminal state (ie suspended or aborted). - // Does not support restarts a job in a transitional (ie pending, suspending, aborting) or unknown status + // Does not support stopping a job in a transitional (ie pending, suspending, aborting), + // terminal state (ie suspended or aborted) or unknown status rpc RestartIngestionJob(RestartIngestionJobRequest) returns (RestartIngestionJobResponse); // Stop an Ingestion Job. Stop (Aborts) the ingestion job with the given job id. diff --git a/sdk/python/feast/client.py b/sdk/python/feast/client.py index fdc5dd208a0..c1e87d9d031 100644 --- a/sdk/python/feast/client.py +++ b/sdk/python/feast/client.py @@ -691,9 +691,8 @@ def restart_ingest_job(self, job: IngestJob): """ Restart ingestion job currently registered in Feast. NOTE: Data might be lost during the restart for some job runners. - Just starts the job if the job is a terminal state (ie suspended or aborted). - Does not support restarts a job in a transitional (ie pending, suspending, aborting) - or in a unknown status + Does not support stopping a job in a transitional (ie pending, suspending, aborting), + terminal state (ie suspended or aborted) or unknown status Args: job: IngestJob to restart From 5fb774c8eb4d183a7c455d0692d8845ab75fa775 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Fri, 3 Apr 2020 12:00:35 +0800 Subject: [PATCH 52/66] Added __str__ and __repr__ to IngestJob to render ingestjob in human readable string --- sdk/python/feast/job.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/sdk/python/feast/job.py b/sdk/python/feast/job.py index 76783ea4e36..a2d6ca40169 100644 --- a/sdk/python/feast/job.py +++ b/sdk/python/feast/job.py @@ -7,6 +7,7 @@ import fastavro import pandas as pd from google.cloud import storage +from google.protobuf.json_format import MessageToJson from feast.feature_set import FeatureSet from feast.source import Source @@ -289,3 +290,12 @@ def wait( # raise error if timeout if elapsed > timeout: raise TimeoutError("Wait for IngestJob's status to transition timed out") + + def __str__(self): + # render the contents of ingest job as human readable string + self.reload() + return str(MessageToJson(self.proto)) + + def __repr__(self): + # render the ingest job as human readable string + return f"IngestJob<{self.id}>" From 80ef3c91d889e49a1b5aecde0f472396f2e22fcc Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Sat, 4 Apr 2020 19:22:07 +0800 Subject: [PATCH 53/66] Added FeatureSetRef to represent references to featursets --- sdk/python/feast/client.py | 11 ++--- sdk/python/feast/feature_set.py | 81 +++++++++++++++++++++++++++++---- 2 files changed, 76 insertions(+), 16 deletions(-) diff --git a/sdk/python/feast/client.py b/sdk/python/feast/client.py index c1e87d9d031..a29be38c18a 100644 --- a/sdk/python/feast/client.py +++ b/sdk/python/feast/client.py @@ -56,8 +56,7 @@ ) from feast.core.CoreService_pb2_grpc import CoreServiceStub from feast.core.FeatureSet_pb2 import FeatureSetStatus -from feast.core.FeatureSetReference_pb2 import FeatureSetReference -from feast.feature_set import Entity, FeatureSet +from feast.feature_set import Entity, FeatureSet, FeatureSetRef from feast.job import Job, IngestJob from feast.loaders.abstract_producer import get_producer from feast.loaders.file import export_source_to_staging_location @@ -671,13 +670,9 @@ def list_ingest_jobs( # construct list request feature_set_ref = None if feature_set is not None: - feature_set_ref = FeatureSetReference( - name=feature_set.name, - project=feature_set.project, - version=feature_set.version, - ) + feature_set_ref = FeatureSetRef.from_feature_set(feature_set).to_proto() list_filter = ListIngestionJobsRequest.Filter( - id=job_id, feature_set_reference=feature_set_ref, store_name=store_name + id=job_id, feature_set_reference=feature_set_ref, store_name=store_name, ) request = ListIngestionJobsRequest(filter=list_filter) # make list request & unpack response diff --git a/sdk/python/feast/feature_set.py b/sdk/python/feast/feature_set.py index 4ebfecf1675..5b74764b16e 100644 --- a/sdk/python/feast/feature_set.py +++ b/sdk/python/feast/feature_set.py @@ -27,6 +27,9 @@ from feast.core.FeatureSet_pb2 import FeatureSet as FeatureSetProto from feast.core.FeatureSet_pb2 import FeatureSetMeta as FeatureSetMetaProto from feast.core.FeatureSet_pb2 import FeatureSetSpec as FeatureSetSpecProto +from feast.core.FeatureSetReference_pb2 import ( + FeatureSetReference as FeatureSetReferenceProto, +) from feast.entity import Entity from feast.feature import Feature, Field from feast.loaders import yaml as feast_yaml @@ -88,14 +91,7 @@ def __str__(self): return str(MessageToJson(self.to_proto())) def __repr__(self): - ref = "" - if self.project: - ref += self.project + "/" - if self.name: - ref += self.name - if self.version: - ref += ":" + str(self.version).strip() - return ref + return FeatureSetRef.from_feature_set(self).__repr__() @property def fields(self) -> Dict[str, Field]: @@ -761,6 +757,75 @@ def to_proto(self) -> FeatureSetProto: return FeatureSetProto(spec=spec, meta=meta) +class FeatureSetRef: + """ + Represents a reference to a featureset + """ + + def __init__(self, project: str = None, name: str = None, version: int = None): + self.proto = FeatureSetReferenceProto( + project=project, name=name, version=version + ) + + @classmethod + def from_feature_set(cls, feature_set: FeatureSet): + """ + Construct a feature set reference that refers to the given feature set. + + Args: + feature_set: Feature set to create reference from. + + Returns: + FeatureSetRef that refers to the given feature set + """ + return cls(feature_set.project, feature_set.name, feature_set.version) + + @classmethod + def from_str(cls, ref_str: str): + """ + Parse a feature reference from string representation. + (as defined by __repr__()) + + Args: + ref_str: string representation of the reference. + + Returns: + FeatureSetRef constructed from the string + """ + if "/" in ref_str: + project, ref_str = ref_str.split("/") + if ":" in ref_str: + ref_str, version = ref_str.split(":") + name = ref_str + + return cls(project, name, version) + + def to_proto(self, arg1): + """ + Convert and return this feature set reference to protobuf. + + Returns: + Protobuf version of this feature set reference. + """ + return self.proto + + def __str__(self): + # human readable string of the reference + return f"FeatureSetRef<{self.__repr__()}>" + + def __repr__(self): + # return string representation of the reference + # [project/]name[:version] + ref_str = "" + if self.proto.project: + ref_str += self.proto.project + "/" + if self.proto.name: + ref_str += self.proto.name + if self.proto.version: + ref_str += ":" + str(self.proto.version).strip() + return ref_str + + def _infer_pd_column_type(column, series, rows_to_sample): dtype = None sample_count = 0 From d3c9a3dbcc91495a933d1ae558779017db4c21ac Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 6 Apr 2020 10:55:48 +0800 Subject: [PATCH 54/66] Admend client's list_ingest_jobs() to accept feature references directly --- sdk/python/feast/client.py | 9 +++++---- sdk/python/tests/test_client.py | 24 +++++++++++++++--------- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/sdk/python/feast/client.py b/sdk/python/feast/client.py index a29be38c18a..4458da243a9 100644 --- a/sdk/python/feast/client.py +++ b/sdk/python/feast/client.py @@ -652,7 +652,10 @@ def get_online_features( ) def list_ingest_jobs( - self, job_id: str = None, feature_set: FeatureSet = None, store_name: str = None + self, + job_id: str = None, + feature_set_ref: FeatureSetRef = None, + store_name: str = None, ): """ List the ingestion jobs currently registered in Feast, with optional filters. @@ -660,7 +663,7 @@ def list_ingest_jobs( Args: job_id: Select specific ingestion job with the given job_id - feature_set: Filter ingestion jobs by those tied to the given feature set. + feature_set_ref: Filter ingestion jobs by target feature set (via reference) store_name: Filter ingestion jobs by target feast store's name Returns: @@ -669,8 +672,6 @@ def list_ingest_jobs( self._connect_core() # construct list request feature_set_ref = None - if feature_set is not None: - feature_set_ref = FeatureSetRef.from_feature_set(feature_set).to_proto() list_filter = ListIngestionJobsRequest.Filter( id=job_id, feature_set_reference=feature_set_ref, store_name=store_name, ) diff --git a/sdk/python/tests/test_client.py b/sdk/python/tests/test_client.py index e317b39dd79..f7f5676ced5 100644 --- a/sdk/python/tests/test_client.py +++ b/sdk/python/tests/test_client.py @@ -44,7 +44,7 @@ from feast.core.FeatureSet_pb2 import FeatureSpec as FeatureSpecProto from feast.core.Source_pb2 import KafkaSourceConfig, Source, SourceType from feast.entity import Entity -from feast.feature_set import Feature, FeatureSet +from feast.feature_set import Feature, FeatureSet, FeatureSetRef from feast.job import IngestJob from feast.serving.ServingService_pb2 import ( GetFeastServingInfoResponse, @@ -312,6 +312,13 @@ def test_list_ingest_jobs(self, mocked_client, mocker): "_core_service_stub", return_value=Core.CoreServiceStub(grpc.insecure_channel("")), ) + + feature_set_proto = FeatureSetProto( + spec=FeatureSetSpecProto( + project="test", name="driver", max_age=Duration(seconds=3600), + ) + ) + mocker.patch.object( mocked_client._core_service_stub, "ListIngestionJobs", @@ -321,13 +328,7 @@ def test_list_ingest_jobs(self, mocked_client, mocker): id="kafka-to-redis", external_id="job-2222", status=IngestionJobStatus.RUNNING, - feature_sets=[ - FeatureSetProto( - spec=FeatureSetSpecProto( - name="driver", max_age=Duration(seconds=3600), - ) - ) - ], + feature_sets=[feature_set_proto], source=Source( type=SourceType.KAFKA, kafka_source_config=KafkaSourceConfig( @@ -340,7 +341,12 @@ def test_list_ingest_jobs(self, mocked_client, mocker): ), ) - ingest_jobs = mocked_client.list_ingest_jobs(job_id="kafka-to-redis") + # list ingestion jobs by target feature set reference + ingest_jobs = mocked_client.list_ingest_jobs( + feature_set_ref=FeatureSetRef.from_feature_set( + FeatureSet.from_proto(feature_set_proto) + ) + ) assert len(ingest_jobs) >= 1 ingest_job = ingest_jobs[0] From 0cfc32aa2a9231ae2aa3a4c8f2e043a06448262d Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 6 Apr 2020 11:20:22 +0800 Subject: [PATCH 55/66] Fixed typo in IngestJob.store property --- protos/feast/core/CoreService.proto | 6 +++--- sdk/python/feast/job.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/protos/feast/core/CoreService.proto b/protos/feast/core/CoreService.proto index 73165676645..b7760d0b9aa 100644 --- a/protos/feast/core/CoreService.proto +++ b/protos/feast/core/CoreService.proto @@ -242,11 +242,11 @@ message ListIngestionJobsRequest { Filter filter = 1; message Filter { - // Job ID assigned by Feast + // Filter by Job ID assigned by Feast string id = 1; - // Feature set reference + // Filter by ingestion job target feature set. FeatureSetReference feature_set_reference = 2; - // Name of store + // Filter by Name of store string store_name = 3; } } diff --git a/sdk/python/feast/job.py b/sdk/python/feast/job.py index a2d6ca40169..fb82afefeba 100644 --- a/sdk/python/feast/job.py +++ b/sdk/python/feast/job.py @@ -266,7 +266,7 @@ def store(self) -> Store: """ Getter for the IngestJob's target feast store. """ - return self.proto.source + return self.proto.store def wait( self, status: IngestionJobStatus, timeout: float = 300, interval: float = 5 From a540157554929105985358f5faed529cd6df8242 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 6 Apr 2020 13:18:33 +0800 Subject: [PATCH 56/66] Fixed issue with FeatureSetRef.from_str not converting version to int --- sdk/python/feast/feature_set.py | 35 +++++++++++++++++++++++++--- sdk/python/tests/test_feature_set.py | 19 ++++++++++++++- 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/sdk/python/feast/feature_set.py b/sdk/python/feast/feature_set.py index 5b74764b16e..c4cedaf6b2a 100644 --- a/sdk/python/feast/feature_set.py +++ b/sdk/python/feast/feature_set.py @@ -767,6 +767,27 @@ def __init__(self, project: str = None, name: str = None, version: int = None): project=project, name=name, version=version ) + @property + def project(self) -> str: + """ + Get the project of feature set referenced by this reference + """ + return self.proto.project + + @property + def name(self) -> str: + """ + Get the name of feature set referenced by this reference + """ + return self.proto.name + + @property + def version(self) -> int: + """ + Get the version of feature set referenced by this reference + """ + return self.proto.version + @classmethod def from_feature_set(cls, feature_set: FeatureSet): """ @@ -795,12 +816,12 @@ def from_str(cls, ref_str: str): if "/" in ref_str: project, ref_str = ref_str.split("/") if ":" in ref_str: - ref_str, version = ref_str.split(":") + ref_str, version_str = ref_str.split(":") name = ref_str - return cls(project, name, version) + return cls(project, name, int(version_str)) - def to_proto(self, arg1): + def to_proto(self, arg1) -> FeatureSetReferenceProto: """ Convert and return this feature set reference to protobuf. @@ -825,6 +846,14 @@ def __repr__(self): ref_str += ":" + str(self.proto.version).strip() return ref_str + def __eq__(self, other): + # compare with other feature set + return hash(self) == hash(other) + + def __hash__(self): + # hash this reference + return hash(repr(self)) + def _infer_pd_column_type(column, series, rows_to_sample): dtype = None diff --git a/sdk/python/tests/test_feature_set.py b/sdk/python/tests/test_feature_set.py index 2c539ebe0a7..bd31d712bb3 100644 --- a/sdk/python/tests/test_feature_set.py +++ b/sdk/python/tests/test_feature_set.py @@ -23,7 +23,7 @@ import feast.core.CoreService_pb2_grpc as Core from feast.client import Client from feast.entity import Entity -from feast.feature_set import Feature, FeatureSet +from feast.feature_set import Feature, FeatureSet, FeatureSetRef from feast.value_type import ValueType from feast_core_server import CoreServicer @@ -167,3 +167,20 @@ def test_add_features_from_df_success( ) assert len(my_feature_set.features) == feature_count assert len(my_feature_set.entities) == entity_count + + +class TestFeatureSetRef: + def test_from_feature_set(self): + feature_set = FeatureSet("test", "test") + feature_set.version = 2 + ref = FeatureSetRef.from_feature_set(feature_set) + + assert ref.name == "test" + assert ref.project == "test" + assert ref.version == 2 + + def test_str_ref(self): + original_ref = FeatureSetRef(project="test", name="test", version=2) + ref_str = repr(original_ref) + parsed_ref = FeatureSetRef.from_str(ref_str) + assert original_ref == parsed_ref From 355b571a6c59307acdda7e171531c9552a226610 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 6 Apr 2020 14:48:08 +0800 Subject: [PATCH 57/66] Make the grpc error message more apparent on stop_ingest_job() and restart_ingest_job() --- sdk/python/feast/client.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/sdk/python/feast/client.py b/sdk/python/feast/client.py index 4458da243a9..2279390fe91 100644 --- a/sdk/python/feast/client.py +++ b/sdk/python/feast/client.py @@ -695,7 +695,10 @@ def restart_ingest_job(self, job: IngestJob): """ self._connect_core() request = RestartIngestionJobRequest(id=job.id) - self._core_service_stub.RestartIngestionJob(request) + try: + self._core_service_stub.RestartIngestionJob(request) + except grpc.RpcError as e: + raise grpc.RpcError(e.details()) def stop_ingest_job(self, job: IngestJob): """ @@ -709,7 +712,10 @@ def stop_ingest_job(self, job: IngestJob): """ self._connect_core() request = StopIngestionJobRequest(id=job.id) - self._core_service_stub.StopIngestionJob(request) + try: + self._core_service_stub.StopIngestionJob(request) + except grpc.RpcError as e: + raise grpc.RpcError(e.details()) def ingest( self, From 05b33a6a20d762da7b6996d3dbe860d92964eca6 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 6 Apr 2020 15:03:18 +0800 Subject: [PATCH 58/66] Added feast ingest-job list, describe, stop, restart to CLI --- sdk/python/feast/cli.py | 117 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 114 insertions(+), 3 deletions(-) diff --git a/sdk/python/feast/cli.py b/sdk/python/feast/cli.py index dc4784b3025..cd1146b4810 100644 --- a/sdk/python/feast/cli.py +++ b/sdk/python/feast/cli.py @@ -22,8 +22,9 @@ from feast.client import Client from feast.config import Config -from feast.feature_set import FeatureSet +from feast.feature_set import FeatureSet, FeatureSetRef from feast.loaders.yaml import yaml_loader +from feast.core.IngestionJob_pb2 import IngestionJobStatus _logger = logging.getLogger(__name__) @@ -128,11 +129,11 @@ def feature_set_list(): table = [] for fs in feast_client.list_feature_sets(): - table.append([fs.name, fs.version]) + table.append([fs.name, fs.version, repr(fs)]) from tabulate import tabulate - print(tabulate(table, headers=["NAME", "VERSION"], tablefmt="plain")) + print(tabulate(table, headers=["NAME", "VERSION", "REFERENCE"], tablefmt="plain")) @feature_set.command("apply") @@ -214,6 +215,116 @@ def project_list(): print(tabulate(table, headers=["NAME"], tablefmt="plain")) +@cli.group(name="ingest-jobs") +def ingest_job(): + """ + Manage ingestion jobs + """ + pass + + +@ingest_job.command("list") +@click.option("--job-id", "-i", help="Show only ingestion jobs with the given job id") +@click.option( + "--feature-set-ref", + "-f", + help="Show only ingestion job targeting the feature set with the given reference", +) +@click.option( + "--store-name", + "-s", + help="List only ingestion job that ingest into feast store with given name", +) +# TODO: types +def ingest_job_list(job_id, feature_set_ref, store_name): + """ + List ingestion jobs + """ + # parse feature set reference + if feature_set_ref is not None: + feature_set_ref = FeatureSetRef.from_str(feature_set_ref) + + # pull & render ingestion jobs as a table + feast_client = Client() + table = [] + for ingest_job in feast_client.list_ingest_jobs( + job_id=job_id, feature_set_ref=feature_set_ref, store_name=store_name + ): + table.append([ingest_job.id, IngestionJobStatus.Name(ingest_job.status)]) + + from tabulate import tabulate + + print(tabulate(table, headers=["ID", "STATUS"], tablefmt="plain")) + + +@ingest_job.command("describe") +@click.argument("job_id") +def ingest_job_describe(job_id: str): + """ + Describe the ingestion job with the given id. + """ + # find ingestion job for id + feast_client = Client() + jobs = feast_client.list_ingest_jobs(job_id=job_id) + if len(jobs) < 1: + print(f"Ingestion Job with id {job_id} could not be found") + sys.exit(1) + job = jobs[0] + + # pretty render ingestion job as yaml + print( + yaml.dump(yaml.safe_load(str(job)), default_flow_style=False, sort_keys=False) + ) + + +@ingest_job.command("stop") +@click.option( + "--wait", "-w", is_flag=True, help="Wait for the ingestion job to fully stop." +) +@click.option( + "--timeout", + "-t", + default=600, + help="Timeout in seconds to wait for the job to stop.", +) +@click.argument("job_id") +def ingest_job_stop(wait: bool, timeout: int, job_id: str): + """ + Stop ingestion job for id. + """ + # find ingestion job for id + feast_client = Client() + jobs = feast_client.list_ingest_jobs(job_id=job_id) + if len(jobs) < 1: + print(f"Ingestion Job with id {job_id} could not be found") + sys.exit(1) + job = jobs[0] + + feast_client.stop_ingest_job(job) + + # wait for ingestion job to stop + if wait: + job.wait(IngestionJobStatus.ABORTED, timeout=timeout) + + +@ingest_job.command("restart") +@click.argument("job_id") +def ingest_job_restart(job_id: str): + """ + Restart job for id. + Waits for the job to fully restart. + """ + # find ingestion job for id + feast_client = Client() + jobs = feast_client.list_ingest_jobs(job_id=job_id) + if len(jobs) < 1: + print(f"Ingestion Job with id {job_id} could not be found") + sys.exit(1) + job = jobs[0] + + feast_client.restart_ingest_job(job) + + @cli.command() @click.option( "--name", "-n", help="Feature set name to ingest data into", required=True From 3b8198a439a0f0935b53b35a7d63eb8fa18d102d Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 6 Apr 2020 15:09:30 +0800 Subject: [PATCH 59/66] Rename Job to RetrievalJob to prevent confusion with IngestJob --- sdk/python/feast/client.py | 10 +++++----- sdk/python/feast/job.py | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/sdk/python/feast/client.py b/sdk/python/feast/client.py index 2279390fe91..f5aed118cfd 100644 --- a/sdk/python/feast/client.py +++ b/sdk/python/feast/client.py @@ -57,7 +57,7 @@ from feast.core.CoreService_pb2_grpc import CoreServiceStub from feast.core.FeatureSet_pb2 import FeatureSetStatus from feast.feature_set import Entity, FeatureSet, FeatureSetRef -from feast.job import Job, IngestJob +from feast.job import RetrievalJob, IngestJob from feast.loaders.abstract_producer import get_producer from feast.loaders.file import export_source_to_staging_location from feast.loaders.ingest import KAFKA_CHUNK_PRODUCTION_TIMEOUT, get_feature_row_chunks @@ -510,7 +510,7 @@ def get_batch_features( feature_refs: List[str], entity_rows: Union[pd.DataFrame, str], default_project: str = None, - ) -> Job: + ) -> RetrievalJob: """ Retrieves historical features from a Feast Serving deployment. @@ -528,8 +528,8 @@ def get_batch_features( default_project: Default project where feature values will be found. Returns: - feast.job.Job: - Returns a job object that can be used to monitor retrieval + feast.job.RetrievalJob: + Returns a retrival job object that can be used to monitor retrieval progress asynchronously, and can be used to materialize the results. @@ -609,7 +609,7 @@ def get_batch_features( # Retrieve Feast Job object to manage life cycle of retrieval response = self._serving_service_stub.GetBatchFeatures(request) - return Job(response.job, self._serving_service_stub) + return RetrievalJob(response.job, self._serving_service_stub) def get_online_features( self, diff --git a/sdk/python/feast/job.py b/sdk/python/feast/job.py index fb82afefeba..7baf68e2314 100644 --- a/sdk/python/feast/job.py +++ b/sdk/python/feast/job.py @@ -32,7 +32,7 @@ MAX_WAIT_INTERVAL_SEC: int = 60 -class Job: +class RetrievalJob: """ A class representing a job for feature retrieval in Feast. """ @@ -199,7 +199,7 @@ def __iter__(self): class IngestJob: """ - Defines a job that ingests feature data into feast. + Defines a job for feature ingestion in feast. """ def __init__(self, job_proto: IngestJobProto, core_stub: CoreServiceStub): From 19bd141c731b7377b694d3d04ce33389c14cdd1e Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 6 Apr 2020 16:37:33 +0800 Subject: [PATCH 60/66] Updated e2e tests to use FeatureSetRef in list_ingest_jobs() --- tests/e2e/basic-ingest-redis-serving.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/e2e/basic-ingest-redis-serving.py b/tests/e2e/basic-ingest-redis-serving.py index a55d572f3da..62af6a848b1 100644 --- a/tests/e2e/basic-ingest-redis-serving.py +++ b/tests/e2e/basic-ingest-redis-serving.py @@ -10,7 +10,7 @@ from feast.core.IngestionJob_pb2 import IngestionJobStatus from feast.types.Value_pb2 import Value as Value from feast.client import Client -from feast.feature_set import FeatureSet +from feast.feature_set import FeatureSet, FeatureSetRef from feast.type_map import ValueType from google.protobuf.duration_pb2 import Duration from datetime import datetime @@ -157,7 +157,8 @@ def test_basic_retrieve_online_success(client, basic_dataframe): def test_basic_ingest_jobs(client, basic_dataframe): # list ingestion jobs given featureset cust_trans_fs = client.get_feature_set(name="customer_transactions") - ingest_jobs = client.list_ingest_jobs(feature_set=cust_trans_fs) + ingest_jobs = client.list_ingest_jobs( + feature_set_ref=FeatureSetRef.from_feature_set(cust_trans_fs)) assert len(ingest_jobs) >= 1 for ingest_job in ingest_jobs: @@ -338,7 +339,8 @@ def test_all_types_retrieve_online_success(client, all_types_dataframe): def test_all_types_ingest_jobs(client, all_types_dataframe): # list ingestion jobs given featureset all_types_fs = client.get_feature_set(name="all_types") - ingest_jobs = client.list_ingest_jobs(feature_set=all_types_fs) + ingest_jobs = client.list_ingest_jobs( + feature_set_ref=FeatureSetRef.from_feature_set(all_types_fs)) assert len(ingest_jobs) >= 1 for ingest_job in ingest_jobs: From 9f6cf6a607572ff45a3762d8fc22331c5222cde1 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 6 Apr 2020 17:28:30 +0800 Subject: [PATCH 61/66] Fixed due e2e tests to cater to new limitations on stop_ingest_job() --- tests/e2e/basic-ingest-redis-serving.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/e2e/basic-ingest-redis-serving.py b/tests/e2e/basic-ingest-redis-serving.py index 62af6a848b1..c68878bf53e 100644 --- a/tests/e2e/basic-ingest-redis-serving.py +++ b/tests/e2e/basic-ingest-redis-serving.py @@ -153,7 +153,7 @@ def test_basic_retrieve_online_success(client, basic_dataframe): break @pytest.mark.timeout(300) -@pytest.mark.run(order=14) +@pytest.mark.run(order=19) def test_basic_ingest_jobs(client, basic_dataframe): # list ingestion jobs given featureset cust_trans_fs = client.get_feature_set(name="customer_transactions") @@ -165,16 +165,16 @@ def test_basic_ingest_jobs(client, basic_dataframe): ingest_job.wait(IngestionJobStatus.RUNNING) assert ingest_job.status == IngestionJobStatus.RUNNING - # stop ingestion ingest_job - client.stop_ingest_job(ingest_job) - ingest_job.wait(IngestionJobStatus.ABORTED) - assert ingest_job.status == IngestionJobStatus.ABORTED - # restart ingestion ingest_job client.restart_ingest_job(ingest_job) ingest_job.wait(IngestionJobStatus.RUNNING) assert ingest_job.status == IngestionJobStatus.RUNNING + # stop ingestion ingest_job + client.stop_ingest_job(ingest_job) + ingest_job.wait(IngestionJobStatus.ABORTED) + assert ingest_job.status == IngestionJobStatus.ABORTED + @pytest.fixture(scope='module') def all_types_dataframe(): @@ -335,7 +335,7 @@ def test_all_types_retrieve_online_success(client, all_types_dataframe): break @pytest.mark.timeout(300) -@pytest.mark.run(order=23) +@pytest.mark.run(order=29) def test_all_types_ingest_jobs(client, all_types_dataframe): # list ingestion jobs given featureset all_types_fs = client.get_feature_set(name="all_types") @@ -347,16 +347,16 @@ def test_all_types_ingest_jobs(client, all_types_dataframe): ingest_job.wait(IngestionJobStatus.RUNNING) assert ingest_job.status == IngestionJobStatus.RUNNING - # stop ingestion ingest_job - client.stop_ingest_job(ingest_job) - ingest_job.wait(IngestionJobStatus.ABORTED) - assert ingest_job.status == IngestionJobStatus.ABORTED - # restart ingestion ingest_job client.restart_ingest_job(ingest_job) ingest_job.wait(IngestionJobStatus.RUNNING) assert ingest_job.status == IngestionJobStatus.RUNNING + # stop ingestion ingest_job + client.stop_ingest_job(ingest_job) + ingest_job.wait(IngestionJobStatus.ABORTED) + assert ingest_job.status == IngestionJobStatus.ABORTED + @pytest.fixture(scope='module') def large_volume_dataframe(): ROW_COUNT = 100000 From 87054b98b1dda4adc4b076abffa5dc6830451112 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Mon, 6 Apr 2020 18:08:59 +0800 Subject: [PATCH 62/66] Increase timeout on test_all_types_ingest_jobs() e2e test. --- tests/e2e/basic-ingest-redis-serving.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/e2e/basic-ingest-redis-serving.py b/tests/e2e/basic-ingest-redis-serving.py index c68878bf53e..90091e4584d 100644 --- a/tests/e2e/basic-ingest-redis-serving.py +++ b/tests/e2e/basic-ingest-redis-serving.py @@ -334,7 +334,7 @@ def test_all_types_retrieve_online_success(client, all_types_dataframe): ): break -@pytest.mark.timeout(300) +@pytest.mark.timeout(600) @pytest.mark.run(order=29) def test_all_types_ingest_jobs(client, all_types_dataframe): # list ingestion jobs given featureset From 1848dd41396de4efcc16ef67e24122da7e2ca258 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Tue, 7 Apr 2020 10:03:49 +0800 Subject: [PATCH 63/66] Configure IngestJob.wait() to backoff with a exponentially larger wait duration --- sdk/python/feast/job.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/sdk/python/feast/job.py b/sdk/python/feast/job.py index 7baf68e2314..3576bc1b385 100644 --- a/sdk/python/feast/job.py +++ b/sdk/python/feast/job.py @@ -24,7 +24,7 @@ from feast.core.CoreService_pb2_grpc import CoreServiceStub from feast.core.CoreService_pb2 import ListIngestionJobsRequest -# Maximum no of seconds to wait until the jobs status is DONE in Feast +# Maximum no of seconds to wait until the retrieval jobs status is DONE in Feast # Currently set to the maximum query execution time limit in BigQuery DEFAULT_TIMEOUT_SEC: int = 21600 @@ -268,27 +268,27 @@ def store(self) -> Store: """ return self.proto.store - def wait( - self, status: IngestionJobStatus, timeout: float = 300, interval: float = 5 - ): + def wait(self, status: IngestionJobStatus, timeout_secs: float = 300): """ Wait for this IngestJob to transtion to the given status. Raises TimeoutError if the wait operation times out. Args: status: The IngestionJobStatus to wait for. - timeout: Maximum seconds to wait before timing out. - interval: The interval to wait between checking the IngestJob status. + timeout_secs: Maximum seconds to wait before timing out. """ # poll & wait for job status to transition wait_begin = time.time() - elapsed = 0 - while self.status != status and elapsed <= timeout: - time.sleep(interval) - elapsed = time.time() - wait_begin + wait_secs = 2 + elapsed_secs = 0 + while self.status != status and elapsed_secs <= timeout_secs: + time.sleep(wait_secs) + # back off wait duration exponentially, capped at MAX_WAIT_INTERVAL_SEC + wait_secs = min(wait_secs * 2, MAX_WAIT_INTERVAL_SEC) + elapsed_secs = time.time() - wait_begin # raise error if timeout - if elapsed > timeout: + if elapsed_secs > timeout_secs: raise TimeoutError("Wait for IngestJob's status to transition timed out") def __str__(self): From 146fb2bc327c427167b133dbae5742ed39e3477e Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Tue, 7 Apr 2020 10:06:46 +0800 Subject: [PATCH 64/66] Added print statements to debug e2e test failure. --- tests/e2e/basic-ingest-redis-serving.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/e2e/basic-ingest-redis-serving.py b/tests/e2e/basic-ingest-redis-serving.py index 90091e4584d..c1b76647f04 100644 --- a/tests/e2e/basic-ingest-redis-serving.py +++ b/tests/e2e/basic-ingest-redis-serving.py @@ -334,7 +334,7 @@ def test_all_types_retrieve_online_success(client, all_types_dataframe): ): break -@pytest.mark.timeout(600) +@pytest.mark.timeout(300) @pytest.mark.run(order=29) def test_all_types_ingest_jobs(client, all_types_dataframe): # list ingestion jobs given featureset @@ -344,6 +344,8 @@ def test_all_types_ingest_jobs(client, all_types_dataframe): assert len(ingest_jobs) >= 1 for ingest_job in ingest_jobs: + print("status: ", ingest_job.status) + print("job:", str(ingest_job)) ingest_job.wait(IngestionJobStatus.RUNNING) assert ingest_job.status == IngestionJobStatus.RUNNING From 0fa6b9340a59980dac2f0cd874839849a0308776 Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Tue, 7 Apr 2020 10:32:37 +0800 Subject: [PATCH 65/66] Revert "Added print statements to debug e2e test failure." This reverts commit 146fb2bc327c427167b133dbae5742ed39e3477e. --- tests/e2e/basic-ingest-redis-serving.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/e2e/basic-ingest-redis-serving.py b/tests/e2e/basic-ingest-redis-serving.py index c1b76647f04..90091e4584d 100644 --- a/tests/e2e/basic-ingest-redis-serving.py +++ b/tests/e2e/basic-ingest-redis-serving.py @@ -334,7 +334,7 @@ def test_all_types_retrieve_online_success(client, all_types_dataframe): ): break -@pytest.mark.timeout(300) +@pytest.mark.timeout(600) @pytest.mark.run(order=29) def test_all_types_ingest_jobs(client, all_types_dataframe): # list ingestion jobs given featureset @@ -344,8 +344,6 @@ def test_all_types_ingest_jobs(client, all_types_dataframe): assert len(ingest_jobs) >= 1 for ingest_job in ingest_jobs: - print("status: ", ingest_job.status) - print("job:", str(ingest_job)) ingest_job.wait(IngestionJobStatus.RUNNING) assert ingest_job.status == IngestionJobStatus.RUNNING From 66d3f77c8c2eeeea4de240ffa88cda4dbf5f211d Mon Sep 17 00:00:00 2001 From: Zhu Zhanyan Date: Tue, 7 Apr 2020 10:35:17 +0800 Subject: [PATCH 66/66] Fixed issue of test waiting for aborted job to become running causing timeout. --- tests/e2e/basic-ingest-redis-serving.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/e2e/basic-ingest-redis-serving.py b/tests/e2e/basic-ingest-redis-serving.py index 90091e4584d..8e40794344e 100644 --- a/tests/e2e/basic-ingest-redis-serving.py +++ b/tests/e2e/basic-ingest-redis-serving.py @@ -159,12 +159,11 @@ def test_basic_ingest_jobs(client, basic_dataframe): cust_trans_fs = client.get_feature_set(name="customer_transactions") ingest_jobs = client.list_ingest_jobs( feature_set_ref=FeatureSetRef.from_feature_set(cust_trans_fs)) + # filter ingestion jobs to only those that are running + ingest_jobs = [job for job in ingest_jobs if job.status == IngestionJobStatus.RUNNING] assert len(ingest_jobs) >= 1 for ingest_job in ingest_jobs: - ingest_job.wait(IngestionJobStatus.RUNNING) - assert ingest_job.status == IngestionJobStatus.RUNNING - # restart ingestion ingest_job client.restart_ingest_job(ingest_job) ingest_job.wait(IngestionJobStatus.RUNNING) @@ -334,19 +333,18 @@ def test_all_types_retrieve_online_success(client, all_types_dataframe): ): break -@pytest.mark.timeout(600) +@pytest.mark.timeout(300) @pytest.mark.run(order=29) def test_all_types_ingest_jobs(client, all_types_dataframe): # list ingestion jobs given featureset all_types_fs = client.get_feature_set(name="all_types") ingest_jobs = client.list_ingest_jobs( feature_set_ref=FeatureSetRef.from_feature_set(all_types_fs)) + # filter ingestion jobs to only those that are running + ingest_jobs = [job for job in ingest_jobs if job.status == IngestionJobStatus.RUNNING] assert len(ingest_jobs) >= 1 for ingest_job in ingest_jobs: - ingest_job.wait(IngestionJobStatus.RUNNING) - assert ingest_job.status == IngestionJobStatus.RUNNING - # restart ingestion ingest_job client.restart_ingest_job(ingest_job) ingest_job.wait(IngestionJobStatus.RUNNING)