From 6bd142ada15515ddc40bcf03149a9e39645ba6fa Mon Sep 17 00:00:00 2001 From: Willem Pienaar Date: Sat, 11 Apr 2020 10:50:25 +0800 Subject: [PATCH 01/17] Add validation to Core configuration and fix version loading Refactor, document, and validate Feast Core Properties Refactor FeastProperties to support nested store configuration Localize all store configuration in Serving in Spring configuration Various configuration updates * Allow Feast Serving to use types properties instead of maps * Reuse Feast Core Store model in serving * Remove redundant config classes for Redis * Update Serving Beans and Config classes to use ne1w configuration getters * Remove hot-loading from store configuration. This reduces a bit of flexibility, but simplifies the code and configuration --- core/pom.xml | 24 ++ .../feast/core/config/FeastProperties.java | 234 ++++++++++++++- .../core/config/FeatureStreamConfig.java | 9 +- .../java/feast/core/config/JobConfig.java | 17 +- .../core/job/dataflow/DataflowJobManager.java | 8 +- .../core/service/JobCoordinatorService.java | 11 +- core/src/main/resources/application.yml | 1 - .../service/JobCoordinatorServiceTest.java | 19 +- pom.xml | 8 + protos/feast/core/Store.proto | 3 + serving/pom.xml | 21 +- .../java/feast/serving/FeastProperties.java | 191 ------------ .../feast/serving/ServingApplication.java | 1 + .../ContextClosedHandler.java | 2 +- .../feast/serving/config/FeastProperties.java | 283 ++++++++++++++++++ .../InstrumentationConfig.java | 3 +- .../JobServiceConfig.java | 21 +- .../JobStoreConfig.java} | 27 +- .../ServingApiConfiguration.java | 2 +- .../ServingServiceConfig.java | 64 ++-- .../SpecServiceConfig.java | 14 +- .../redis/JobStoreRedisConfig.java | 68 ----- .../redis/ServingStoreRedisConfig.java | 62 ---- .../ServingServiceGRpcController.java | 2 +- .../ServingServiceRestController.java | 2 +- .../service/RedisBackedJobService.java | 5 + .../serving/specs/CachedSpecService.java | 39 +-- .../util/mappers/YamlToProtoMapper.java | 22 +- serving/src/main/resources/application.yml | 67 ++--- .../ServingServiceGRpcControllerTest.java | 2 +- .../service/CachedSpecServiceTest.java | 29 +- .../service/RedisBackedJobServiceTest.java | 3 +- .../redis/retriever/RedisOnlineRetriever.java | 11 + 33 files changed, 702 insertions(+), 573 deletions(-) delete mode 100644 serving/src/main/java/feast/serving/FeastProperties.java rename serving/src/main/java/feast/serving/{configuration => config}/ContextClosedHandler.java (96%) create mode 100644 serving/src/main/java/feast/serving/config/FeastProperties.java rename serving/src/main/java/feast/serving/{configuration => config}/InstrumentationConfig.java (96%) rename serving/src/main/java/feast/serving/{configuration => config}/JobServiceConfig.java (62%) rename serving/src/main/java/feast/serving/{configuration/StoreConfiguration.java => config/JobStoreConfig.java} (57%) rename serving/src/main/java/feast/serving/{configuration => config}/ServingApiConfiguration.java (97%) rename serving/src/main/java/feast/serving/{configuration => config}/ServingServiceConfig.java (64%) rename serving/src/main/java/feast/serving/{configuration => config}/SpecServiceConfig.java (90%) delete mode 100644 serving/src/main/java/feast/serving/configuration/redis/JobStoreRedisConfig.java delete mode 100644 serving/src/main/java/feast/serving/configuration/redis/ServingStoreRedisConfig.java diff --git a/core/pom.xml b/core/pom.xml index 7961b45074b..f4fb6c659c0 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -38,6 +38,14 @@ false + + + build-info + + build-info + + + @@ -207,5 +215,21 @@ jaxb-api + + javax.validation + validation-api + 2.0.0.Final + + + org.hibernate.validator + hibernate-validator + 6.1.2.Final + + + org.hibernate.validator + hibernate-validator-annotation-processor + 6.1.2.Final + + diff --git a/core/src/main/java/feast/core/config/FeastProperties.java b/core/src/main/java/feast/core/config/FeastProperties.java index b9c787b6c77..59324d9567e 100644 --- a/core/src/main/java/feast/core/config/FeastProperties.java +++ b/core/src/main/java/feast/core/config/FeastProperties.java @@ -16,53 +16,257 @@ */ package feast.core.config; +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; +import feast.core.config.FeastProperties.JobProperties.RunnerOptions; +import feast.core.config.FeastProperties.StreamProperties.FeatureStreamOptions; +import java.util.Arrays; +import java.util.HashMap; import java.util.Map; +import java.util.Objects; +import java.util.Set; +import javax.annotation.PostConstruct; +import javax.validation.ConstraintViolation; +import javax.validation.ConstraintViolationException; +import javax.validation.Validation; +import javax.validation.Validator; +import javax.validation.ValidatorFactory; +import javax.validation.constraints.AssertTrue; +import javax.validation.constraints.NotBlank; +import javax.validation.constraints.NotNull; +import javax.validation.constraints.Positive; import lombok.Getter; import lombok.Setter; +import org.hibernate.validator.constraints.URL; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.info.BuildProperties; @Getter @Setter @ConfigurationProperties(prefix = "feast", ignoreInvalidFields = true) public class FeastProperties { - private String version; - private JobProperties jobs; + @Autowired + public FeastProperties(BuildProperties buildProperties) { + setVersion(buildProperties.getVersion()); + } + + public FeastProperties() { + setVersion("unknown"); + } + + /* Feast Core Build Version */ + @NotBlank private String version; + + /* Population job properties */ + @NotNull private JobProperties jobs; + + @NotNull + /* Feast Kafka stream properties */ private StreamProperties stream; @Getter @Setter public static class JobProperties { + @NotBlank + /* Apache Beam runner type. Possible options: DirectRunner, DataflowRunner */ private String runner; - private Map options; + + /* Apache Beam runner options for population jobs */ + private RunnerOptions runnerOptions; + + /* (Optional) Additional arguments to pass to Beam population jobs */ + private Map extraRunnerOptions; + + @NotNull + /* Population job metric properties */ private MetricsProperties metrics; - private JobUpdatesProperties updates; - } - @Getter - @Setter - public static class JobUpdatesProperties { + /* Timeout in seconds for each attempt to update or submit a new job to the runner */ + @Positive private long jobUpdateTimeout; + + /* Job update polling interval in millisecond. How frequently Feast will update running jobs. */ + @Positive private long pollingIntervalMillis; + + /** Apache Beam runner options for population jobs */ + @Getter + @Setter + public static class RunnerOptions { + + /* (Dataflow Runner Only) Project id to use when launching jobs. */ + @NotBlank private String project; + + /* (Dataflow Runner Only) The Google Compute Engine region for creating Dataflow jobs. */ + @NotBlank private String region; + + /* (Dataflow Runner Only) GCP availability zone for operations. */ + @NotBlank private String zone; + + /* (Dataflow Runner Only) Run the job as a specific service account, instead of the default GCE robot. */ + @NotBlank private String serviceAccount; + + /* (Dataflow Runner Only) GCE network for launching workers. */ + @NotBlank private String network; + + /* (Dataflow Runner Only) GCE subnetwork for launching workers. */ + @NotBlank private String subnetwork; + + /* (Dataflow Runner Only) Machine type to create Dataflow worker VMs as. */ + private String workerMachineType; + + /* (Dataflow Runner Only) The autoscaling algorithm to use for the workerpool. */ + private String autoscalingAlgorithm; + + /* (Dataflow Runner Only) Specifies whether worker pools should be started with public IP addresses. */ + private Boolean usePublicIps; + + /** + * (Dataflow Runner Only) A pipeline level default location for storing temporary files. + * Support Google Cloud Storage locations, e.g. gs://bucket/object + */ + @NotBlank private String tempLocation; + + /* (Dataflow Runner Only) The maximum number of workers to use for the workerpool. */ + private Integer maxNumWorkers; - private long timeoutSeconds; - private long pollingIntervalMillis; + /** + * (Direct Runner Only) Controls the amount of target parallelism the DirectRunner will use. + * Defaults to the greater of the number of available processors and 3. Must be a value + * greater than zero. + */ + private Integer targetParallelism; + + /* BigQuery table specification, e.g. PROJECT_ID:DATASET_ID.PROJECT_ID */ + private String deadLetterTableSpec; + } + + public Map getRunnerOptionsMap() { + // First collect the existing "extra options" + Map combinedOptions = new HashMap(getExtraRunnerOptions()); + + // Convert all fields in RunnerOptions to and merge + ObjectMapper oMapper = new ObjectMapper(); + combinedOptions.putAll( + oMapper.convertValue( + getRunnerOptions(), new TypeReference>() {})); + + return combinedOptions; + } } + @AssertTrue + public boolean isValidJobRunnerSelected() { + String[] validRunners = new String[] {"DataflowRunner", "DirectRunner"}; + return Arrays.asList(validRunners).contains(getJobs().getRunner()); + } + + /** Properties used to configure Feast's managed Kafka feature stream. */ @Getter @Setter public static class StreamProperties { - private String type; - private Map options; + /* Feature stream type. Only "kafka" is supported. */ + @NotBlank private String type; + + /* Feature stream options */ + @NotNull private FeatureStreamOptions options; + + /** Feature stream options */ + @Getter + @Setter + public static class FeatureStreamOptions { + + /* Kafka topic to use for feature sets without source topics. */ + @NotBlank private String topic = "feast-features"; + + /** + * Comma separated list of Kafka bootstrap servers. Used for feature sets without a defined + * source. + */ + @NotBlank private String bootstrapServers = "localhost:9092"; + + /* Defines the number of copies of managed feature stream Kafka. */ + @Positive private short replicationFactor = 1; + + /* Number of Kafka partitions to to use for managed feature stream. */ + @Positive private int partitions = 1; + } } + @AssertTrue + public boolean isValidStreamTypeSelected() { + return Objects.equals(getStream().getType(), "kafka"); + } + + /** Feast population job metrics */ @Getter @Setter public static class MetricsProperties { + /* Population job metrics enabled */ private boolean enabled; - private String type; - private String host; - private int port; + + /* Metric type. Possible options: statsd */ + @NotBlank private String type; + + /* Host of metric sink */ + @URL private String host; + + /* Port of metric sink */ + @Positive private int port; + } + + /** + * Validates all FeastProperties. This method runs after properties have been initialized and + * individually and conditionally validates each class. + */ + @PostConstruct + public void validate() { + ValidatorFactory factory = Validation.buildDefaultValidatorFactory(); + Validator validator = factory.getValidator(); + + // Validate root fields in FeastProperties + Set> violations = validator.validate(this); + if (!violations.isEmpty()) { + throw new ConstraintViolationException(violations); + } + + // Validate Stream properties + Set> streamPropertyViolations = + validator.validate(getStream()); + if (!streamPropertyViolations.isEmpty()) { + throw new ConstraintViolationException(streamPropertyViolations); + } + + // Validate Stream Options + Set> featureStreamOptionsViolations = + validator.validate(getStream().getOptions()); + if (!featureStreamOptionsViolations.isEmpty()) { + throw new ConstraintViolationException(featureStreamOptionsViolations); + } + + // Validate JobProperties + Set> jobPropertiesViolations = validator.validate(getJobs()); + if (!jobPropertiesViolations.isEmpty()) { + throw new ConstraintViolationException(jobPropertiesViolations); + } + + // Validate RunnerOptions + Set> runnerOptionsViolations = + validator.validate(getJobs().getRunnerOptions()); + if (!runnerOptionsViolations.isEmpty()) { + throw new ConstraintViolationException(runnerOptionsViolations); + } + + // Validate MetricsProperties + if (getJobs().getMetrics().isEnabled()) { + Set> jobMetricViolations = + validator.validate(getJobs().getMetrics()); + if (!jobMetricViolations.isEmpty()) { + throw new ConstraintViolationException(jobMetricViolations); + } + } } } diff --git a/core/src/main/java/feast/core/config/FeatureStreamConfig.java b/core/src/main/java/feast/core/config/FeatureStreamConfig.java index 45de359ac76..44f0e0e0993 100644 --- a/core/src/main/java/feast/core/config/FeatureStreamConfig.java +++ b/core/src/main/java/feast/core/config/FeatureStreamConfig.java @@ -48,8 +48,8 @@ public Source getDefaultSource(FeastProperties feastProperties) { SourceType featureStreamType = SourceType.valueOf(streamProperties.getType().toUpperCase()); switch (featureStreamType) { case KAFKA: - String bootstrapServers = streamProperties.getOptions().get("bootstrapServers"); - String topicName = streamProperties.getOptions().get("topic"); + String bootstrapServers = streamProperties.getOptions().getBootstrapServers(); + String topicName = streamProperties.getOptions().getTopic(); Map map = new HashMap<>(); map.put(AdminClientConfig.BOOTSTRAP_SERVERS_CONFIG, bootstrapServers); map.put( @@ -59,9 +59,8 @@ public Source getDefaultSource(FeastProperties feastProperties) { NewTopic newTopic = new NewTopic( topicName, - Integer.valueOf(streamProperties.getOptions().getOrDefault("numPartitions", "1")), - Short.valueOf( - streamProperties.getOptions().getOrDefault("replicationFactor", "1"))); + streamProperties.getOptions().getPartitions(), + streamProperties.getOptions().getReplicationFactor()); CreateTopicsResult createTopicsResult = client.createTopics(Collections.singleton(newTopic)); try { diff --git a/core/src/main/java/feast/core/config/JobConfig.java b/core/src/main/java/feast/core/config/JobConfig.java index 728fc0545bf..85641681bff 100644 --- a/core/src/main/java/feast/core/config/JobConfig.java +++ b/core/src/main/java/feast/core/config/JobConfig.java @@ -23,7 +23,6 @@ import com.google.api.services.dataflow.DataflowScopes; import com.google.common.base.Strings; import feast.core.config.FeastProperties.JobProperties; -import feast.core.config.FeastProperties.JobUpdatesProperties; import feast.core.job.JobManager; import feast.core.job.Runner; import feast.core.job.dataflow.DataflowJobManager; @@ -31,7 +30,6 @@ import feast.core.job.direct.DirectRunnerJobManager; import java.io.IOException; import java.security.GeneralSecurityException; -import java.util.HashMap; import java.util.Map; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; @@ -55,10 +53,7 @@ public JobManager getJobManager( JobProperties jobProperties = feastProperties.getJobs(); Runner runner = Runner.fromString(jobProperties.getRunner()); - if (jobProperties.getOptions() == null) { - jobProperties.setOptions(new HashMap<>()); - } - Map jobOptions = jobProperties.getOptions(); + Map jobOptions = jobProperties.getRunnerOptionsMap(); switch (runner) { case DATAFLOW: if (Strings.isNullOrEmpty(jobOptions.getOrDefault("region", null)) @@ -77,7 +72,7 @@ public JobManager getJobManager( credential); return new DataflowJobManager( - dataflow, jobProperties.getOptions(), jobProperties.getMetrics()); + dataflow, jobProperties.getRunnerOptionsMap(), jobProperties.getMetrics()); } catch (IOException e) { throw new IllegalStateException( "Unable to find credential required for Dataflow monitoring API", e); @@ -88,7 +83,7 @@ public JobManager getJobManager( } case DIRECT: return new DirectRunnerJobManager( - jobProperties.getOptions(), directJobRegistry, jobProperties.getMetrics()); + jobProperties.getRunnerOptionsMap(), directJobRegistry, jobProperties.getMetrics()); default: throw new IllegalArgumentException("Unsupported runner: " + jobProperties.getRunner()); } @@ -99,10 +94,4 @@ public JobManager getJobManager( public DirectJobRegistry directJobRegistry() { return new DirectJobRegistry(); } - - /** Extracts job update options from feast core options. */ - @Bean - public JobUpdatesProperties jobUpdatesProperties(FeastProperties feastProperties) { - return feastProperties.getJobs().getUpdates(); - } } 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 c2313d75ecc..9dc3dc0b57c 100644 --- a/core/src/main/java/feast/core/job/dataflow/DataflowJobManager.java +++ b/core/src/main/java/feast/core/job/dataflow/DataflowJobManager.java @@ -60,12 +60,12 @@ public class DataflowJobManager implements JobManager { private final MetricsProperties metrics; public DataflowJobManager( - Dataflow dataflow, Map defaultOptions, MetricsProperties metricsProperties) { - this.defaultOptions = defaultOptions; + Dataflow dataflow, Map runnerOptions, MetricsProperties metricsProperties) { + this.defaultOptions = runnerOptions; this.dataflow = dataflow; this.metrics = metricsProperties; - this.projectId = defaultOptions.get("project"); - this.location = defaultOptions.get("region"); + this.projectId = runnerOptions.get("project"); + this.location = runnerOptions.get("region"); } @Override diff --git a/core/src/main/java/feast/core/service/JobCoordinatorService.java b/core/src/main/java/feast/core/service/JobCoordinatorService.java index b66d181022e..24115883ed2 100644 --- a/core/src/main/java/feast/core/service/JobCoordinatorService.java +++ b/core/src/main/java/feast/core/service/JobCoordinatorService.java @@ -24,7 +24,8 @@ import feast.core.FeatureSetProto.FeatureSetStatus; import feast.core.StoreProto; import feast.core.StoreProto.Store.Subscription; -import feast.core.config.FeastProperties.JobUpdatesProperties; +import feast.core.config.FeastProperties; +import feast.core.config.FeastProperties.JobProperties; import feast.core.dao.FeatureSetRepository; import feast.core.dao.JobRepository; import feast.core.job.JobManager; @@ -58,7 +59,7 @@ public class JobCoordinatorService { private FeatureSetRepository featureSetRepository; private SpecService specService; private JobManager jobManager; - private JobUpdatesProperties jobUpdatesProperties; + private JobProperties jobProperties; @Autowired public JobCoordinatorService( @@ -66,12 +67,12 @@ public JobCoordinatorService( FeatureSetRepository featureSetRepository, SpecService specService, JobManager jobManager, - JobUpdatesProperties jobUpdatesProperties) { + FeastProperties feastProperties) { this.jobRepository = jobRepository; this.featureSetRepository = featureSetRepository; this.specService = specService; this.jobManager = jobManager; - this.jobUpdatesProperties = jobUpdatesProperties; + this.jobProperties = feastProperties.getJobs(); } /** @@ -121,7 +122,7 @@ public void Poll() throws InvalidProtocolBufferException { store, originalJob, jobManager, - jobUpdatesProperties.getTimeoutSeconds())); + jobProperties.getJobUpdateTimeout())); }); } } diff --git a/core/src/main/resources/application.yml b/core/src/main/resources/application.yml index ee060fffc95..84aa79a6fc4 100644 --- a/core/src/main/resources/application.yml +++ b/core/src/main/resources/application.yml @@ -23,7 +23,6 @@ grpc: enable-reflection: true feast: -# version: @project.version@ jobs: # Runner type for feature population jobs. Currently supported runner types are # DirectRunner and DataflowRunner. diff --git a/core/src/test/java/feast/core/service/JobCoordinatorServiceTest.java b/core/src/test/java/feast/core/service/JobCoordinatorServiceTest.java index aa71f201dde..aed889af86e 100644 --- a/core/src/test/java/feast/core/service/JobCoordinatorServiceTest.java +++ b/core/src/test/java/feast/core/service/JobCoordinatorServiceTest.java @@ -39,7 +39,8 @@ import feast.core.StoreProto.Store.RedisConfig; import feast.core.StoreProto.Store.StoreType; import feast.core.StoreProto.Store.Subscription; -import feast.core.config.FeastProperties.JobUpdatesProperties; +import feast.core.config.FeastProperties; +import feast.core.config.FeastProperties.JobProperties; import feast.core.dao.FeatureSetRepository; import feast.core.dao.JobRepository; import feast.core.job.JobManager; @@ -65,13 +66,15 @@ public class JobCoordinatorServiceTest { @Mock SpecService specService; @Mock FeatureSetRepository featureSetRepository; - private JobUpdatesProperties jobUpdatesProperties; + private FeastProperties feastProperties; @Before public void setUp() { initMocks(this); - jobUpdatesProperties = new JobUpdatesProperties(); - jobUpdatesProperties.setTimeoutSeconds(5); + feastProperties = new FeastProperties(); + JobProperties jobProperties = new JobProperties(); + jobProperties.setJobUpdateTimeout(5); + feastProperties.setJobs(jobProperties); } @Test @@ -79,7 +82,7 @@ public void shouldDoNothingIfNoStoresFound() throws InvalidProtocolBufferExcepti when(specService.listStores(any())).thenReturn(ListStoresResponse.newBuilder().build()); JobCoordinatorService jcs = new JobCoordinatorService( - jobRepository, featureSetRepository, specService, jobManager, jobUpdatesProperties); + jobRepository, featureSetRepository, specService, jobManager, feastProperties); jcs.Poll(); verify(jobRepository, times(0)).saveAndFlush(any()); } @@ -105,7 +108,7 @@ public void shouldDoNothingIfNoMatchingFeatureSetsFound() throws InvalidProtocol .thenReturn(ListFeatureSetsResponse.newBuilder().build()); JobCoordinatorService jcs = new JobCoordinatorService( - jobRepository, featureSetRepository, specService, jobManager, jobUpdatesProperties); + jobRepository, featureSetRepository, specService, jobManager, feastProperties); jcs.Poll(); verify(jobRepository, times(0)).saveAndFlush(any()); } @@ -196,7 +199,7 @@ public void shouldGenerateAndSubmitJobsIfAny() throws InvalidProtocolBufferExcep JobCoordinatorService jcs = new JobCoordinatorService( - jobRepository, featureSetRepository, specService, jobManager, jobUpdatesProperties); + jobRepository, featureSetRepository, specService, jobManager, feastProperties); jcs.Poll(); verify(jobRepository, times(1)).saveAndFlush(jobArgCaptor.capture()); Job actual = jobArgCaptor.getValue(); @@ -318,7 +321,7 @@ public void shouldGroupJobsBySource() throws InvalidProtocolBufferException { JobCoordinatorService jcs = new JobCoordinatorService( - jobRepository, featureSetRepository, specService, jobManager, jobUpdatesProperties); + jobRepository, featureSetRepository, specService, jobManager, feastProperties); jcs.Poll(); verify(jobRepository, times(2)).saveAndFlush(jobArgCaptor.capture()); diff --git a/pom.xml b/pom.xml index 5a9ab5292ab..7b7cd1d0fed 100644 --- a/pom.xml +++ b/pom.xml @@ -486,6 +486,14 @@ true + + + build-info + + build-info + + + diff --git a/protos/feast/core/Store.proto b/protos/feast/core/Store.proto index 931a9d46b69..de9af0a99fe 100644 --- a/protos/feast/core/Store.proto +++ b/protos/feast/core/Store.proto @@ -120,6 +120,9 @@ message Store { message BigQueryConfig { string project_id = 1; string dataset_id = 2; + string staging_location = 3; + int32 initial_retry_delay_seconds = 4; + int32 total_timeout_seconds = 5; } message CassandraConfig { diff --git a/serving/pom.xml b/serving/pom.xml index 1390bfdc80c..1036f437de3 100644 --- a/serving/pom.xml +++ b/serving/pom.xml @@ -34,7 +34,7 @@ spring-plugins Spring Plugins - http://repo.spring.io/plugins-release + https://repo.spring.io/plugins-release @@ -46,6 +46,14 @@ false + + + build-info + + build-info + + + org.apache.maven.plugins @@ -76,6 +84,12 @@ ${project.version} + + dev.feast + feast-core + ${project.version} + + dev.feast feast-storage-api @@ -259,6 +273,11 @@ embedded-redis test + + org.projectlombok + lombok + compile + diff --git a/serving/src/main/java/feast/serving/FeastProperties.java b/serving/src/main/java/feast/serving/FeastProperties.java deleted file mode 100644 index 505d7d03301..00000000000 --- a/serving/src/main/java/feast/serving/FeastProperties.java +++ /dev/null @@ -1,191 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * Copyright 2018-2019 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.serving; - -// Feast configuration properties that maps Feast configuration from default application.yml file to -// a Java object. -// https://www.baeldung.com/configuration-properties-in-spring-boot -// https://docs.spring.io/spring-boot/docs/current/reference/html/boot-features-external-config.html#boot-features-external-config-typesafe-configuration-properties - -import java.util.Map; -import org.springframework.boot.context.properties.ConfigurationProperties; - -@ConfigurationProperties(prefix = "feast") -public class FeastProperties { - private String version; - private String coreHost; - private int coreGrpcPort; - private StoreProperties store; - private JobProperties jobs; - private TracingProperties tracing; - - public String getVersion() { - return this.version; - } - - public String getCoreHost() { - return this.coreHost; - } - - public int getCoreGrpcPort() { - return this.coreGrpcPort; - } - - public StoreProperties getStore() { - return this.store; - } - - public JobProperties getJobs() { - return this.jobs; - } - - public TracingProperties getTracing() { - return this.tracing; - } - - public void setVersion(String version) { - this.version = version; - } - - public void setCoreHost(String coreHost) { - this.coreHost = coreHost; - } - - public void setCoreGrpcPort(int coreGrpcPort) { - this.coreGrpcPort = coreGrpcPort; - } - - public void setStore(StoreProperties store) { - this.store = store; - } - - public void setJobs(JobProperties jobs) { - this.jobs = jobs; - } - - public void setTracing(TracingProperties tracing) { - this.tracing = tracing; - } - - public static class StoreProperties { - private String configPath; - private int redisPoolMaxSize; - private int redisPoolMaxIdle; - - public String getConfigPath() { - return this.configPath; - } - - public int getRedisPoolMaxSize() { - return this.redisPoolMaxSize; - } - - public int getRedisPoolMaxIdle() { - return this.redisPoolMaxIdle; - } - - public void setConfigPath(String configPath) { - this.configPath = configPath; - } - - public void setRedisPoolMaxSize(int redisPoolMaxSize) { - this.redisPoolMaxSize = redisPoolMaxSize; - } - - public void setRedisPoolMaxIdle(int redisPoolMaxIdle) { - this.redisPoolMaxIdle = redisPoolMaxIdle; - } - } - - public static class JobProperties { - private String stagingLocation; - private int bigqueryInitialRetryDelaySecs; - private int bigqueryTotalTimeoutSecs; - private String storeType; - private Map storeOptions; - - public String getStagingLocation() { - return this.stagingLocation; - } - - public int getBigqueryInitialRetryDelaySecs() { - return bigqueryInitialRetryDelaySecs; - } - - public int getBigqueryTotalTimeoutSecs() { - return bigqueryTotalTimeoutSecs; - } - - public String getStoreType() { - return this.storeType; - } - - public Map getStoreOptions() { - return this.storeOptions; - } - - public void setStagingLocation(String stagingLocation) { - this.stagingLocation = stagingLocation; - } - - public void setBigqueryInitialRetryDelaySecs(int bigqueryInitialRetryDelaySecs) { - this.bigqueryInitialRetryDelaySecs = bigqueryInitialRetryDelaySecs; - } - - public void setBigqueryTotalTimeoutSecs(int bigqueryTotalTimeoutSecs) { - this.bigqueryTotalTimeoutSecs = bigqueryTotalTimeoutSecs; - } - - public void setStoreType(String storeType) { - this.storeType = storeType; - } - - public void setStoreOptions(Map storeOptions) { - this.storeOptions = storeOptions; - } - } - - public static class TracingProperties { - private boolean enabled; - private String tracerName; - private String serviceName; - - public boolean isEnabled() { - return this.enabled; - } - - public String getTracerName() { - return this.tracerName; - } - - public String getServiceName() { - return this.serviceName; - } - - public void setEnabled(boolean enabled) { - this.enabled = enabled; - } - - public void setTracerName(String tracerName) { - this.tracerName = tracerName; - } - - public void setServiceName(String serviceName) { - this.serviceName = serviceName; - } - } -} diff --git a/serving/src/main/java/feast/serving/ServingApplication.java b/serving/src/main/java/feast/serving/ServingApplication.java index ae9bb87a0b5..064f7b3e8d8 100644 --- a/serving/src/main/java/feast/serving/ServingApplication.java +++ b/serving/src/main/java/feast/serving/ServingApplication.java @@ -16,6 +16,7 @@ */ package feast.serving; +import feast.serving.config.FeastProperties; import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.SpringBootApplication; import org.springframework.boot.context.properties.EnableConfigurationProperties; diff --git a/serving/src/main/java/feast/serving/configuration/ContextClosedHandler.java b/serving/src/main/java/feast/serving/config/ContextClosedHandler.java similarity index 96% rename from serving/src/main/java/feast/serving/configuration/ContextClosedHandler.java rename to serving/src/main/java/feast/serving/config/ContextClosedHandler.java index a4f6d64d84f..2bc97439f38 100644 --- a/serving/src/main/java/feast/serving/configuration/ContextClosedHandler.java +++ b/serving/src/main/java/feast/serving/config/ContextClosedHandler.java @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package feast.serving.configuration; +package feast.serving.config; import java.util.concurrent.ScheduledExecutorService; import org.springframework.beans.factory.annotation.Autowired; diff --git a/serving/src/main/java/feast/serving/config/FeastProperties.java b/serving/src/main/java/feast/serving/config/FeastProperties.java new file mode 100644 index 00000000000..088eef958d5 --- /dev/null +++ b/serving/src/main/java/feast/serving/config/FeastProperties.java @@ -0,0 +1,283 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright 2018-2019 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.serving.config; + +// Feast configuration properties that maps Feast configuration from default application.yml file to +// a Java object. +// https://www.baeldung.com/configuration-properties-in-spring-boot +// https://docs.spring.io/spring-boot/docs/current/reference/html/boot-features-external-config.html#boot-features-external-config-typesafe-configuration-properties + +import javax.validation.constraints.NotBlank; +import javax.validation.constraints.Positive; +import org.apache.logging.log4j.core.config.plugins.validation.constraints.ValidHost; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.info.BuildProperties; + +/** Feast Serving properties. */ +@ConfigurationProperties(prefix = "feast", ignoreInvalidFields = true) +public class FeastProperties { + + /** + * Instantiates a new Feast Serving properties. + * + * @param buildProperties the build properties + */ + @Autowired + public FeastProperties(BuildProperties buildProperties) { + setVersion(buildProperties.getVersion()); + } + + public FeastProperties() {} + + /* Feast Serving build version */ + @NotBlank private String version = "unknown"; + + /* Feast Core host to connect to. */ + @ValidHost @NotBlank private String coreHost; + + /* Feast Core port to connect to. */ + @Positive private int coreGrpcPort; + + /** + * The "store" string should contain a YAML representation of the store configuration. Store + * configurations can be seen in protos/feast/core/Store.proto + */ + private feast.core.model.Store store; + + /* Job Store properties to retain state of async jobs. */ + private JobStoreProperties jobStore; + + /* Metric tracing properties. */ + private TracingProperties tracing; + + /** + * Gets Serving store configuration deserialiazed as a {@link feast.core.model.Store}. + * + * @return the store + */ + public feast.core.model.Store getStore() { + return store; + } + + /** + * Gets Feast Serving build version. + * + * @return the build version + */ + public String getVersion() { + return version; + } + + /** + * Sets build version + * + * @param version the build version + */ + public void setVersion(String version) { + this.version = version; + } + + /** + * Gets Feast Core host. + * + * @return Feast Core host + */ + public String getCoreHost() { + return coreHost; + } + + /** + * Sets Feast Core host to connect to. + * + * @param coreHost Feast Core host + */ + public void setCoreHost(String coreHost) { + this.coreHost = coreHost; + } + + /** + * Gets Feast Core gRPC port. + * + * @return Port + */ + public int getCoreGrpcPort() { + return coreGrpcPort; + } + + /** + * Sets Feast Core gRPC port. + * + * @param coreGrpcPort gRPC port of Feast Core + */ + public void setCoreGrpcPort(int coreGrpcPort) { + this.coreGrpcPort = coreGrpcPort; + } + + /** + * Sets store properties. + * + * @param store properties comes from a YAML string + */ + public void setStore(feast.core.model.Store store) { + this.store = store; + } + + /** + * Gets job store properties + * + * @return the job store properties + */ + public JobStoreProperties getJobStore() { + return jobStore; + } + + /** + * Set job store properties + * + * @param jobStore Job store properties to set + */ + public void setJobStore(JobStoreProperties jobStore) { + this.jobStore = jobStore; + } + + /** + * Gets tracing properties + * + * @return tracing properties + */ + public TracingProperties getTracing() { + return tracing; + } + + public void setTracing(TracingProperties tracing) { + this.tracing = tracing; + } + + /** The type Job store properties. */ + public static class JobStoreProperties { + + /** Job Store Redis Host */ + private String redisHost; + + /** Job Store Redis Host */ + private int redisPort; + + /** + * Gets redis host. + * + * @return the redis host + */ + public String getRedisHost() { + return redisHost; + } + + /** + * Sets redis host. + * + * @param redisHost the redis host + */ + public void setRedisHost(String redisHost) { + this.redisHost = redisHost; + } + + /** + * Gets redis port. + * + * @return the redis port + */ + public int getRedisPort() { + return redisPort; + } + + /** + * Sets redis port. + * + * @param redisPort the redis port + */ + public void setRedisPort(int redisPort) { + this.redisPort = redisPort; + } + } + + /** Trace metric collection properties */ + public static class TracingProperties { + + /** Tracing enabled/disabled */ + private boolean enabled; + + /** Name of tracer to use (only "jaeger") */ + private String tracerName; + + /** Service name uniquely identifies this Feast Serving deployment */ + private String serviceName; + + /** + * Is tracing enabled + * + * @return boolean flag + */ + public boolean isEnabled() { + return enabled; + } + + /** + * Sets tracing enabled or disabled. + * + * @param enabled flag + */ + public void setEnabled(boolean enabled) { + this.enabled = enabled; + } + + /** + * Gets tracer name ('jaeger') + * + * @return the tracer name + */ + public String getTracerName() { + return tracerName; + } + + /** + * Sets tracer name. + * + * @param tracerName the tracer name + */ + public void setTracerName(String tracerName) { + this.tracerName = tracerName; + } + + /** + * Gets the service name. The service name uniquely identifies this Feast serving instance. + * + * @return the service name + */ + public String getServiceName() { + return serviceName; + } + + /** + * Sets service name. + * + * @param serviceName the service name + */ + public void setServiceName(String serviceName) { + this.serviceName = serviceName; + } + } +} diff --git a/serving/src/main/java/feast/serving/configuration/InstrumentationConfig.java b/serving/src/main/java/feast/serving/config/InstrumentationConfig.java similarity index 96% rename from serving/src/main/java/feast/serving/configuration/InstrumentationConfig.java rename to serving/src/main/java/feast/serving/config/InstrumentationConfig.java index 2cd284829c4..30269c5d0ec 100644 --- a/serving/src/main/java/feast/serving/configuration/InstrumentationConfig.java +++ b/serving/src/main/java/feast/serving/config/InstrumentationConfig.java @@ -14,9 +14,8 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package feast.serving.configuration; +package feast.serving.config; -import feast.serving.FeastProperties; import io.opentracing.Tracer; import io.opentracing.noop.NoopTracerFactory; import io.prometheus.client.exporter.MetricsServlet; diff --git a/serving/src/main/java/feast/serving/configuration/JobServiceConfig.java b/serving/src/main/java/feast/serving/config/JobServiceConfig.java similarity index 62% rename from serving/src/main/java/feast/serving/configuration/JobServiceConfig.java rename to serving/src/main/java/feast/serving/config/JobServiceConfig.java index fa94dab8329..f94a9c28c6e 100644 --- a/serving/src/main/java/feast/serving/configuration/JobServiceConfig.java +++ b/serving/src/main/java/feast/serving/config/JobServiceConfig.java @@ -14,10 +14,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package feast.serving.configuration; +package feast.serving.config; import feast.core.StoreProto.Store.StoreType; -import feast.serving.FeastProperties; import feast.serving.service.JobService; import feast.serving.service.NoopJobService; import feast.serving.service.RedisBackedJobService; @@ -29,24 +28,10 @@ public class JobServiceConfig { @Bean - public JobService jobService( - FeastProperties feastProperties, - CachedSpecService specService, - StoreConfiguration storeConfiguration) { + public JobService jobService(CachedSpecService specService, JobStoreConfig jobStoreConfig) { if (!specService.getStore().getType().equals(StoreType.BIGQUERY)) { return new NoopJobService(); } - StoreType storeType = StoreType.valueOf(feastProperties.getJobs().getStoreType()); - switch (storeType) { - case REDIS: - return new RedisBackedJobService(storeConfiguration.getJobStoreRedisConnection()); - case INVALID: - case BIGQUERY: - case CASSANDRA: - case UNRECOGNIZED: - default: - throw new IllegalArgumentException( - String.format("Unsupported store type '%s' for job store", storeType)); - } + return new RedisBackedJobService(jobStoreConfig); } } diff --git a/serving/src/main/java/feast/serving/configuration/StoreConfiguration.java b/serving/src/main/java/feast/serving/config/JobStoreConfig.java similarity index 57% rename from serving/src/main/java/feast/serving/configuration/StoreConfiguration.java rename to serving/src/main/java/feast/serving/config/JobStoreConfig.java index 84dc7b7f8d4..02bef55ddae 100644 --- a/serving/src/main/java/feast/serving/configuration/StoreConfiguration.java +++ b/serving/src/main/java/feast/serving/config/JobStoreConfig.java @@ -14,31 +14,30 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package feast.serving.configuration; +package feast.serving.config; +import io.lettuce.core.RedisClient; +import io.lettuce.core.RedisURI; import io.lettuce.core.api.StatefulRedisConnection; -import org.springframework.beans.factory.ObjectProvider; +import io.lettuce.core.codec.ByteArrayCodec; +import io.lettuce.core.resource.DefaultClientResources; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Configuration; @Configuration -public class StoreConfiguration { +public class JobStoreConfig { - // We can define other store specific beans here - // These beans can be autowired or can be created in this class. - private final StatefulRedisConnection servingRedisConnection; private final StatefulRedisConnection jobStoreRedisConnection; @Autowired - public StoreConfiguration( - ObjectProvider> servingRedisConnection, - ObjectProvider> jobStoreRedisConnection) { - this.servingRedisConnection = servingRedisConnection.getIfAvailable(); - this.jobStoreRedisConnection = jobStoreRedisConnection.getIfAvailable(); - } + public JobStoreConfig(FeastProperties feastProperties) { + RedisURI uri = + RedisURI.create( + feastProperties.getJobStore().getRedisHost(), + feastProperties.getJobStore().getRedisPort()); - public StatefulRedisConnection getServingRedisConnection() { - return servingRedisConnection; + jobStoreRedisConnection = + RedisClient.create(DefaultClientResources.create(), uri).connect(new ByteArrayCodec()); } public StatefulRedisConnection getJobStoreRedisConnection() { diff --git a/serving/src/main/java/feast/serving/configuration/ServingApiConfiguration.java b/serving/src/main/java/feast/serving/config/ServingApiConfiguration.java similarity index 97% rename from serving/src/main/java/feast/serving/configuration/ServingApiConfiguration.java rename to serving/src/main/java/feast/serving/config/ServingApiConfiguration.java index 539b25a0fcd..ce4fe134373 100644 --- a/serving/src/main/java/feast/serving/configuration/ServingApiConfiguration.java +++ b/serving/src/main/java/feast/serving/config/ServingApiConfiguration.java @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package feast.serving.configuration; +package feast.serving.config; import java.util.List; import org.springframework.beans.factory.annotation.Autowired; diff --git a/serving/src/main/java/feast/serving/configuration/ServingServiceConfig.java b/serving/src/main/java/feast/serving/config/ServingServiceConfig.java similarity index 64% rename from serving/src/main/java/feast/serving/configuration/ServingServiceConfig.java rename to serving/src/main/java/feast/serving/config/ServingServiceConfig.java index 28df853e224..376f26f81af 100644 --- a/serving/src/main/java/feast/serving/configuration/ServingServiceConfig.java +++ b/serving/src/main/java/feast/serving/config/ServingServiceConfig.java @@ -14,25 +14,26 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package feast.serving.configuration; +package feast.serving.config; import com.google.cloud.bigquery.BigQuery; import com.google.cloud.bigquery.BigQueryOptions; import com.google.cloud.storage.Storage; import com.google.cloud.storage.StorageOptions; -import feast.core.StoreProto.Store; +import com.google.protobuf.InvalidProtocolBufferException; +import feast.core.StoreProto; import feast.core.StoreProto.Store.BigQueryConfig; -import feast.core.StoreProto.Store.RedisConfig; -import feast.core.StoreProto.Store.Subscription; -import feast.serving.FeastProperties; -import feast.serving.service.*; +import feast.serving.service.HistoricalServingService; +import feast.serving.service.JobService; +import feast.serving.service.NoopJobService; +import feast.serving.service.OnlineServingService; +import feast.serving.service.ServingService; import feast.serving.specs.CachedSpecService; import feast.storage.api.retriever.HistoricalRetriever; import feast.storage.api.retriever.OnlineRetriever; import feast.storage.connectors.bigquery.retriever.BigQueryHistoricalRetriever; import feast.storage.connectors.redis.retriever.RedisOnlineRetriever; import io.opentracing.Tracer; -import java.util.Map; import org.slf4j.Logger; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -42,52 +43,27 @@ public class ServingServiceConfig { private static final Logger log = org.slf4j.LoggerFactory.getLogger(ServingServiceConfig.class); - private Store setStoreConfig(Store.Builder builder, Map options) { - switch (builder.getType()) { - case REDIS: - RedisConfig redisConfig = - RedisConfig.newBuilder() - .setHost(options.get("host")) - .setPort(Integer.parseInt(options.get("port"))) - .build(); - return builder.setRedisConfig(redisConfig).build(); - case BIGQUERY: - BigQueryConfig bqConfig = - BigQueryConfig.newBuilder() - .setProjectId(options.get("projectId")) - .setDatasetId(options.get("datasetId")) - .build(); - return builder.setBigqueryConfig(bqConfig).build(); - case CASSANDRA: - default: - throw new IllegalArgumentException( - String.format( - "Unsupported store %s provided, only REDIS or BIGQUERY are currently supported.", - builder.getType())); - } - } - @Bean public ServingService servingService( FeastProperties feastProperties, CachedSpecService specService, JobService jobService, - Tracer tracer, - StoreConfiguration storeConfiguration) { + Tracer tracer) + throws InvalidProtocolBufferException { ServingService servingService = null; - Store store = specService.getStore(); + StoreProto.Store store = feastProperties.getStore().toProto(); switch (store.getType()) { case REDIS: - OnlineRetriever redisRetriever = - new RedisOnlineRetriever(storeConfiguration.getServingRedisConnection()); + OnlineRetriever redisRetriever = new RedisOnlineRetriever(store.getRedisConfig()); servingService = new OnlineServingService(redisRetriever, specService, tracer); break; case BIGQUERY: BigQueryConfig bqConfig = store.getBigqueryConfig(); + String jobStagingLocation = bqConfig.getStagingLocation(); BigQuery bigquery = BigQueryOptions.getDefaultInstance().getService(); Storage storage = StorageOptions.getDefaultInstance().getService(); - String jobStagingLocation = feastProperties.getJobs().getStagingLocation(); + if (!jobStagingLocation.contains("://")) { throw new IllegalArgumentException( String.format("jobStagingLocation is not a valid URI: %s", jobStagingLocation)); @@ -110,10 +86,9 @@ public ServingService servingService( .setBigquery(bigquery) .setDatasetId(bqConfig.getDatasetId()) .setProjectId(bqConfig.getProjectId()) - .setJobStagingLocation(jobStagingLocation) - .setInitialRetryDelaySecs( - feastProperties.getJobs().getBigqueryInitialRetryDelaySecs()) - .setTotalTimeoutSecs(feastProperties.getJobs().getBigqueryTotalTimeoutSecs()) + .setJobStagingLocation(bqConfig.getStagingLocation()) + .setInitialRetryDelaySecs(bqConfig.getInitialRetryDelaySeconds()) + .setTotalTimeoutSecs(bqConfig.getTotalTimeoutSeconds()) .setStorage(storage) .build(); @@ -130,9 +105,4 @@ public ServingService servingService( return servingService; } - - private Subscription parseSubscription(String subscription) { - String[] split = subscription.split(":"); - return Subscription.newBuilder().setName(split[0]).setVersion(split[1]).build(); - } } diff --git a/serving/src/main/java/feast/serving/configuration/SpecServiceConfig.java b/serving/src/main/java/feast/serving/config/SpecServiceConfig.java similarity index 90% rename from serving/src/main/java/feast/serving/configuration/SpecServiceConfig.java rename to serving/src/main/java/feast/serving/config/SpecServiceConfig.java index 26ebfa956ca..2682e176d35 100644 --- a/serving/src/main/java/feast/serving/configuration/SpecServiceConfig.java +++ b/serving/src/main/java/feast/serving/config/SpecServiceConfig.java @@ -14,13 +14,12 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package feast.serving.configuration; +package feast.serving.config; -import feast.serving.FeastProperties; +import com.google.protobuf.InvalidProtocolBufferException; +import feast.core.StoreProto; import feast.serving.specs.CachedSpecService; import feast.serving.specs.CoreSpecService; -import java.nio.file.Path; -import java.nio.file.Paths; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; @@ -58,10 +57,11 @@ public ScheduledExecutorService cachedSpecServiceScheduledExecutorService( } @Bean - public CachedSpecService specService(FeastProperties feastProperties) { + public CachedSpecService specService(FeastProperties feastProperties) + throws InvalidProtocolBufferException { CoreSpecService coreService = new CoreSpecService(feastCoreHost, feastCorePort); - Path path = Paths.get(feastProperties.getStore().getConfigPath()); - CachedSpecService cachedSpecStorage = new CachedSpecService(coreService, path); + StoreProto.Store storeProto = feastProperties.getStore().toProto(); + CachedSpecService cachedSpecStorage = new CachedSpecService(coreService, storeProto); try { cachedSpecStorage.populateCache(); } catch (Exception e) { diff --git a/serving/src/main/java/feast/serving/configuration/redis/JobStoreRedisConfig.java b/serving/src/main/java/feast/serving/configuration/redis/JobStoreRedisConfig.java deleted file mode 100644 index 77d9262bcb3..00000000000 --- a/serving/src/main/java/feast/serving/configuration/redis/JobStoreRedisConfig.java +++ /dev/null @@ -1,68 +0,0 @@ -/* - * 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.serving.configuration.redis; - -import com.google.common.base.Enums; -import feast.core.StoreProto; -import feast.serving.FeastProperties; -import io.lettuce.core.RedisClient; -import io.lettuce.core.RedisURI; -import io.lettuce.core.api.StatefulRedisConnection; -import io.lettuce.core.codec.ByteArrayCodec; -import io.lettuce.core.resource.ClientResources; -import io.lettuce.core.resource.DefaultClientResources; -import java.util.Map; -import org.springframework.beans.factory.ObjectProvider; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; - -@Configuration -public class JobStoreRedisConfig { - - @Bean(destroyMethod = "shutdown") - ClientResources jobStoreClientResources() { - return DefaultClientResources.create(); - } - - @Bean(destroyMethod = "shutdown") - RedisClient jobStoreRedisClient( - ClientResources jobStoreClientResources, FeastProperties feastProperties) { - StoreProto.Store.StoreType storeType = - Enums.getIfPresent( - StoreProto.Store.StoreType.class, feastProperties.getJobs().getStoreType()) - .orNull(); - if (storeType != StoreProto.Store.StoreType.REDIS) return null; - Map jobStoreConf = feastProperties.getJobs().getStoreOptions(); - // If job conf is empty throw StoreException - if (jobStoreConf == null - || jobStoreConf.get("host") == null - || jobStoreConf.get("host").isEmpty() - || jobStoreConf.get("port") == null - || jobStoreConf.get("port").isEmpty()) - throw new IllegalArgumentException("Store Configuration is not set"); - RedisURI uri = - RedisURI.create(jobStoreConf.get("host"), Integer.parseInt(jobStoreConf.get("port"))); - return RedisClient.create(jobStoreClientResources, uri); - } - - @Bean(destroyMethod = "close") - StatefulRedisConnection jobStoreRedisConnection( - ObjectProvider jobStoreRedisClient) { - if (jobStoreRedisClient.getIfAvailable() == null) return null; - return jobStoreRedisClient.getIfAvailable().connect(new ByteArrayCodec()); - } -} diff --git a/serving/src/main/java/feast/serving/configuration/redis/ServingStoreRedisConfig.java b/serving/src/main/java/feast/serving/configuration/redis/ServingStoreRedisConfig.java deleted file mode 100644 index 17a50eef6d6..00000000000 --- a/serving/src/main/java/feast/serving/configuration/redis/ServingStoreRedisConfig.java +++ /dev/null @@ -1,62 +0,0 @@ -/* - * 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.serving.configuration.redis; - -import feast.core.StoreProto; -import feast.serving.specs.CachedSpecService; -import io.lettuce.core.RedisClient; -import io.lettuce.core.RedisURI; -import io.lettuce.core.api.StatefulRedisConnection; -import io.lettuce.core.codec.ByteArrayCodec; -import io.lettuce.core.resource.ClientResources; -import io.lettuce.core.resource.DefaultClientResources; -import org.springframework.beans.factory.ObjectProvider; -import org.springframework.context.annotation.*; - -@Configuration -public class ServingStoreRedisConfig { - - @Bean - StoreProto.Store.RedisConfig servingStoreRedisConf(CachedSpecService specService) { - if (specService.getStore().getType() != StoreProto.Store.StoreType.REDIS) return null; - return specService.getStore().getRedisConfig(); - } - - @Bean(destroyMethod = "shutdown") - ClientResources servingClientResources() { - return DefaultClientResources.create(); - } - - @Bean(destroyMethod = "shutdown") - RedisClient servingRedisClient( - ClientResources servingClientResources, - ObjectProvider servingStoreRedisConf) { - if (servingStoreRedisConf.getIfAvailable() == null) return null; - RedisURI redisURI = - RedisURI.create( - servingStoreRedisConf.getIfAvailable().getHost(), - servingStoreRedisConf.getIfAvailable().getPort()); - return RedisClient.create(servingClientResources, redisURI); - } - - @Bean(destroyMethod = "close") - StatefulRedisConnection servingRedisConnection( - ObjectProvider servingRedisClient) { - if (servingRedisClient.getIfAvailable() == null) return null; - return servingRedisClient.getIfAvailable().connect(new ByteArrayCodec()); - } -} diff --git a/serving/src/main/java/feast/serving/controller/ServingServiceGRpcController.java b/serving/src/main/java/feast/serving/controller/ServingServiceGRpcController.java index 0eba67d4b4e..91f38e2bd41 100644 --- a/serving/src/main/java/feast/serving/controller/ServingServiceGRpcController.java +++ b/serving/src/main/java/feast/serving/controller/ServingServiceGRpcController.java @@ -16,7 +16,6 @@ */ package feast.serving.controller; -import feast.serving.FeastProperties; import feast.serving.ServingAPIProto.GetBatchFeaturesRequest; import feast.serving.ServingAPIProto.GetBatchFeaturesResponse; import feast.serving.ServingAPIProto.GetFeastServingInfoRequest; @@ -26,6 +25,7 @@ import feast.serving.ServingAPIProto.GetOnlineFeaturesRequest; import feast.serving.ServingAPIProto.GetOnlineFeaturesResponse; import feast.serving.ServingServiceGrpc.ServingServiceImplBase; +import feast.serving.config.FeastProperties; import feast.serving.exception.SpecRetrievalException; import feast.serving.interceptors.GrpcMonitoringInterceptor; import feast.serving.service.ServingService; diff --git a/serving/src/main/java/feast/serving/controller/ServingServiceRestController.java b/serving/src/main/java/feast/serving/controller/ServingServiceRestController.java index b0e349fd6b0..344ab7cf3ae 100644 --- a/serving/src/main/java/feast/serving/controller/ServingServiceRestController.java +++ b/serving/src/main/java/feast/serving/controller/ServingServiceRestController.java @@ -18,11 +18,11 @@ import static feast.serving.util.mappers.ResponseJSONMapper.mapGetOnlineFeaturesResponse; -import feast.serving.FeastProperties; import feast.serving.ServingAPIProto.GetFeastServingInfoRequest; import feast.serving.ServingAPIProto.GetFeastServingInfoResponse; import feast.serving.ServingAPIProto.GetOnlineFeaturesRequest; import feast.serving.ServingAPIProto.GetOnlineFeaturesResponse; +import feast.serving.config.FeastProperties; import feast.serving.service.ServingService; import feast.serving.util.RequestHelper; import io.opentracing.Tracer; diff --git a/serving/src/main/java/feast/serving/service/RedisBackedJobService.java b/serving/src/main/java/feast/serving/service/RedisBackedJobService.java index 0bf53630379..99933585b27 100644 --- a/serving/src/main/java/feast/serving/service/RedisBackedJobService.java +++ b/serving/src/main/java/feast/serving/service/RedisBackedJobService.java @@ -19,6 +19,7 @@ import com.google.protobuf.util.JsonFormat; import feast.serving.ServingAPIProto.Job; import feast.serving.ServingAPIProto.Job.Builder; +import feast.serving.config.JobStoreConfig; import io.lettuce.core.api.StatefulRedisConnection; import io.lettuce.core.api.sync.RedisCommands; import java.util.Optional; @@ -37,6 +38,10 @@ public class RedisBackedJobService implements JobService { // and since users normally don't require info about relatively old jobs. private final int defaultExpirySeconds = (int) Duration.standardDays(1).getStandardSeconds(); + public RedisBackedJobService(JobStoreConfig jobStoreConfig) { + this.syncCommand = jobStoreConfig.getJobStoreRedisConnection().sync(); + } + public RedisBackedJobService(StatefulRedisConnection connection) { this.syncCommand = connection.sync(); } diff --git a/serving/src/main/java/feast/serving/specs/CachedSpecService.java b/serving/src/main/java/feast/serving/specs/CachedSpecService.java index 246be8c5fdd..2f68711bf20 100644 --- a/serving/src/main/java/feast/serving/specs/CachedSpecService.java +++ b/serving/src/main/java/feast/serving/specs/CachedSpecService.java @@ -18,7 +18,6 @@ import static feast.serving.util.RefUtil.generateFeatureSetStringRef; import static feast.serving.util.RefUtil.generateFeatureStringRef; -import static feast.serving.util.mappers.YamlToProtoMapper.yamlToStoreProto; import static java.util.Comparator.comparingInt; import static java.util.stream.Collectors.groupingBy; @@ -27,11 +26,10 @@ import com.google.common.cache.LoadingCache; import feast.core.CoreServiceProto.ListFeatureSetsRequest; import feast.core.CoreServiceProto.ListFeatureSetsResponse; -import feast.core.CoreServiceProto.UpdateStoreRequest; -import feast.core.CoreServiceProto.UpdateStoreResponse; import feast.core.FeatureSetProto.FeatureSet; import feast.core.FeatureSetProto.FeatureSetSpec; import feast.core.FeatureSetProto.FeatureSpec; +import feast.core.StoreProto; import feast.core.StoreProto.Store; import feast.core.StoreProto.Store.Subscription; import feast.serving.ServingAPIProto.FeatureReference; @@ -39,9 +37,6 @@ import feast.storage.api.retriever.FeatureSetRequest; import io.grpc.StatusRuntimeException; import io.prometheus.client.Gauge; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -59,7 +54,6 @@ public class CachedSpecService { private static final Logger log = org.slf4j.LoggerFactory.getLogger(CachedSpecService.class); private final CoreSpecService coreService; - private final Path configPath; private final Map featureToFeatureSetMapping; @@ -80,10 +74,9 @@ public class CachedSpecService { .help("epoch time of the last time the cache was updated") .register(); - public CachedSpecService(CoreSpecService coreService, Path configPath) { - this.configPath = configPath; + public CachedSpecService(CoreSpecService coreService, StoreProto.Store store) { this.coreService = coreService; - this.store = updateStore(readConfig(configPath)); + this.store = store; Map featureSets = getFeatureSetMap(); featureToFeatureSetMapping = @@ -156,7 +149,6 @@ public List getFeatureSets(List featureRefe * from core to preload the cache. */ public void populateCache() { - this.store = updateStore(readConfig(configPath)); Map featureSetMap = getFeatureSetMap(); featureSetCache.putAll(featureSetMap); featureToFeatureSetMapping.putAll(getFeatureToFeatureSetMapping(featureSetMap)); @@ -239,29 +231,4 @@ private Map getFeatureToFeatureSetMapping( }); return mapping; } - - private Store readConfig(Path path) { - try { - List fileContents = Files.readAllLines(path); - String yaml = fileContents.stream().reduce("", (l1, l2) -> l1 + "\n" + l2); - log.info("loaded store config at {}: \n{}", path.toString(), yaml); - return yamlToStoreProto(yaml); - } catch (IOException e) { - throw new RuntimeException( - String.format("Unable to read store config at %s", path.toAbsolutePath()), e); - } - } - - private Store updateStore(Store store) { - UpdateStoreRequest request = UpdateStoreRequest.newBuilder().setStore(store).build(); - try { - UpdateStoreResponse updateStoreResponse = coreService.updateStore(request); - if (!updateStoreResponse.getStore().equals(store)) { - throw new RuntimeException("Core store config not matching current store config"); - } - return updateStoreResponse.getStore(); - } catch (Exception e) { - throw new RuntimeException("Unable to update store configuration", e); - } - } } diff --git a/serving/src/main/java/feast/serving/util/mappers/YamlToProtoMapper.java b/serving/src/main/java/feast/serving/util/mappers/YamlToProtoMapper.java index 00ad1fabb1c..784a21552bd 100644 --- a/serving/src/main/java/feast/serving/util/mappers/YamlToProtoMapper.java +++ b/serving/src/main/java/feast/serving/util/mappers/YamlToProtoMapper.java @@ -19,19 +19,29 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; import com.google.protobuf.util.JsonFormat; +import feast.core.StoreProto; import feast.core.StoreProto.Store; import feast.core.StoreProto.Store.Builder; import java.io.IOException; +import org.slf4j.Logger; public class YamlToProtoMapper { + + private static final Logger log = org.slf4j.LoggerFactory.getLogger(YamlToProtoMapper.class); + private static final ObjectMapper yamlReader = new ObjectMapper(new YAMLFactory()); private static final ObjectMapper jsonWriter = new ObjectMapper(); - public static Store yamlToStoreProto(String yaml) throws IOException { - Object obj = yamlReader.readValue(yaml, Object.class); - String jsonString = jsonWriter.writeValueAsString(obj); - Builder builder = Store.newBuilder(); - JsonFormat.parser().merge(jsonString, builder); - return builder.build(); + public static Store yamlToStoreProto(String yaml) { + try { + Object obj = yamlReader.readValue(yaml, Object.class); + String jsonString = jsonWriter.writeValueAsString(obj); + Builder builder = Store.newBuilder(); + JsonFormat.parser().merge(jsonString, builder); + return builder.build(); + } catch (IOException e) { + log.error("Could not parse store configuration YAML", e); + return StoreProto.Store.getDefaultInstance(); + } } } diff --git a/serving/src/main/resources/application.yml b/serving/src/main/resources/application.yml index 96713c80287..264a98320f9 100644 --- a/serving/src/main/resources/application.yml +++ b/serving/src/main/resources/application.yml @@ -1,7 +1,4 @@ feast: - # This value is retrieved from project.version properties in pom.xml - # https://docs.spring.io/spring-boot/docs/current/reference/html/ - version: @project.version@ # GRPC service address for Feast Core # Feast Serving requires connection to Feast Core to retrieve and reload Feast metadata (e.g. FeatureSpecs, Store information) core-host: ${FEAST_CORE_HOST:localhost} @@ -18,40 +15,38 @@ feast: service-name: feast_serving store: - # Path containing the store configuration for this serving store. - config-path: ${FEAST_STORE_CONFIG_PATH:serving/sample_redis_config.yml} - # If serving redis, the redis pool max size - redis-pool-max-size: ${FEAST_REDIS_POOL_MAX_SIZE:128} - # If serving redis, the redis pool max idle conns - redis-pool-max-idle: ${FEAST_REDIS_POOL_MAX_IDLE:16} + name: serving + type: REDIS # Alternative, BIGQUERY + redis_config: + host: localhost + port: 6379 + bigquery_config: + # GCP Project + project_id: my_project - jobs: - # staging-location specifies the URI to store intermediate files for batch serving. - # Feast Serving client is expected to have read access to this staging location - # to download the batch features. - # - # For example: gs://mybucket/myprefix - # Please omit the trailing slash in the URI. - staging-location: ${FEAST_JOB_STAGING_LOCATION:} - # - # Retry options for BigQuery jobs: - bigquery-initial-retry-delay-secs: 1 - bigquery-total-timeout-secs: 21600 - # - # Type of store to store job metadata. This only needs to be set if the - # serving store type is Bigquery. - store-type: ${FEAST_JOB_STORE_TYPE:} - # - # Job store connection options. If the job store is redis, the following items are required: - # - # store-options: - # host: localhost - # port: 6379 - # Optionally, you can configure the connection pool with the following items: - # max-conn: 8 - # max-idle: 8 - # max-wait-millis: 50 - store-options: {} + # BigQuery Dataset Id + dataset_id: my_dataset + + # staging-location specifies the URI to store intermediate files for batch serving. + # Feast Serving client is expected to have read access to this staging location + # to download the batch features. + # For example: gs://mybucket/myprefix + # Please omit the trailing slash in the URI. + staging-location: ${FEAST_JOB_STAGING_LOCATION:} + + # Retry options for BigQuery retrieval jobs + bigquery-initial-retry-delay-secs: 1 + + # BigQuery timeout for retrieval jobs + bigquery-total-timeout-secs: 21600 + subscriptions: + - name: "*" + project: "*" + version: "*" + + job_store: + redis_host: localhost + redis_port: 6379 grpc: # The port number Feast Serving GRPC service should listen on diff --git a/serving/src/test/java/feast/serving/controller/ServingServiceGRpcControllerTest.java b/serving/src/test/java/feast/serving/controller/ServingServiceGRpcControllerTest.java index f2c51bc7dde..d23f9da1d25 100644 --- a/serving/src/test/java/feast/serving/controller/ServingServiceGRpcControllerTest.java +++ b/serving/src/test/java/feast/serving/controller/ServingServiceGRpcControllerTest.java @@ -19,11 +19,11 @@ import static org.mockito.MockitoAnnotations.initMocks; import com.google.protobuf.Timestamp; -import feast.serving.FeastProperties; import feast.serving.ServingAPIProto.FeatureReference; import feast.serving.ServingAPIProto.GetOnlineFeaturesRequest; import feast.serving.ServingAPIProto.GetOnlineFeaturesRequest.EntityRow; import feast.serving.ServingAPIProto.GetOnlineFeaturesResponse; +import feast.serving.config.FeastProperties; import feast.serving.service.ServingService; import feast.types.ValueProto.Value; import io.grpc.StatusRuntimeException; diff --git a/serving/src/test/java/feast/serving/service/CachedSpecServiceTest.java b/serving/src/test/java/feast/serving/service/CachedSpecServiceTest.java index 01c9304bda0..580b45a224b 100644 --- a/serving/src/test/java/feast/serving/service/CachedSpecServiceTest.java +++ b/serving/src/test/java/feast/serving/service/CachedSpecServiceTest.java @@ -46,7 +46,6 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -55,7 +54,6 @@ public class CachedSpecServiceTest { - private File configFile; private Store store; @Rule public final ExpectedException expectedException = ExpectedException.none(); @@ -66,27 +64,9 @@ public class CachedSpecServiceTest { private CachedSpecService cachedSpecService; @Before - public void setUp() throws IOException { + public void setUp() { initMocks(this); - configFile = File.createTempFile("serving", ".yml"); - String yamlString = - "name: SERVING\n" - + "type: REDIS\n" - + "redis_config:\n" - + " host: localhost\n" - + " port: 6379\n" - + "subscriptions:\n" - + "- project: project\n" - + " name: fs1\n" - + " version: \"*\"\n" - + "- project: project\n" - + " name: fs2\n" - + " version: \"*\""; - BufferedWriter writer = new BufferedWriter(new FileWriter(configFile)); - writer.write(yamlString); - writer.close(); - store = Store.newBuilder() .setName("SERVING") @@ -164,12 +144,7 @@ public void setUp() throws IOException { .build())) .thenReturn(ListFeatureSetsResponse.newBuilder().addAllFeatureSets(fs2FeatureSets).build()); - cachedSpecService = new CachedSpecService(coreService, configFile.toPath()); - } - - @After - public void tearDown() { - configFile.delete(); + cachedSpecService = new CachedSpecService(coreService, store); } @Test diff --git a/serving/src/test/java/feast/serving/service/RedisBackedJobServiceTest.java b/serving/src/test/java/feast/serving/service/RedisBackedJobServiceTest.java index 34bc31d2c26..23626c2cb85 100644 --- a/serving/src/test/java/feast/serving/service/RedisBackedJobServiceTest.java +++ b/serving/src/test/java/feast/serving/service/RedisBackedJobServiceTest.java @@ -26,6 +26,7 @@ import redis.embedded.RedisServer; public class RedisBackedJobServiceTest { + private static Integer REDIS_PORT = 51235; private RedisServer redis; @@ -41,7 +42,7 @@ public void teardown() { } @Test - public void shouldRecoverIfRedisConnectionIsLost() throws IOException { + public void shouldRecoverIfRedisConnectionIsLost() { RedisClient client = RedisClient.create(RedisURI.create("localhost", REDIS_PORT)); RedisBackedJobService jobService = new RedisBackedJobService(client.connect(new ByteArrayCodec())); diff --git a/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/retriever/RedisOnlineRetriever.java b/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/retriever/RedisOnlineRetriever.java index c8bb33de5fd..99de7f9112c 100644 --- a/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/retriever/RedisOnlineRetriever.java +++ b/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/retriever/RedisOnlineRetriever.java @@ -20,6 +20,7 @@ import com.google.protobuf.InvalidProtocolBufferException; import feast.core.FeatureSetProto.EntitySpec; import feast.core.FeatureSetProto.FeatureSetSpec; +import feast.core.StoreProto.Store.RedisConfig; import feast.serving.ServingAPIProto.FeatureReference; import feast.serving.ServingAPIProto.GetOnlineFeaturesRequest.EntityRow; import feast.storage.RedisProto.RedisKey; @@ -29,8 +30,11 @@ import feast.types.FieldProto.Field; import feast.types.ValueProto.Value; import io.grpc.Status; +import io.lettuce.core.RedisClient; +import io.lettuce.core.RedisURI; import io.lettuce.core.api.StatefulRedisConnection; import io.lettuce.core.api.sync.RedisCommands; +import io.lettuce.core.codec.ByteArrayCodec; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -45,6 +49,13 @@ public RedisOnlineRetriever(StatefulRedisConnection connection) this.syncCommands = connection.sync(); } + public RedisOnlineRetriever(RedisConfig config) { + StatefulRedisConnection connection = + RedisClient.create(RedisURI.create(config.getHost(), config.getPort())) + .connect(new ByteArrayCodec()); + this.syncCommands = connection.sync(); + } + /** * Gets online features from redis. This method returns a list of {@link FeatureRow}s * corresponding to each feature set spec. Each feature row in the list then corresponds to an From baf382534f6a9135d65bb4fff0a0b6843826cae5 Mon Sep 17 00:00:00 2001 From: Willem Pienaar Date: Sat, 11 Apr 2020 11:09:34 +0800 Subject: [PATCH 02/17] Set default build version in Feast Core "version" field in Feast Properties --- core/src/main/java/feast/core/config/FeastProperties.java | 6 ++---- .../java/feast/serving/service/CachedSpecServiceTest.java | 4 ---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/feast/core/config/FeastProperties.java b/core/src/main/java/feast/core/config/FeastProperties.java index 59324d9567e..b21ed254614 100644 --- a/core/src/main/java/feast/core/config/FeastProperties.java +++ b/core/src/main/java/feast/core/config/FeastProperties.java @@ -52,12 +52,10 @@ public FeastProperties(BuildProperties buildProperties) { setVersion(buildProperties.getVersion()); } - public FeastProperties() { - setVersion("unknown"); - } + public FeastProperties() {} /* Feast Core Build Version */ - @NotBlank private String version; + @NotBlank private String version = "unknown"; /* Population job properties */ @NotNull private JobProperties jobs; diff --git a/serving/src/test/java/feast/serving/service/CachedSpecServiceTest.java b/serving/src/test/java/feast/serving/service/CachedSpecServiceTest.java index 580b45a224b..f4f795ed32f 100644 --- a/serving/src/test/java/feast/serving/service/CachedSpecServiceTest.java +++ b/serving/src/test/java/feast/serving/service/CachedSpecServiceTest.java @@ -38,10 +38,6 @@ import feast.serving.specs.CachedSpecService; import feast.serving.specs.CoreSpecService; import feast.storage.api.retriever.FeatureSetRequest; -import java.io.BufferedWriter; -import java.io.File; -import java.io.FileWriter; -import java.io.IOException; import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; From 2514da5663e2983f3d41e54a369b1cbc8c7a8904 Mon Sep 17 00:00:00 2001 From: Willem Pienaar Date: Sun, 12 Apr 2020 09:24:44 +0800 Subject: [PATCH 03/17] Ensure FeatureSink creation is consistent for both Redis and BigQuery --- .../main/java/feast/ingestion/utils/StoreUtil.java | 7 ++----- .../bigquery/writer/BigQueryFeatureSink.java | 4 +++- .../connectors/redis/writer/RedisFeatureSink.java | 14 ++++++++++++++ 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/ingestion/src/main/java/feast/ingestion/utils/StoreUtil.java b/ingestion/src/main/java/feast/ingestion/utils/StoreUtil.java index 1b884333818..b62f83f0f30 100644 --- a/ingestion/src/main/java/feast/ingestion/utils/StoreUtil.java +++ b/ingestion/src/main/java/feast/ingestion/utils/StoreUtil.java @@ -83,12 +83,9 @@ public static FeatureSink getFeatureSink( StoreType storeType = store.getType(); switch (storeType) { case REDIS: - return RedisFeatureSink.builder() - .setRedisConfig(store.getRedisConfig()) - .setFeatureSetSpecs(featureSetSpecs) - .build(); + return RedisFeatureSink.fromConfig(store.getRedisConfig(), featureSetSpecs); case BIGQUERY: - return BigQueryFeatureSink.fromConfig(store.getBigqueryConfig()); + return BigQueryFeatureSink.fromConfig(store.getBigqueryConfig(), featureSetSpecs); default: throw new RuntimeException(String.format("Store type '{}' is unsupported", storeType)); } diff --git a/storage/connectors/bigquery/src/main/java/feast/storage/connectors/bigquery/writer/BigQueryFeatureSink.java b/storage/connectors/bigquery/src/main/java/feast/storage/connectors/bigquery/writer/BigQueryFeatureSink.java index 8860db2622a..d155d3f1f50 100644 --- a/storage/connectors/bigquery/src/main/java/feast/storage/connectors/bigquery/writer/BigQueryFeatureSink.java +++ b/storage/connectors/bigquery/src/main/java/feast/storage/connectors/bigquery/writer/BigQueryFeatureSink.java @@ -57,9 +57,11 @@ public abstract class BigQueryFeatureSink implements FeatureSink { * your own client. * * @param config {@link BigQueryConfig} + * @param featureSetSpecs * @return {@link BigQueryFeatureSink.Builder} */ - public static BigQueryFeatureSink fromConfig(BigQueryConfig config) { + public static FeatureSink fromConfig( + BigQueryConfig config, Map featureSetSpecs) { return builder() .setDatasetId(config.getDatasetId()) .setProjectId(config.getProjectId()) diff --git a/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/writer/RedisFeatureSink.java b/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/writer/RedisFeatureSink.java index 63c8c68d9bb..8801460231e 100644 --- a/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/writer/RedisFeatureSink.java +++ b/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/writer/RedisFeatureSink.java @@ -19,6 +19,7 @@ import com.google.auto.value.AutoValue; import feast.core.FeatureSetProto.FeatureSet; import feast.core.FeatureSetProto.FeatureSetSpec; +import feast.core.StoreProto; import feast.core.StoreProto.Store.RedisConfig; import feast.storage.api.writer.FeatureSink; import feast.storage.api.writer.WriteResult; @@ -33,6 +34,18 @@ @AutoValue public abstract class RedisFeatureSink implements FeatureSink { + /** + * Initialize a {@link RedisFeatureSink.Builder} from a {@link StoreProto.Store.RedisConfig}. + * + * @param redisConfig {@link RedisConfig} + * @param featureSetSpecs + * @return {@link RedisFeatureSink.Builder} + */ + public static FeatureSink fromConfig( + RedisConfig redisConfig, Map featureSetSpecs) { + return builder().setFeatureSetSpecs(featureSetSpecs).setRedisConfig(redisConfig).build(); + } + public abstract RedisConfig getRedisConfig(); public abstract Map getFeatureSetSpecs(); @@ -54,6 +67,7 @@ public abstract static class Builder { @Override public void prepareWrite(FeatureSet featureSet) { + RedisClient redisClient = RedisClient.create(RedisURI.create(getRedisConfig().getHost(), getRedisConfig().getPort())); try { From f38fb9127805c3ace33e6f5534bed9ee5ea86cde Mon Sep 17 00:00:00 2001 From: Willem Pienaar Date: Sun, 12 Apr 2020 09:41:45 +0800 Subject: [PATCH 04/17] Move BigQueryHistoricalRetriever configuration into Retriever from ServingServiceConfig --- .../serving/config/ServingServiceConfig.java | 36 ++----------------- .../BigQueryHistoricalRetriever.java | 32 +++++++++++++++++ 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/serving/src/main/java/feast/serving/config/ServingServiceConfig.java b/serving/src/main/java/feast/serving/config/ServingServiceConfig.java index 376f26f81af..4955757e973 100644 --- a/serving/src/main/java/feast/serving/config/ServingServiceConfig.java +++ b/serving/src/main/java/feast/serving/config/ServingServiceConfig.java @@ -16,13 +16,8 @@ */ package feast.serving.config; -import com.google.cloud.bigquery.BigQuery; -import com.google.cloud.bigquery.BigQueryOptions; -import com.google.cloud.storage.Storage; -import com.google.cloud.storage.StorageOptions; import com.google.protobuf.InvalidProtocolBufferException; import feast.core.StoreProto; -import feast.core.StoreProto.Store.BigQueryConfig; import feast.serving.service.HistoricalServingService; import feast.serving.service.JobService; import feast.serving.service.NoopJobService; @@ -59,39 +54,12 @@ public ServingService servingService( servingService = new OnlineServingService(redisRetriever, specService, tracer); break; case BIGQUERY: - BigQueryConfig bqConfig = store.getBigqueryConfig(); - String jobStagingLocation = bqConfig.getStagingLocation(); - BigQuery bigquery = BigQueryOptions.getDefaultInstance().getService(); - Storage storage = StorageOptions.getDefaultInstance().getService(); - - if (!jobStagingLocation.contains("://")) { - throw new IllegalArgumentException( - String.format("jobStagingLocation is not a valid URI: %s", jobStagingLocation)); - } - if (jobStagingLocation.endsWith("/")) { - jobStagingLocation = jobStagingLocation.substring(0, jobStagingLocation.length() - 1); - } - if (!jobStagingLocation.startsWith("gs://")) { - throw new IllegalArgumentException( - "Store type BIGQUERY requires job staging location to be a valid and existing Google Cloud Storage URI. Invalid staging location: " - + jobStagingLocation); - } if (jobService.getClass() == NoopJobService.class) { throw new IllegalArgumentException( - "Unable to instantiate jobService for BigQuery store."); + "Unable to instantiate JobService which is required by BigQueryHistoricalRetriever."); } - HistoricalRetriever bqRetriever = - BigQueryHistoricalRetriever.builder() - .setBigquery(bigquery) - .setDatasetId(bqConfig.getDatasetId()) - .setProjectId(bqConfig.getProjectId()) - .setJobStagingLocation(bqConfig.getStagingLocation()) - .setInitialRetryDelaySecs(bqConfig.getInitialRetryDelaySeconds()) - .setTotalTimeoutSecs(bqConfig.getTotalTimeoutSeconds()) - .setStorage(storage) - .build(); - + BigQueryHistoricalRetriever.fromConfig(store.getBigqueryConfig()); servingService = new HistoricalServingService(bqRetriever, specService, jobService); break; case CASSANDRA: diff --git a/storage/connectors/bigquery/src/main/java/feast/storage/connectors/bigquery/retriever/BigQueryHistoricalRetriever.java b/storage/connectors/bigquery/src/main/java/feast/storage/connectors/bigquery/retriever/BigQueryHistoricalRetriever.java index 27ba07e82ec..0c1d02cae41 100644 --- a/storage/connectors/bigquery/src/main/java/feast/storage/connectors/bigquery/retriever/BigQueryHistoricalRetriever.java +++ b/storage/connectors/bigquery/src/main/java/feast/storage/connectors/bigquery/retriever/BigQueryHistoricalRetriever.java @@ -24,6 +24,8 @@ import com.google.cloud.bigquery.*; import com.google.cloud.storage.Blob; import com.google.cloud.storage.Storage; +import com.google.cloud.storage.StorageOptions; +import feast.core.StoreProto; import feast.serving.ServingAPIProto; import feast.serving.ServingAPIProto.DatasetSource; import feast.storage.api.retriever.FeatureSetRequest; @@ -85,6 +87,36 @@ public abstract static class Builder { public abstract BigQueryHistoricalRetriever build(); } + public static BigQueryHistoricalRetriever fromConfig(StoreProto.Store.BigQueryConfig config) { + + String jobStagingLocation = config.getStagingLocation(); + BigQuery bigquery = BigQueryOptions.getDefaultInstance().getService(); + Storage storage = StorageOptions.getDefaultInstance().getService(); + + if (!jobStagingLocation.contains("://")) { + throw new IllegalArgumentException( + String.format("jobStagingLocation is not a valid URI: %s", jobStagingLocation)); + } + if (jobStagingLocation.endsWith("/")) { + jobStagingLocation = jobStagingLocation.substring(0, jobStagingLocation.length() - 1); + } + if (!jobStagingLocation.startsWith("gs://")) { + throw new IllegalArgumentException( + "Store type BIGQUERY requires job staging location to be a valid and existing Google Cloud Storage URI. Invalid staging location: " + + jobStagingLocation); + } + + return builder() + .setBigquery(bigquery) + .setDatasetId(config.getDatasetId()) + .setProjectId(config.getProjectId()) + .setJobStagingLocation(config.getStagingLocation()) + .setInitialRetryDelaySecs(config.getInitialRetryDelaySeconds()) + .setTotalTimeoutSecs(config.getTotalTimeoutSeconds()) + .setStorage(storage) + .build(); + } + @Override public String getStagingLocation() { return jobStagingLocation(); From d8170e49b94f1691507c0e6c6e0391b597c807c5 Mon Sep 17 00:00:00 2001 From: Willem Pienaar Date: Sun, 12 Apr 2020 10:06:40 +0800 Subject: [PATCH 05/17] Allow a list of stores to be configured for forward compability --- .../feast/serving/config/FeastProperties.java | 49 +++++++++++---- .../serving/config/ServingServiceConfig.java | 2 +- .../serving/config/SpecServiceConfig.java | 2 +- serving/src/main/resources/application.yml | 60 ++++++++++--------- 4 files changed, 72 insertions(+), 41 deletions(-) diff --git a/serving/src/main/java/feast/serving/config/FeastProperties.java b/serving/src/main/java/feast/serving/config/FeastProperties.java index 088eef958d5..8ead2d2a773 100644 --- a/serving/src/main/java/feast/serving/config/FeastProperties.java +++ b/serving/src/main/java/feast/serving/config/FeastProperties.java @@ -21,6 +21,8 @@ // https://www.baeldung.com/configuration-properties-in-spring-boot // https://docs.spring.io/spring-boot/docs/current/reference/html/boot-features-external-config.html#boot-features-external-config-typesafe-configuration-properties +import feast.core.model.Store; +import java.util.List; import javax.validation.constraints.NotBlank; import javax.validation.constraints.Positive; import org.apache.logging.log4j.core.config.plugins.validation.constraints.ValidHost; @@ -54,10 +56,35 @@ public FeastProperties() {} @Positive private int coreGrpcPort; /** - * The "store" string should contain a YAML representation of the store configuration. Store - * configurations can be seen in protos/feast/core/Store.proto + * Finds and returns the active store + * + * @return Returns the {@link Store} model object + */ + public Store getActiveStore() { + for (Store store : getStores()) { + if (activeStore.equals(store.getName())) { + return store; + } + } + throw new RuntimeException("Active store is either misconfigured."); + } + + /** + * Set the name of the active store found in the "stores" configuration list + * + * @param activeStore String name to active store + */ + public void setActiveStore(String activeStore) { + this.activeStore = activeStore; + } + + /** Name of the active store configuration (only one store can be active at a time). */ + @NotBlank private String activeStore; + + /** + * Collection of store configurations. The active store is selected by the "activeStore" field. */ - private feast.core.model.Store store; + private List stores; /* Job Store properties to retain state of async jobs. */ private JobStoreProperties jobStore; @@ -66,12 +93,12 @@ public FeastProperties() {} private TracingProperties tracing; /** - * Gets Serving store configuration deserialiazed as a {@link feast.core.model.Store}. + * Gets Serving store configuration as a list of {@link Store}. * - * @return the store + * @return List of stores objects */ - public feast.core.model.Store getStore() { - return store; + public List getStores() { + return stores; } /** @@ -129,12 +156,12 @@ public void setCoreGrpcPort(int coreGrpcPort) { } /** - * Sets store properties. + * Sets the collection of configured stores. * - * @param store properties comes from a YAML string + * @param stores List of {@link Store} */ - public void setStore(feast.core.model.Store store) { - this.store = store; + public void setStores(List stores) { + this.stores = stores; } /** diff --git a/serving/src/main/java/feast/serving/config/ServingServiceConfig.java b/serving/src/main/java/feast/serving/config/ServingServiceConfig.java index 4955757e973..80ecb936c01 100644 --- a/serving/src/main/java/feast/serving/config/ServingServiceConfig.java +++ b/serving/src/main/java/feast/serving/config/ServingServiceConfig.java @@ -46,7 +46,7 @@ public ServingService servingService( Tracer tracer) throws InvalidProtocolBufferException { ServingService servingService = null; - StoreProto.Store store = feastProperties.getStore().toProto(); + StoreProto.Store store = feastProperties.getActiveStore().toProto(); switch (store.getType()) { case REDIS: diff --git a/serving/src/main/java/feast/serving/config/SpecServiceConfig.java b/serving/src/main/java/feast/serving/config/SpecServiceConfig.java index 2682e176d35..0db4610f211 100644 --- a/serving/src/main/java/feast/serving/config/SpecServiceConfig.java +++ b/serving/src/main/java/feast/serving/config/SpecServiceConfig.java @@ -60,7 +60,7 @@ public ScheduledExecutorService cachedSpecServiceScheduledExecutorService( public CachedSpecService specService(FeastProperties feastProperties) throws InvalidProtocolBufferException { CoreSpecService coreService = new CoreSpecService(feastCoreHost, feastCorePort); - StoreProto.Store storeProto = feastProperties.getStore().toProto(); + StoreProto.Store storeProto = feastProperties.getActiveStore().toProto(); CachedSpecService cachedSpecStorage = new CachedSpecService(coreService, storeProto); try { cachedSpecStorage.populateCache(); diff --git a/serving/src/main/resources/application.yml b/serving/src/main/resources/application.yml index 264a98320f9..4d3bd07d417 100644 --- a/serving/src/main/resources/application.yml +++ b/serving/src/main/resources/application.yml @@ -14,35 +14,39 @@ feast: # The service name identifier for the tracing data service-name: feast_serving - store: - name: serving - type: REDIS # Alternative, BIGQUERY - redis_config: - host: localhost - port: 6379 - bigquery_config: - # GCP Project - project_id: my_project + active_store: serving - # BigQuery Dataset Id - dataset_id: my_dataset - - # staging-location specifies the URI to store intermediate files for batch serving. - # Feast Serving client is expected to have read access to this staging location - # to download the batch features. - # For example: gs://mybucket/myprefix - # Please omit the trailing slash in the URI. - staging-location: ${FEAST_JOB_STAGING_LOCATION:} - - # Retry options for BigQuery retrieval jobs - bigquery-initial-retry-delay-secs: 1 - - # BigQuery timeout for retrieval jobs - bigquery-total-timeout-secs: 21600 - subscriptions: - - name: "*" - project: "*" - version: "*" + stores: + - name: online + type: REDIS + config: + host: localhost + port: 6379 + subscriptions: + - name: "*" + project: "*" + version: "*" + - name: historical + type: BIGQUERY + config: + # GCP Project + project_id: my_project + # BigQuery Dataset Id + dataset_id: my_dataset + # staging-location specifies the URI to store intermediate files for batch serving. + # Feast Serving client is expected to have read access to this staging location + # to download the batch features. + # For example: gs://mybucket/myprefix + # Please omit the trailing slash in the URI. + staging-location: ${FEAST_JOB_STAGING_LOCATION:} + # Retry options for BigQuery retrieval jobs + bigquery-initial-retry-delay-secs: 1 + # BigQuery timeout for retrieval jobs + bigquery-total-timeout-secs: 21600 + subscriptions: + - name: "*" + project: "*" + version: "*" job_store: redis_host: localhost From a3b3b6a66dac013a5526d64cb0af8bd5932a4e42 Mon Sep 17 00:00:00 2001 From: Willem Pienaar Date: Sun, 12 Apr 2020 10:07:51 +0800 Subject: [PATCH 06/17] Remove Lombok from Serving configuration --- serving/lombok.config | 1 - serving/pom.xml | 14 +++++--------- 2 files changed, 5 insertions(+), 10 deletions(-) delete mode 100644 serving/lombok.config diff --git a/serving/lombok.config b/serving/lombok.config deleted file mode 100644 index 8f7e8aa1ac9..00000000000 --- a/serving/lombok.config +++ /dev/null @@ -1 +0,0 @@ -lombok.addLombokGeneratedAnnotation = true \ No newline at end of file diff --git a/serving/pom.xml b/serving/pom.xml index 1036f437de3..8ed91b6bb96 100644 --- a/serving/pom.xml +++ b/serving/pom.xml @@ -84,27 +84,28 @@ ${project.version} + dev.feast - feast-core + feast-storage-api ${project.version} dev.feast - feast-storage-api + feast-storage-connector-redis ${project.version} dev.feast - feast-storage-connector-redis + feast-storage-connector-bigquery ${project.version} dev.feast - feast-storage-connector-bigquery + feast-core ${project.version} @@ -273,11 +274,6 @@ embedded-redis test - - org.projectlombok - lombok - compile - From 917c4b8cb7b64ed13269bf5f57d0f3f517f9a2be Mon Sep 17 00:00:00 2001 From: Willem Pienaar Date: Sun, 12 Apr 2020 16:00:31 +0800 Subject: [PATCH 07/17] Update Store configuration loading in serving to use a store model --- serving/pom.xml | 6 - .../feast/serving/ServingApplication.java | 10 +- .../feast/serving/config/FeastProperties.java | 133 +++++++++++++++++- .../serving/config/ServingServiceConfig.java | 3 +- .../serving/config/SpecServiceConfig.java | 3 +- .../util/mappers/YamlToProtoMapper.java | 47 ------- serving/src/main/resources/application.yml | 3 +- .../util/mappers/YamlToProtoMapperTest.java | 54 ------- 8 files changed, 144 insertions(+), 115 deletions(-) delete mode 100644 serving/src/main/java/feast/serving/util/mappers/YamlToProtoMapper.java delete mode 100644 serving/src/test/java/feast/serving/util/mappers/YamlToProtoMapperTest.java diff --git a/serving/pom.xml b/serving/pom.xml index 8ed91b6bb96..bbb694011a3 100644 --- a/serving/pom.xml +++ b/serving/pom.xml @@ -103,12 +103,6 @@ ${project.version} - - dev.feast - feast-core - ${project.version} - - org.slf4j diff --git a/serving/src/main/java/feast/serving/ServingApplication.java b/serving/src/main/java/feast/serving/ServingApplication.java index 064f7b3e8d8..ab036d04d18 100644 --- a/serving/src/main/java/feast/serving/ServingApplication.java +++ b/serving/src/main/java/feast/serving/ServingApplication.java @@ -19,9 +19,17 @@ import feast.serving.config.FeastProperties; import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.SpringBootApplication; +import org.springframework.boot.autoconfigure.jdbc.DataSourceAutoConfiguration; +import org.springframework.boot.autoconfigure.jdbc.DataSourceTransactionManagerAutoConfiguration; +import org.springframework.boot.autoconfigure.orm.jpa.HibernateJpaAutoConfiguration; import org.springframework.boot.context.properties.EnableConfigurationProperties; -@SpringBootApplication +@SpringBootApplication( + exclude = { + DataSourceAutoConfiguration.class, + DataSourceTransactionManagerAutoConfiguration.class, + HibernateJpaAutoConfiguration.class + }) @EnableConfigurationProperties(FeastProperties.class) public class ServingApplication { public static void main(String[] args) { diff --git a/serving/src/main/java/feast/serving/config/FeastProperties.java b/serving/src/main/java/feast/serving/config/FeastProperties.java index 8ead2d2a773..99ae8db75bf 100644 --- a/serving/src/main/java/feast/serving/config/FeastProperties.java +++ b/serving/src/main/java/feast/serving/config/FeastProperties.java @@ -21,8 +21,16 @@ // https://www.baeldung.com/configuration-properties-in-spring-boot // https://docs.spring.io/spring-boot/docs/current/reference/html/boot-features-external-config.html#boot-features-external-config-typesafe-configuration-properties -import feast.core.model.Store; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.protobuf.InvalidProtocolBufferException; +import com.google.protobuf.util.JsonFormat; +import feast.core.StoreProto; +import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; import javax.validation.constraints.NotBlank; import javax.validation.constraints.Positive; import org.apache.logging.log4j.core.config.plugins.validation.constraints.ValidHost; @@ -66,7 +74,8 @@ public Store getActiveStore() { return store; } } - throw new RuntimeException("Active store is either misconfigured."); + throw new RuntimeException( + String.format("Active store is misconfigured. Could not find store: %s.", activeStore)); } /** @@ -84,7 +93,7 @@ public void setActiveStore(String activeStore) { /** * Collection of store configurations. The active store is selected by the "activeStore" field. */ - private List stores; + private List stores = new ArrayList<>(); /* Job Store properties to retain state of async jobs. */ private JobStoreProperties jobStore; @@ -164,6 +173,124 @@ public void setStores(List stores) { this.stores = stores; } + public static class Store { + + private String name; + + private String type; + + private Map config = new HashMap<>(); + + private List subscriptions = new ArrayList<>(); + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public String getType() { + return type; + } + + public void setType(String type) { + this.type = type; + } + + public StoreProto.Store toProto() + throws InvalidProtocolBufferException, JsonProcessingException { + List subscriptions = getSubscriptions(); + List subscriptionProtos = + subscriptions.stream().map(Subscription::toProto).collect(Collectors.toList()); + + StoreProto.Store.Builder storeProtoBuilder = + StoreProto.Store.newBuilder() + .setName(name) + .setType(StoreProto.Store.StoreType.valueOf(type)) + .addAllSubscriptions(subscriptionProtos); + + ObjectMapper jsonWriter = new ObjectMapper(); + + // TODO: All of this logic should be moved to the store layer. Only a Map + // should be sent to a store and it should do its own validation. + switch (StoreProto.Store.StoreType.valueOf(type)) { + case REDIS: + StoreProto.Store.RedisConfig.Builder redisConfig = + StoreProto.Store.RedisConfig.newBuilder(); + JsonFormat.parser().merge(jsonWriter.writeValueAsString(config), redisConfig); + return storeProtoBuilder.setRedisConfig(redisConfig.build()).build(); + case BIGQUERY: + StoreProto.Store.BigQueryConfig.Builder bqConfig = + StoreProto.Store.BigQueryConfig.newBuilder(); + JsonFormat.parser().merge(jsonWriter.writeValueAsString(config), bqConfig); + return storeProtoBuilder.setBigqueryConfig(bqConfig.build()).build(); + case CASSANDRA: + StoreProto.Store.CassandraConfig.Builder cassandraConfig = + StoreProto.Store.CassandraConfig.newBuilder(); + JsonFormat.parser().merge(jsonWriter.writeValueAsString(config), cassandraConfig); + return storeProtoBuilder.setCassandraConfig(cassandraConfig.build()).build(); + default: + throw new InvalidProtocolBufferException("Invalid store set"); + } + } + + private List getSubscriptions() { + return subscriptions; + } + + public Map getConfig() { + return config; + } + + public void setConfig(Map config) { + this.config = config; + } + + public void setSubscriptions(List subscriptions) { + this.subscriptions = subscriptions; + } + + public class Subscription { + String project; + String name; + String version; + + public String getProject() { + return project; + } + + public void setProject(String project) { + this.project = project; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public String getVersion() { + return version; + } + + public void setVersion(String version) { + this.version = version; + } + + public StoreProto.Store.Subscription toProto() { + return StoreProto.Store.Subscription.newBuilder() + .setName(getName()) + .setProject(getProject()) + .setVersion(getVersion()) + .build(); + } + } + } + /** * Gets job store properties * diff --git a/serving/src/main/java/feast/serving/config/ServingServiceConfig.java b/serving/src/main/java/feast/serving/config/ServingServiceConfig.java index 80ecb936c01..13dd9400475 100644 --- a/serving/src/main/java/feast/serving/config/ServingServiceConfig.java +++ b/serving/src/main/java/feast/serving/config/ServingServiceConfig.java @@ -16,6 +16,7 @@ */ package feast.serving.config; +import com.fasterxml.jackson.core.JsonProcessingException; import com.google.protobuf.InvalidProtocolBufferException; import feast.core.StoreProto; import feast.serving.service.HistoricalServingService; @@ -44,7 +45,7 @@ public ServingService servingService( CachedSpecService specService, JobService jobService, Tracer tracer) - throws InvalidProtocolBufferException { + throws InvalidProtocolBufferException, JsonProcessingException { ServingService servingService = null; StoreProto.Store store = feastProperties.getActiveStore().toProto(); diff --git a/serving/src/main/java/feast/serving/config/SpecServiceConfig.java b/serving/src/main/java/feast/serving/config/SpecServiceConfig.java index 0db4610f211..dbe2de665ee 100644 --- a/serving/src/main/java/feast/serving/config/SpecServiceConfig.java +++ b/serving/src/main/java/feast/serving/config/SpecServiceConfig.java @@ -16,6 +16,7 @@ */ package feast.serving.config; +import com.fasterxml.jackson.core.JsonProcessingException; import com.google.protobuf.InvalidProtocolBufferException; import feast.core.StoreProto; import feast.serving.specs.CachedSpecService; @@ -58,7 +59,7 @@ public ScheduledExecutorService cachedSpecServiceScheduledExecutorService( @Bean public CachedSpecService specService(FeastProperties feastProperties) - throws InvalidProtocolBufferException { + throws InvalidProtocolBufferException, JsonProcessingException { CoreSpecService coreService = new CoreSpecService(feastCoreHost, feastCorePort); StoreProto.Store storeProto = feastProperties.getActiveStore().toProto(); CachedSpecService cachedSpecStorage = new CachedSpecService(coreService, storeProto); diff --git a/serving/src/main/java/feast/serving/util/mappers/YamlToProtoMapper.java b/serving/src/main/java/feast/serving/util/mappers/YamlToProtoMapper.java deleted file mode 100644 index 784a21552bd..00000000000 --- a/serving/src/main/java/feast/serving/util/mappers/YamlToProtoMapper.java +++ /dev/null @@ -1,47 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * Copyright 2018-2019 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.serving.util.mappers; - -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; -import com.google.protobuf.util.JsonFormat; -import feast.core.StoreProto; -import feast.core.StoreProto.Store; -import feast.core.StoreProto.Store.Builder; -import java.io.IOException; -import org.slf4j.Logger; - -public class YamlToProtoMapper { - - private static final Logger log = org.slf4j.LoggerFactory.getLogger(YamlToProtoMapper.class); - - private static final ObjectMapper yamlReader = new ObjectMapper(new YAMLFactory()); - private static final ObjectMapper jsonWriter = new ObjectMapper(); - - public static Store yamlToStoreProto(String yaml) { - try { - Object obj = yamlReader.readValue(yaml, Object.class); - String jsonString = jsonWriter.writeValueAsString(obj); - Builder builder = Store.newBuilder(); - JsonFormat.parser().merge(jsonString, builder); - return builder.build(); - } catch (IOException e) { - log.error("Could not parse store configuration YAML", e); - return StoreProto.Store.getDefaultInstance(); - } - } -} diff --git a/serving/src/main/resources/application.yml b/serving/src/main/resources/application.yml index 4d3bd07d417..4d87c04170d 100644 --- a/serving/src/main/resources/application.yml +++ b/serving/src/main/resources/application.yml @@ -14,8 +14,7 @@ feast: # The service name identifier for the tracing data service-name: feast_serving - active_store: serving - + active_store: online stores: - name: online type: REDIS diff --git a/serving/src/test/java/feast/serving/util/mappers/YamlToProtoMapperTest.java b/serving/src/test/java/feast/serving/util/mappers/YamlToProtoMapperTest.java deleted file mode 100644 index 6f95f5307b2..00000000000 --- a/serving/src/test/java/feast/serving/util/mappers/YamlToProtoMapperTest.java +++ /dev/null @@ -1,54 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * Copyright 2018-2019 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.serving.util.mappers; - -import static org.hamcrest.core.IsEqual.equalTo; -import static org.junit.Assert.*; - -import feast.core.StoreProto.Store; -import feast.core.StoreProto.Store.RedisConfig; -import feast.core.StoreProto.Store.StoreType; -import feast.core.StoreProto.Store.Subscription; -import java.io.IOException; -import org.junit.Test; - -public class YamlToProtoMapperTest { - - @Test - public void shouldConvertYamlToProto() throws IOException { - String yaml = - "name: test\n" - + "type: REDIS\n" - + "redis_config:\n" - + " host: localhost\n" - + " port: 6379\n" - + "subscriptions:\n" - + "- project: \"*\"\n" - + " name: \"*\"\n" - + " version: \"*\"\n"; - Store store = YamlToProtoMapper.yamlToStoreProto(yaml); - Store expected = - Store.newBuilder() - .setName("test") - .setType(StoreType.REDIS) - .setRedisConfig(RedisConfig.newBuilder().setHost("localhost").setPort(6379)) - .addSubscriptions( - Subscription.newBuilder().setProject("*").setName("*").setVersion("*")) - .build(); - assertThat(store, equalTo(expected)); - } -} From 971bd239a852b9b8562e8e20d34294993cda8db6 Mon Sep 17 00:00:00 2001 From: Willem Pienaar Date: Sun, 12 Apr 2020 16:50:36 +0800 Subject: [PATCH 08/17] Update RedisBackedJobService to instantiate its own Redis Client --- .../serving/config/JobServiceConfig.java | 10 ++-- .../feast/serving/config/JobStoreConfig.java | 46 ------------------- .../service/RedisBackedJobService.java | 16 +++++-- 3 files changed, 19 insertions(+), 53 deletions(-) delete mode 100644 serving/src/main/java/feast/serving/config/JobStoreConfig.java diff --git a/serving/src/main/java/feast/serving/config/JobServiceConfig.java b/serving/src/main/java/feast/serving/config/JobServiceConfig.java index f94a9c28c6e..fa2272e5cd0 100644 --- a/serving/src/main/java/feast/serving/config/JobServiceConfig.java +++ b/serving/src/main/java/feast/serving/config/JobServiceConfig.java @@ -16,11 +16,12 @@ */ package feast.serving.config; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.google.protobuf.InvalidProtocolBufferException; import feast.core.StoreProto.Store.StoreType; import feast.serving.service.JobService; import feast.serving.service.NoopJobService; import feast.serving.service.RedisBackedJobService; -import feast.serving.specs.CachedSpecService; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -28,10 +29,11 @@ public class JobServiceConfig { @Bean - public JobService jobService(CachedSpecService specService, JobStoreConfig jobStoreConfig) { - if (!specService.getStore().getType().equals(StoreType.BIGQUERY)) { + public JobService jobService(FeastProperties feastProperties) + throws InvalidProtocolBufferException, JsonProcessingException { + if (!feastProperties.getActiveStore().toProto().getType().equals(StoreType.BIGQUERY)) { return new NoopJobService(); } - return new RedisBackedJobService(jobStoreConfig); + return new RedisBackedJobService(feastProperties.getJobStore()); } } diff --git a/serving/src/main/java/feast/serving/config/JobStoreConfig.java b/serving/src/main/java/feast/serving/config/JobStoreConfig.java deleted file mode 100644 index 02bef55ddae..00000000000 --- a/serving/src/main/java/feast/serving/config/JobStoreConfig.java +++ /dev/null @@ -1,46 +0,0 @@ -/* - * 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.serving.config; - -import io.lettuce.core.RedisClient; -import io.lettuce.core.RedisURI; -import io.lettuce.core.api.StatefulRedisConnection; -import io.lettuce.core.codec.ByteArrayCodec; -import io.lettuce.core.resource.DefaultClientResources; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.context.annotation.Configuration; - -@Configuration -public class JobStoreConfig { - - private final StatefulRedisConnection jobStoreRedisConnection; - - @Autowired - public JobStoreConfig(FeastProperties feastProperties) { - RedisURI uri = - RedisURI.create( - feastProperties.getJobStore().getRedisHost(), - feastProperties.getJobStore().getRedisPort()); - - jobStoreRedisConnection = - RedisClient.create(DefaultClientResources.create(), uri).connect(new ByteArrayCodec()); - } - - public StatefulRedisConnection getJobStoreRedisConnection() { - return jobStoreRedisConnection; - } -} diff --git a/serving/src/main/java/feast/serving/service/RedisBackedJobService.java b/serving/src/main/java/feast/serving/service/RedisBackedJobService.java index 99933585b27..dd010e58970 100644 --- a/serving/src/main/java/feast/serving/service/RedisBackedJobService.java +++ b/serving/src/main/java/feast/serving/service/RedisBackedJobService.java @@ -19,9 +19,13 @@ import com.google.protobuf.util.JsonFormat; import feast.serving.ServingAPIProto.Job; import feast.serving.ServingAPIProto.Job.Builder; -import feast.serving.config.JobStoreConfig; +import feast.serving.config.FeastProperties; +import io.lettuce.core.RedisClient; +import io.lettuce.core.RedisURI; import io.lettuce.core.api.StatefulRedisConnection; import io.lettuce.core.api.sync.RedisCommands; +import io.lettuce.core.codec.ByteArrayCodec; +import io.lettuce.core.resource.DefaultClientResources; import java.util.Optional; import org.joda.time.Duration; import org.slf4j.Logger; @@ -38,8 +42,14 @@ public class RedisBackedJobService implements JobService { // and since users normally don't require info about relatively old jobs. private final int defaultExpirySeconds = (int) Duration.standardDays(1).getStandardSeconds(); - public RedisBackedJobService(JobStoreConfig jobStoreConfig) { - this.syncCommand = jobStoreConfig.getJobStoreRedisConnection().sync(); + public RedisBackedJobService(FeastProperties.JobStoreProperties jobStoreProperties) { + RedisURI uri = + RedisURI.create(jobStoreProperties.getRedisHost(), jobStoreProperties.getRedisPort()); + + this.syncCommand = + RedisClient.create(DefaultClientResources.create(), uri) + .connect(new ByteArrayCodec()) + .sync(); } public RedisBackedJobService(StatefulRedisConnection connection) { From 38e57dd76e04f6860922642a0cd2e50d3f3c6145 Mon Sep 17 00:00:00 2001 From: Willem Pienaar Date: Sun, 12 Apr 2020 16:51:08 +0800 Subject: [PATCH 09/17] Update comments in FeastProperties --- .../feast/serving/config/FeastProperties.java | 116 +++++++++++++++++- 1 file changed, 110 insertions(+), 6 deletions(-) diff --git a/serving/src/main/java/feast/serving/config/FeastProperties.java b/serving/src/main/java/feast/serving/config/FeastProperties.java index 99ae8db75bf..7fdcdcd2321 100644 --- a/serving/src/main/java/feast/serving/config/FeastProperties.java +++ b/serving/src/main/java/feast/serving/config/FeastProperties.java @@ -52,6 +52,7 @@ public FeastProperties(BuildProperties buildProperties) { setVersion(buildProperties.getVersion()); } + /** Instantiates a new Feast class. */ public FeastProperties() {} /* Feast Serving build version */ @@ -149,7 +150,7 @@ public void setCoreHost(String coreHost) { /** * Gets Feast Core gRPC port. * - * @return Port + * @return Feast Core gRPC port */ public int getCoreGrpcPort() { return coreGrpcPort; @@ -173,6 +174,7 @@ public void setStores(List stores) { this.stores = stores; } + /** Store configuration class for database that this Feast Serving uses. */ public static class Store { private String name; @@ -183,22 +185,49 @@ public static class Store { private List subscriptions = new ArrayList<>(); + /** + * Gets name of this store. This is unique to this specific instance. + * + * @return the name of the store + */ public String getName() { return name; } + /** + * Sets the name of this store. + * + * @param name the name of the store + */ public void setName(String name) { this.name = name; } + /** + * Gets the store type. Example are REDIS or BIGQUERY + * + * @return the store type as a String. + */ public String getType() { return type; } + /** + * Sets the store type + * + * @param type the type + */ public void setType(String type) { this.type = type; } + /** + * Converts this {@link Store} to a {@StoreProto.Store} + * + * @return {@StoreProto.Store} with configuration set + * @throws InvalidProtocolBufferException the invalid protocol buffer exception + * @throws JsonProcessingException the json processing exception + */ public StoreProto.Store toProto() throws InvalidProtocolBufferException, JsonProcessingException { List subscriptions = getSubscriptions(); @@ -236,51 +265,121 @@ public StoreProto.Store toProto() } } - private List getSubscriptions() { + /** + * Get the subscriptions to this specific store. The subscriptions indicate which feature sets a + * store subscribes to. + * + * @return List of subscriptions in the form of {@link List}. + */ + public List getSubscriptions() { return subscriptions; } + /** + * Sets the store specific configuration. See getSubscriptions() for more details. + * + * @param subscriptions the subscriptions list + */ + public void setSubscriptions(List subscriptions) { + this.subscriptions = subscriptions; + } + + /** + * Gets the configuration to this specific store. This is a map of strings. These options are + * unique to the store. Please see protos/feast/core/Store.proto for the store specific + * configuration options + * + * @return the config as a {@link Map} + */ public Map getConfig() { return config; } + /** + * Sets the store config. Please protos/feast/core/Store.proto for the specific options for each + * store. + * + * @param config the config map + */ public void setConfig(Map config) { this.config = config; } - public void setSubscriptions(List subscriptions) { - this.subscriptions = subscriptions; - } - + /** + * The Subscription type. + * + *

Note: Please see protos/feast/core/CoreService.proto for details on how to subscribe to + * feature sets. + */ public class Subscription { + /** Feast project to subscribe to. */ String project; + + /** Feature set to subscribe to. */ String name; + + /** Feature set versions to subscribe to. */ String version; + /** + * Gets Feast project subscribed to. + * + * @return the project string + */ public String getProject() { return project; } + /** + * Sets Feast project to subscribe to for this store. + * + * @param project the project + */ public void setProject(String project) { this.project = project; } + /** + * Gets the feature set name to subscribe to. + * + * @return the name + */ public String getName() { return name; } + /** + * Sets the feature set name to subscribe to. + * + * @param name the name + */ public void setName(String name) { this.name = name; } + /** + * Gets the feature set version that is being subscribed to by this store. + * + * @return the version + */ public String getVersion() { return version; } + /** + * Sets the feature set version that is being subscribed to by this store. + * + * @param version the version + */ public void setVersion(String version) { this.version = version; } + /** + * Convert this {@link Subscription} to a {@link StoreProto.Store.Subscription}. + * + * @return the store proto . store . subscription + */ public StoreProto.Store.Subscription toProto() { return StoreProto.Store.Subscription.newBuilder() .setName(getName()) @@ -318,6 +417,11 @@ public TracingProperties getTracing() { return tracing; } + /** + * Sets the tracing configuration. + * + * @param tracing the tracing + */ public void setTracing(TracingProperties tracing) { this.tracing = tracing; } From ca9eb597fb670152f21439d4acd99ec76bf380ea Mon Sep 17 00:00:00 2001 From: Willem Pienaar Date: Sun, 12 Apr 2020 19:25:14 +0800 Subject: [PATCH 10/17] Fix broken default application.yml and add comments in Serving --- serving/sample_redis_config.yml | 9 ----- serving/src/main/resources/application.yml | 43 ++++++++++++++-------- 2 files changed, 28 insertions(+), 24 deletions(-) delete mode 100644 serving/sample_redis_config.yml diff --git a/serving/sample_redis_config.yml b/serving/sample_redis_config.yml deleted file mode 100644 index b3461649a1d..00000000000 --- a/serving/sample_redis_config.yml +++ /dev/null @@ -1,9 +0,0 @@ -name: serving -type: REDIS -redis_config: - host: localhost - port: 6379 -subscriptions: - - name: "*" - project: "*" - version: "*" diff --git a/serving/src/main/resources/application.yml b/serving/src/main/resources/application.yml index 4d87c04170d..053fddfafff 100644 --- a/serving/src/main/resources/application.yml +++ b/serving/src/main/resources/application.yml @@ -4,30 +4,29 @@ feast: core-host: ${FEAST_CORE_HOST:localhost} core-grpc-port: ${FEAST_CORE_GRPC_PORT:6565} - tracing: - # If true, Feast will provide tracing data (using OpenTracing API) for various RPC method calls - # which can be useful to debug performance issues and perform benchmarking - enabled: false - # Only Jaeger tracer is supported currently - # https://opentracing.io/docs/supported-tracers/ - tracer-name: jaeger - # The service name identifier for the tracing data - service-name: feast_serving - + # Indicates the active store. Only a single store in the last can be active at one time. In the future this key + # will be deprecated in order to allow multiple stores to be served from a single serving instance active_store: online + + # List of store configurations stores: - - name: online - type: REDIS - config: + # Below are two store configurations. One for Redis and one for BigQuery. + # Please see https://api.docs.feast.dev/grpc/feast.core.pb.html#Store for configuration options + - name: online # Name of the store (referenced by active_store) + type: REDIS # Type of the store. REDIS, BIGQUERY are available options + config: # Store specific configuration. See host: localhost port: 6379 + # Subscriptions indicate which feature sets needs to be retrieved and used to populate this store subscriptions: + # Wildcards match all options. No filtering is done. - name: "*" project: "*" version: "*" + - name: historical type: BIGQUERY - config: + config: # Store specific configuration. # GCP Project project_id: my_project # BigQuery Dataset Id @@ -37,7 +36,7 @@ feast: # to download the batch features. # For example: gs://mybucket/myprefix # Please omit the trailing slash in the URI. - staging-location: ${FEAST_JOB_STAGING_LOCATION:} + staging-location: gs://mybucket/myprefix # Retry options for BigQuery retrieval jobs bigquery-initial-retry-delay-secs: 1 # BigQuery timeout for retrieval jobs @@ -47,8 +46,22 @@ feast: project: "*" version: "*" + tracing: + # If true, Feast will provide tracing data (using OpenTracing API) for various RPC method calls + # which can be useful to debug performance issues and perform benchmarking + enabled: false + # Only Jaeger tracer is supported currently + # https://opentracing.io/docs/supported-tracers/ + tracer-name: jaeger + # The service name identifier for the tracing data + service-name: feast_serving + + # The job store is used to maintain job management state for Feast Serving. This is required when using certain + # historical stores like BigQuery. Only Redis is supported as a job store. job_store: + # Redis host to connect to redis_host: localhost + # Redis port to connect to redis_port: 6379 grpc: From 555a44d28577944d207fe4fa325b6ee826f359a4 Mon Sep 17 00:00:00 2001 From: Willem Pienaar Date: Sun, 12 Apr 2020 19:44:20 +0800 Subject: [PATCH 11/17] Refactored and cleaned up Feast Core configuration for job runners. --- .../feast/core/config/FeastProperties.java | 152 +++++++----------- .../java/feast/core/config/JobConfig.java | 48 +----- .../core/job/dataflow/DataflowJobConfig.java | 25 --- .../core/job/dataflow/DataflowJobManager.java | 37 ++++- .../job/dataflow/DataflowRunnerConfig.java | 140 ++++++++++++++++ .../core/service/JobCoordinatorService.java | 4 +- core/src/main/resources/application.yml | 45 ++++-- .../job/dataflow/DataflowJobManagerTest.java | 2 +- .../service/JobCoordinatorServiceTest.java | 2 +- protos/feast/core/Runner.proto | 73 +++++++++ protos/feast/core/Source.proto | 10 +- 11 files changed, 357 insertions(+), 181 deletions(-) delete mode 100644 core/src/main/java/feast/core/job/dataflow/DataflowJobConfig.java create mode 100644 core/src/main/java/feast/core/job/dataflow/DataflowRunnerConfig.java create mode 100644 protos/feast/core/Runner.proto diff --git a/core/src/main/java/feast/core/config/FeastProperties.java b/core/src/main/java/feast/core/config/FeastProperties.java index b21ed254614..44c6b61f9c2 100644 --- a/core/src/main/java/feast/core/config/FeastProperties.java +++ b/core/src/main/java/feast/core/config/FeastProperties.java @@ -16,21 +16,10 @@ */ package feast.core.config; -import com.fasterxml.jackson.core.type.TypeReference; -import com.fasterxml.jackson.databind.ObjectMapper; -import feast.core.config.FeastProperties.JobProperties.RunnerOptions; import feast.core.config.FeastProperties.StreamProperties.FeatureStreamOptions; -import java.util.Arrays; -import java.util.HashMap; -import java.util.Map; -import java.util.Objects; -import java.util.Set; +import java.util.*; import javax.annotation.PostConstruct; -import javax.validation.ConstraintViolation; -import javax.validation.ConstraintViolationException; -import javax.validation.Validation; -import javax.validation.Validator; -import javax.validation.ValidatorFactory; +import javax.validation.*; import javax.validation.constraints.AssertTrue; import javax.validation.constraints.NotBlank; import javax.validation.constraints.NotNull; @@ -47,11 +36,17 @@ @ConfigurationProperties(prefix = "feast", ignoreInvalidFields = true) public class FeastProperties { + /** + * Instantiates a new Feast properties. + * + * @param buildProperties Feast build properties + */ @Autowired public FeastProperties(BuildProperties buildProperties) { setVersion(buildProperties.getVersion()); } + /** Instantiates a new Feast properties. */ public FeastProperties() {} /* Feast Core Build Version */ @@ -64,100 +59,69 @@ public FeastProperties() {} /* Feast Kafka stream properties */ private StreamProperties stream; + /** Feast job properties. These properties are used for ingestion jobs. */ @Getter @Setter public static class JobProperties { @NotBlank - /* Apache Beam runner type. Possible options: DirectRunner, DataflowRunner */ - private String runner; - - /* Apache Beam runner options for population jobs */ - private RunnerOptions runnerOptions; - - /* (Optional) Additional arguments to pass to Beam population jobs */ - private Map extraRunnerOptions; - - @NotNull - /* Population job metric properties */ - private MetricsProperties metrics; - - /* Timeout in seconds for each attempt to update or submit a new job to the runner */ - @Positive private long jobUpdateTimeout; - - /* Job update polling interval in millisecond. How frequently Feast will update running jobs. */ - @Positive private long pollingIntervalMillis; + /* The active Apache Beam runner name. This name references one instance of the Runner class */ + private String activeRunner; + + /** List of configured job runners. */ + private List runners = new ArrayList<>(); + + /** + * Gets a {@link Runner} instance of the active runner + * + * @return the active runner + */ + public Runner getActiveRunner() { + for (Runner runner : getRunners()) { + if (activeRunner.equals(runner.getName())) { + return runner; + } + } + throw new RuntimeException( + String.format( + "Active runner is misconfigured. Could not find runner: %s.", activeRunner)); + } - /** Apache Beam runner options for population jobs */ + /** Job Runner class. */ @Getter @Setter - public static class RunnerOptions { - - /* (Dataflow Runner Only) Project id to use when launching jobs. */ - @NotBlank private String project; - - /* (Dataflow Runner Only) The Google Compute Engine region for creating Dataflow jobs. */ - @NotBlank private String region; - - /* (Dataflow Runner Only) GCP availability zone for operations. */ - @NotBlank private String zone; - - /* (Dataflow Runner Only) Run the job as a specific service account, instead of the default GCE robot. */ - @NotBlank private String serviceAccount; - - /* (Dataflow Runner Only) GCE network for launching workers. */ - @NotBlank private String network; + public static class Runner { + /** Job runner name. This must be unique. */ + String name; - /* (Dataflow Runner Only) GCE subnetwork for launching workers. */ - @NotBlank private String subnetwork; - - /* (Dataflow Runner Only) Machine type to create Dataflow worker VMs as. */ - private String workerMachineType; - - /* (Dataflow Runner Only) The autoscaling algorithm to use for the workerpool. */ - private String autoscalingAlgorithm; - - /* (Dataflow Runner Only) Specifies whether worker pools should be started with public IP addresses. */ - private Boolean usePublicIps; + /** Job runner type DirectRunner, DataflowRunner currently supported */ + String type; /** - * (Dataflow Runner Only) A pipeline level default location for storing temporary files. - * Support Google Cloud Storage locations, e.g. gs://bucket/object + * Job runner configuration options. See the following for options + * https://api.docs.feast.dev/grpc/feast.core.pb.html#Runner */ - @NotBlank private String tempLocation; - - /* (Dataflow Runner Only) The maximum number of workers to use for the workerpool. */ - private Integer maxNumWorkers; + Map options = new HashMap<>(); /** - * (Direct Runner Only) Controls the amount of target parallelism the DirectRunner will use. - * Defaults to the greater of the number of available processors and 3. Must be a value - * greater than zero. + * Gets the job runner type as an enum. + * + * @return Returns the job runner type as {@link feast.core.job.Runner} */ - private Integer targetParallelism; - - /* BigQuery table specification, e.g. PROJECT_ID:DATASET_ID.PROJECT_ID */ - private String deadLetterTableSpec; + public feast.core.job.Runner getType() { + return feast.core.job.Runner.fromString(type); + } } - public Map getRunnerOptionsMap() { - // First collect the existing "extra options" - Map combinedOptions = new HashMap(getExtraRunnerOptions()); - - // Convert all fields in RunnerOptions to and merge - ObjectMapper oMapper = new ObjectMapper(); - combinedOptions.putAll( - oMapper.convertValue( - getRunnerOptions(), new TypeReference>() {})); + @NotNull + /* Population job metric properties */ + private MetricsProperties metrics; - return combinedOptions; - } - } + /* Timeout in seconds for each attempt to update or submit a new job to the runner */ + @Positive private long jobUpdateTimeoutSeconds; - @AssertTrue - public boolean isValidJobRunnerSelected() { - String[] validRunners = new String[] {"DataflowRunner", "DirectRunner"}; - return Arrays.asList(validRunners).contains(getJobs().getRunner()); + /* Job update polling interval in millisecond. How frequently Feast will update running jobs. */ + @Positive private long pollingIntervalMilliseconds; } /** Properties used to configure Feast's managed Kafka feature stream. */ @@ -193,6 +157,11 @@ public static class FeatureStreamOptions { } } + /** + * Validates whether stream options are correct. + * + * @return Boolean used for assertion + */ @AssertTrue public boolean isValidStreamTypeSelected() { return Objects.equals(getStream().getType(), "kafka"); @@ -251,13 +220,6 @@ public void validate() { throw new ConstraintViolationException(jobPropertiesViolations); } - // Validate RunnerOptions - Set> runnerOptionsViolations = - validator.validate(getJobs().getRunnerOptions()); - if (!runnerOptionsViolations.isEmpty()) { - throw new ConstraintViolationException(runnerOptionsViolations); - } - // Validate MetricsProperties if (getJobs().getMetrics().isEnabled()) { Set> jobMetricViolations = diff --git a/core/src/main/java/feast/core/config/JobConfig.java b/core/src/main/java/feast/core/config/JobConfig.java index 85641681bff..35483fba977 100644 --- a/core/src/main/java/feast/core/config/JobConfig.java +++ b/core/src/main/java/feast/core/config/JobConfig.java @@ -16,20 +16,11 @@ */ package feast.core.config; -import com.google.api.client.googleapis.auth.oauth2.GoogleCredential; -import com.google.api.client.googleapis.javanet.GoogleNetHttpTransport; -import com.google.api.client.json.jackson2.JacksonFactory; -import com.google.api.services.dataflow.Dataflow; -import com.google.api.services.dataflow.DataflowScopes; -import com.google.common.base.Strings; import feast.core.config.FeastProperties.JobProperties; import feast.core.job.JobManager; -import feast.core.job.Runner; import feast.core.job.dataflow.DataflowJobManager; import feast.core.job.direct.DirectJobRegistry; import feast.core.job.direct.DirectRunnerJobManager; -import java.io.IOException; -import java.security.GeneralSecurityException; import java.util.Map; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; @@ -52,40 +43,17 @@ public JobManager getJobManager( FeastProperties feastProperties, DirectJobRegistry directJobRegistry) { JobProperties jobProperties = feastProperties.getJobs(); - Runner runner = Runner.fromString(jobProperties.getRunner()); - Map jobOptions = jobProperties.getRunnerOptionsMap(); - switch (runner) { - case DATAFLOW: - if (Strings.isNullOrEmpty(jobOptions.getOrDefault("region", null)) - || Strings.isNullOrEmpty(jobOptions.getOrDefault("project", null))) { - log.error("Project and location of the Dataflow runner is not configured"); - throw new IllegalStateException( - "Project and location of Dataflow runner must be specified for jobs to be run on Dataflow runner."); - } - try { - GoogleCredential credential = - GoogleCredential.getApplicationDefault().createScoped(DataflowScopes.all()); - Dataflow dataflow = - new Dataflow( - GoogleNetHttpTransport.newTrustedTransport(), - JacksonFactory.getDefaultInstance(), - credential); + FeastProperties.JobProperties.Runner runner = jobProperties.getActiveRunner(); + Map runnerConfigOptions = runner.getOptions(); + FeastProperties.MetricsProperties metrics = jobProperties.getMetrics(); - return new DataflowJobManager( - dataflow, jobProperties.getRunnerOptionsMap(), jobProperties.getMetrics()); - } catch (IOException e) { - throw new IllegalStateException( - "Unable to find credential required for Dataflow monitoring API", e); - } catch (GeneralSecurityException e) { - throw new IllegalStateException("Security exception while connecting to Dataflow API", e); - } catch (Exception e) { - throw new IllegalStateException("Unable to initialize DataflowJobManager", e); - } + switch (runner.getType()) { + case DATAFLOW: + return new DataflowJobManager(runnerConfigOptions, metrics); case DIRECT: - return new DirectRunnerJobManager( - jobProperties.getRunnerOptionsMap(), directJobRegistry, jobProperties.getMetrics()); + return new DirectRunnerJobManager(runnerConfigOptions, directJobRegistry, metrics); default: - throw new IllegalArgumentException("Unsupported runner: " + jobProperties.getRunner()); + throw new IllegalArgumentException("Unsupported runner: " + runner); } } diff --git a/core/src/main/java/feast/core/job/dataflow/DataflowJobConfig.java b/core/src/main/java/feast/core/job/dataflow/DataflowJobConfig.java deleted file mode 100644 index a9bbf345d19..00000000000 --- a/core/src/main/java/feast/core/job/dataflow/DataflowJobConfig.java +++ /dev/null @@ -1,25 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * Copyright 2018-2019 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.job.dataflow; - -import lombok.Value; - -@Value -public class DataflowJobConfig { - private String projectId; - private String location; -} 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 9dc3dc0b57c..bd90206117f 100644 --- a/core/src/main/java/feast/core/job/dataflow/DataflowJobManager.java +++ b/core/src/main/java/feast/core/job/dataflow/DataflowJobManager.java @@ -18,7 +18,11 @@ import static feast.core.util.PipelineUtil.detectClassPathResourcesToStage; +import com.google.api.client.googleapis.auth.oauth2.GoogleCredential; +import com.google.api.client.googleapis.javanet.GoogleNetHttpTransport; +import com.google.api.client.json.jackson2.JacksonFactory; import com.google.api.services.dataflow.Dataflow; +import com.google.api.services.dataflow.DataflowScopes; import com.google.common.base.Strings; import com.google.protobuf.InvalidProtocolBufferException; import com.google.protobuf.util.JsonFormat; @@ -37,6 +41,7 @@ import feast.ingestion.options.ImportOptions; import feast.ingestion.options.OptionCompressor; import java.io.IOException; +import java.security.GeneralSecurityException; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -60,12 +65,36 @@ public class DataflowJobManager implements JobManager { private final MetricsProperties metrics; public DataflowJobManager( - Dataflow dataflow, Map runnerOptions, MetricsProperties metricsProperties) { - this.defaultOptions = runnerOptions; + Map runnerConfigOptions, MetricsProperties metricsProperties) { + + DataflowRunnerConfig config = new DataflowRunnerConfig(runnerConfigOptions); + + GoogleCredential credential = null; + try { + credential = GoogleCredential.getApplicationDefault().createScoped(DataflowScopes.all()); + } catch (IOException e) { + throw new IllegalStateException( + "Unable to find credential required for Dataflow monitoring API", e); + } + + Dataflow dataflow = null; + try { + dataflow = + new Dataflow( + GoogleNetHttpTransport.newTrustedTransport(), + JacksonFactory.getDefaultInstance(), + credential); + } catch (GeneralSecurityException e) { + throw new IllegalStateException("Security exception while connecting to Dataflow API", e); + } catch (IOException e) { + throw new IllegalStateException("Unable to initialize DataflowJobManager", e); + } + + this.defaultOptions = runnerConfigOptions; this.dataflow = dataflow; this.metrics = metricsProperties; - this.projectId = runnerOptions.get("project"); - this.location = runnerOptions.get("region"); + this.projectId = config.getProject(); + this.location = config.getRegion(); } @Override diff --git a/core/src/main/java/feast/core/job/dataflow/DataflowRunnerConfig.java b/core/src/main/java/feast/core/job/dataflow/DataflowRunnerConfig.java new file mode 100644 index 00000000000..d1fbaf06bb8 --- /dev/null +++ b/core/src/main/java/feast/core/job/dataflow/DataflowRunnerConfig.java @@ -0,0 +1,140 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright 2018-2019 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.job.dataflow; + +import java.util.Map; +import java.util.Set; +import javax.validation.*; +import javax.validation.constraints.NotBlank; +import lombok.Getter; +import lombok.Setter; +import org.springframework.beans.BeanUtils; + +@Getter +@Setter +public class DataflowRunnerConfig { + + public DataflowRunnerConfig(Map runnerConfigOptions) { + BeanUtils.copyProperties(this, runnerConfigOptions); + validate(); + + // + // Map props = BeanUtils.describe(someObject); + // if (props.containsKey("name")) { + // BeanUtils.setProperty(someObject, "name", "value"); + // } + // + // + // if (runnerConfigOptions.containsKey("project")) { + // setProject(runnerConfigOptions.get("project")); + // } + // + // if (runnerConfigOptions.containsKey("region")) { + // setRegion(runnerConfigOptions.get("region")); + // } + // + // if (runnerConfigOptions.containsKey("zone")) { + // setZone(runnerConfigOptions.get("zone")); + // } + // + // if (runnerConfigOptions.containsKey("serviceAccount")) { + // setServiceAccount(runnerConfigOptions.get("serviceAccount")); + // } + // + // if (runnerConfigOptions.containsKey("network")) { + // setNetwork(runnerConfigOptions.get("network")); + // } + // + // if (runnerConfigOptions.containsKey("subnetwork")) { + // setSubnetwork(runnerConfigOptions.get("subnetwork")); + // } + // + // if (runnerConfigOptions.containsKey("workerMachineType")) { + // setWorkerMachineType(runnerConfigOptions.get("workerMachineType")); + // } + // + // if (runnerConfigOptions.containsKey("autoscalingAlgorithm")) { + // setAutoscalingAlgorithm(runnerConfigOptions.get("autoscalingAlgorithm")); + // } + // + // if (runnerConfigOptions.containsKey("usePublicIps")) { + // setUsePublicIps(Boolean.parseBoolean(runnerConfigOptions.get("usePublicIps"))); + // } + // + // if (runnerConfigOptions.containsKey("tempLocation")) { + // setTempLocation(runnerConfigOptions.get("tempLocation")); + // } + // + // if (runnerConfigOptions.containsKey("maxNumWorkers")) { + // setMaxNumWorkers(Integer.parseInt(runnerConfigOptions.get("maxNumWorkers"))); + // } + // + // if (runnerConfigOptions.containsKey("deadLetterTableSpec")) { + // setDeadLetterTableSpec(runnerConfigOptions.get("deadLetterTableSpec")); + // } + } + + /* (Dataflow Runner Only) Project id to use when launching jobs. */ + @NotBlank private String project; + + /* (Dataflow Runner Only) The Google Compute Engine region for creating Dataflow jobs. */ + @NotBlank private String region; + + /* (Dataflow Runner Only) GCP availability zone for operations. */ + @NotBlank private String zone; + + /* (Dataflow Runner Only) Run the job as a specific service account, instead of the default GCE robot. */ + @NotBlank private String serviceAccount; + + /* (Dataflow Runner Only) GCE network for launching workers. */ + @NotBlank private String network; + + /* (Dataflow Runner Only) GCE subnetwork for launching workers. */ + @NotBlank private String subnetwork; + + /* (Dataflow Runner Only) Machine type to create Dataflow worker VMs as. */ + private String workerMachineType; + + /* (Dataflow Runner Only) The autoscaling algorithm to use for the workerpool. */ + private String autoscalingAlgorithm; + + /* (Dataflow Runner Only) Specifies whether worker pools should be started with public IP addresses. */ + private Boolean usePublicIps; + + /** + * (Dataflow Runner Only) A pipeline level default location for storing temporary files. Support + * Google Cloud Storage locations, e.g. gs://bucket/object + */ + @NotBlank private String tempLocation; + + /* (Dataflow Runner Only) The maximum number of workers to use for the workerpool. */ + private Integer maxNumWorkers; + + /* BigQuery table specification, e.g. PROJECT_ID:DATASET_ID.PROJECT_ID */ + private String deadLetterTableSpec; + + public void validate() { + ValidatorFactory factory = Validation.buildDefaultValidatorFactory(); + Validator validator = factory.getValidator(); + + Set> dataflowRunnerConfigViolation = + validator.validate(this); + if (!dataflowRunnerConfigViolation.isEmpty()) { + throw new ConstraintViolationException(dataflowRunnerConfigViolation); + } + } +} diff --git a/core/src/main/java/feast/core/service/JobCoordinatorService.java b/core/src/main/java/feast/core/service/JobCoordinatorService.java index 24115883ed2..b4ed341edc6 100644 --- a/core/src/main/java/feast/core/service/JobCoordinatorService.java +++ b/core/src/main/java/feast/core/service/JobCoordinatorService.java @@ -87,7 +87,7 @@ public JobCoordinatorService( *

4) Updates Feature set statuses */ @Transactional - @Scheduled(fixedDelayString = "${feast.jobs.updates.pollingIntervalMillis}") + @Scheduled(fixedDelayString = "${feast.jobs.polling_interval_milliseconds}") public void Poll() throws InvalidProtocolBufferException { log.info("Polling for new jobs..."); List jobUpdateTasks = new ArrayList<>(); @@ -122,7 +122,7 @@ public void Poll() throws InvalidProtocolBufferException { store, originalJob, jobManager, - jobProperties.getJobUpdateTimeout())); + jobProperties.getJobUpdateTimeoutSeconds())); }); } } diff --git a/core/src/main/resources/application.yml b/core/src/main/resources/application.yml index 84aa79a6fc4..51395cf6449 100644 --- a/core/src/main/resources/application.yml +++ b/core/src/main/resources/application.yml @@ -24,16 +24,38 @@ grpc: feast: jobs: - # Runner type for feature population jobs. Currently supported runner types are - # DirectRunner and DataflowRunner. - runner: DirectRunner - # Key-value dict of job options to be passed to the population jobs. - options: {} - updates: - # Job update polling interval in milliseconds: how often Feast checks if new jobs should be sent to the runner. - pollingIntervalMillis: 60000 - # Timeout in seconds for each attempt to update or submit a new job to the runner. - timeoutSeconds: 240 + # Job update polling interval in milliseconds: how often Feast checks if new jobs should be sent to the runner. + polling_interval_milliseconds: 60000 + + # Timeout in seconds for each attempt to update or submit a new job to the runner. + job_update_timeout_seconds: 240 + + # Name of the active runner in "runners" that should be used. Only a single runner can be active at one time. + active_runner: direct + + # List of runner configurations. Please see protos/feast/core/Runner.proto for more details + # Alternatively see the following for options https://api.docs.feast.dev/grpc/feast.core.pb.html#Runner + runners: + - name: direct + type: DirectRunner + options: {} + + - name: dataflow + type: DataflowRunner + options: + project: my_gcp_project + region: asia-east1 + zone: asia-east1-a + tempLocation: gs://bucket/tempLocation + network: default + subnetwork: regions/asia-east1/subnetworks/mysubnetwork + maxNumWorkers: 1 + autoscalingAlgorithm: THROUGHPUT_BASED + usePublicIps: false + workerMachineType: n1-standard-1 + deadLetterTableSpec: project_id:dataset_id.table_id + + # Configuration options for metric collection for all ingestion jobs metrics: # Enable metrics pushing for all ingestion jobs. enabled: false @@ -48,9 +70,10 @@ feast: # Feature stream type. Only kafka is supported. type: kafka # Feature stream options. + # See the following for options https://api.docs.feast.dev/grpc/feast.core.pb.html#KafkaSourceConfig options: topic: feast-features - bootstrapServers: kafka:9092 + bootstrapServers: localhost:9092 replicationFactor: 1 partitions: 1 diff --git a/core/src/test/java/feast/core/job/dataflow/DataflowJobManagerTest.java b/core/src/test/java/feast/core/job/dataflow/DataflowJobManagerTest.java index 2d562d38df2..59aec7f2a2c 100644 --- a/core/src/test/java/feast/core/job/dataflow/DataflowJobManagerTest.java +++ b/core/src/test/java/feast/core/job/dataflow/DataflowJobManagerTest.java @@ -79,7 +79,7 @@ public void setUp() { defaults.put("region", "region"); MetricsProperties metricsProperties = new MetricsProperties(); metricsProperties.setEnabled(false); - dfJobManager = new DataflowJobManager(dataflow, defaults, metricsProperties); + dfJobManager = new DataflowJobManager(defaults, metricsProperties); dfJobManager = spy(dfJobManager); } diff --git a/core/src/test/java/feast/core/service/JobCoordinatorServiceTest.java b/core/src/test/java/feast/core/service/JobCoordinatorServiceTest.java index aed889af86e..52e838c3d9d 100644 --- a/core/src/test/java/feast/core/service/JobCoordinatorServiceTest.java +++ b/core/src/test/java/feast/core/service/JobCoordinatorServiceTest.java @@ -73,7 +73,7 @@ public void setUp() { initMocks(this); feastProperties = new FeastProperties(); JobProperties jobProperties = new JobProperties(); - jobProperties.setJobUpdateTimeout(5); + jobProperties.setJobUpdateTimeoutSeconds(5); feastProperties.setJobs(jobProperties); } diff --git a/protos/feast/core/Runner.proto b/protos/feast/core/Runner.proto new file mode 100644 index 00000000000..55116f0ba34 --- /dev/null +++ b/protos/feast/core/Runner.proto @@ -0,0 +1,73 @@ +// +// * 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 java_package = "feast.core"; +option java_outer_classname = "RunnerProto"; +option go_package = "github.com/gojek/feast/sdk/go/protos/feast/core"; + +message DirectRunnerConfigOptions { + /** + * Controls the amount of target parallelism the DirectRunner will use. + * Defaults to the greater of the number of available processors and 3. Must be a value + * greater than zero. + */ + int32 targetParallelism = 1; + + /* BigQuery table specification, e.g. PROJECT_ID:DATASET_ID.PROJECT_ID */ + string deadLetterTableSpec = 2; +} + +message DataflowRunnerConfigOptions { + /* Project id to use when launching jobs. */ + string project = 1; + + /* The Google Compute Engine region for creating Dataflow jobs. */ + string region = 2; + + /* GCP availability zone for operations. */ + string zone = 3; + + /* Run the job as a specific service account, instead of the default GCE robot. */ + string serviceAccount = 4; + + /* GCE network for launching workers. */ + string network = 5; + + /* GCE subnetwork for launching workers. e.g. regions/asia-east1/subnetworks/mysubnetwork */ + string subnetwork = 6; + + /* Machine type to create Dataflow worker VMs as. */ + string workerMachineType = 7; + + /* The autoscaling algorithm to use for the workerpool. */ + string autoscalingAlgorithm = 8; + + /* Specifies whether worker pools should be started with public IP addresses. */ + bool usePublicIps = 9; + + // A pipeline level default location for storing temporary files. Support Google Cloud Storage locations, + // e.g. gs://bucket/object + string tempLocation = 10; + + /* (Dataflow Runner Only) The maximum number of workers to use for the workerpool. */ + int32 maxNumWorkers = 11; + + /* BigQuery table specification, e.g. PROJECT_ID:DATASET_ID.PROJECT_ID */ + string deadLetterTableSpec = 12; +} \ No newline at end of file diff --git a/protos/feast/core/Source.proto b/protos/feast/core/Source.proto index 8a6cbd415a2..b9e6227199b 100644 --- a/protos/feast/core/Source.proto +++ b/protos/feast/core/Source.proto @@ -39,9 +39,15 @@ enum SourceType { } message KafkaSourceConfig { - // - bootstrapServers: [comma delimited value of host[:port]] + // Comma separated list of Kafka bootstrap servers. Used for feature sets without a defined source host[:port]] string bootstrap_servers = 1; - // - topics: [Kafka topic name. This value is provisioned by core and should not be set by the user.] + // Kafka topic to use for feature sets without user defined topics string topic = 2; + + // Number of Kafka partitions to to use for managed feature stream. + int32 partitions = 3; + + // Defines the number of copies of managed feature stream Kafka. + int32 replicationFactor = 4; } \ No newline at end of file From ed9dd49fdfb40f2e4ee2109833c5414eba1384f9 Mon Sep 17 00:00:00 2001 From: Willem Pienaar Date: Sun, 12 Apr 2020 19:52:09 +0800 Subject: [PATCH 12/17] Remove commented out DataflowRunnerConfig setters --- .../job/dataflow/DataflowRunnerConfig.java | 55 ------------------- 1 file changed, 55 deletions(-) diff --git a/core/src/main/java/feast/core/job/dataflow/DataflowRunnerConfig.java b/core/src/main/java/feast/core/job/dataflow/DataflowRunnerConfig.java index d1fbaf06bb8..a095bacd249 100644 --- a/core/src/main/java/feast/core/job/dataflow/DataflowRunnerConfig.java +++ b/core/src/main/java/feast/core/job/dataflow/DataflowRunnerConfig.java @@ -31,61 +31,6 @@ public class DataflowRunnerConfig { public DataflowRunnerConfig(Map runnerConfigOptions) { BeanUtils.copyProperties(this, runnerConfigOptions); validate(); - - // - // Map props = BeanUtils.describe(someObject); - // if (props.containsKey("name")) { - // BeanUtils.setProperty(someObject, "name", "value"); - // } - // - // - // if (runnerConfigOptions.containsKey("project")) { - // setProject(runnerConfigOptions.get("project")); - // } - // - // if (runnerConfigOptions.containsKey("region")) { - // setRegion(runnerConfigOptions.get("region")); - // } - // - // if (runnerConfigOptions.containsKey("zone")) { - // setZone(runnerConfigOptions.get("zone")); - // } - // - // if (runnerConfigOptions.containsKey("serviceAccount")) { - // setServiceAccount(runnerConfigOptions.get("serviceAccount")); - // } - // - // if (runnerConfigOptions.containsKey("network")) { - // setNetwork(runnerConfigOptions.get("network")); - // } - // - // if (runnerConfigOptions.containsKey("subnetwork")) { - // setSubnetwork(runnerConfigOptions.get("subnetwork")); - // } - // - // if (runnerConfigOptions.containsKey("workerMachineType")) { - // setWorkerMachineType(runnerConfigOptions.get("workerMachineType")); - // } - // - // if (runnerConfigOptions.containsKey("autoscalingAlgorithm")) { - // setAutoscalingAlgorithm(runnerConfigOptions.get("autoscalingAlgorithm")); - // } - // - // if (runnerConfigOptions.containsKey("usePublicIps")) { - // setUsePublicIps(Boolean.parseBoolean(runnerConfigOptions.get("usePublicIps"))); - // } - // - // if (runnerConfigOptions.containsKey("tempLocation")) { - // setTempLocation(runnerConfigOptions.get("tempLocation")); - // } - // - // if (runnerConfigOptions.containsKey("maxNumWorkers")) { - // setMaxNumWorkers(Integer.parseInt(runnerConfigOptions.get("maxNumWorkers"))); - // } - // - // if (runnerConfigOptions.containsKey("deadLetterTableSpec")) { - // setDeadLetterTableSpec(runnerConfigOptions.get("deadLetterTableSpec")); - // } } /* (Dataflow Runner Only) Project id to use when launching jobs. */ From bcd119cc9fafa638642017884b2dede1775ccb45 Mon Sep 17 00:00:00 2001 From: Willem Pienaar Date: Sun, 12 Apr 2020 21:43:18 +0800 Subject: [PATCH 13/17] Clean up getJobManager and simplify field mapping in DataflowRunnerConfig --- .../java/feast/core/config/JobConfig.java | 13 +-- .../job/dataflow/DataflowRunnerConfig.java | 80 +++++++++++++------ 2 files changed, 57 insertions(+), 36 deletions(-) diff --git a/core/src/main/java/feast/core/config/JobConfig.java b/core/src/main/java/feast/core/config/JobConfig.java index 35483fba977..69636963bea 100644 --- a/core/src/main/java/feast/core/config/JobConfig.java +++ b/core/src/main/java/feast/core/config/JobConfig.java @@ -33,14 +33,13 @@ public class JobConfig { /** - * Get a JobManager according to the runner type and dataflow configuration. + * Get a JobManager according to the runner type and Dataflow configuration. * * @param feastProperties feast config properties */ @Bean @Autowired - public JobManager getJobManager( - FeastProperties feastProperties, DirectJobRegistry directJobRegistry) { + public JobManager getJobManager(FeastProperties feastProperties) { JobProperties jobProperties = feastProperties.getJobs(); FeastProperties.JobProperties.Runner runner = jobProperties.getActiveRunner(); @@ -51,15 +50,9 @@ public JobManager getJobManager( case DATAFLOW: return new DataflowJobManager(runnerConfigOptions, metrics); case DIRECT: - return new DirectRunnerJobManager(runnerConfigOptions, directJobRegistry, metrics); + return new DirectRunnerJobManager(runnerConfigOptions, new DirectJobRegistry(), metrics); default: throw new IllegalArgumentException("Unsupported runner: " + runner); } } - - /** Get a direct job registry */ - @Bean - public DirectJobRegistry directJobRegistry() { - return new DirectJobRegistry(); - } } diff --git a/core/src/main/java/feast/core/job/dataflow/DataflowRunnerConfig.java b/core/src/main/java/feast/core/job/dataflow/DataflowRunnerConfig.java index a095bacd249..6fe93ca80cd 100644 --- a/core/src/main/java/feast/core/job/dataflow/DataflowRunnerConfig.java +++ b/core/src/main/java/feast/core/job/dataflow/DataflowRunnerConfig.java @@ -16,62 +16,90 @@ */ package feast.core.job.dataflow; +import java.lang.reflect.Field; import java.util.Map; import java.util.Set; import javax.validation.*; import javax.validation.constraints.NotBlank; import lombok.Getter; import lombok.Setter; -import org.springframework.beans.BeanUtils; +/** DataflowRunnerConfig contains configuration fields for the Dataflow job runner. */ @Getter @Setter public class DataflowRunnerConfig { public DataflowRunnerConfig(Map runnerConfigOptions) { - BeanUtils.copyProperties(this, runnerConfigOptions); + + // Try to find all fields in DataflowRunnerConfig inside the runnerConfigOptions and map it into + // this object + for (Field field : DataflowRunnerConfig.class.getFields()) { + String fieldName = field.getName(); + try { + if (!runnerConfigOptions.containsKey(fieldName)) { + continue; + } + String value = runnerConfigOptions.get(fieldName); + + if (Boolean.class.equals(field.getType())) { + field.set(this, Boolean.valueOf(value)); + continue; + } + if (field.getType() == Integer.class) { + field.set(this, Integer.valueOf(value)); + continue; + } + field.set(this, value); + } catch (IllegalAccessException e) { + throw new RuntimeException( + String.format( + "Could not successfully convert DataflowRunnerConfig for key: %s", fieldName), + e); + } + } validate(); } - /* (Dataflow Runner Only) Project id to use when launching jobs. */ - @NotBlank private String project; + /* Project id to use when launching jobs. */ + @NotBlank public String project; - /* (Dataflow Runner Only) The Google Compute Engine region for creating Dataflow jobs. */ - @NotBlank private String region; + /* The Google Compute Engine region for creating Dataflow jobs. */ + @NotBlank public String region; - /* (Dataflow Runner Only) GCP availability zone for operations. */ - @NotBlank private String zone; + /* GCP availability zone for operations. */ + @NotBlank public String zone; - /* (Dataflow Runner Only) Run the job as a specific service account, instead of the default GCE robot. */ - @NotBlank private String serviceAccount; + /* Run the job as a specific service account, instead of the default GCE robot. */ + public String serviceAccount; - /* (Dataflow Runner Only) GCE network for launching workers. */ - @NotBlank private String network; + /* GCE network for launching workers. */ + @NotBlank public String network; - /* (Dataflow Runner Only) GCE subnetwork for launching workers. */ - @NotBlank private String subnetwork; + /* GCE subnetwork for launching workers. */ + @NotBlank public String subnetwork; - /* (Dataflow Runner Only) Machine type to create Dataflow worker VMs as. */ - private String workerMachineType; + /* Machine type to create Dataflow worker VMs as. */ + public String workerMachineType; - /* (Dataflow Runner Only) The autoscaling algorithm to use for the workerpool. */ - private String autoscalingAlgorithm; + /* The autoscaling algorithm to use for the workerpool. */ + public String autoscalingAlgorithm; - /* (Dataflow Runner Only) Specifies whether worker pools should be started with public IP addresses. */ - private Boolean usePublicIps; + /* Specifies whether worker pools should be started with public IP addresses. */ + public Boolean usePublicIps; /** - * (Dataflow Runner Only) A pipeline level default location for storing temporary files. Support - * Google Cloud Storage locations, e.g. gs://bucket/object + * A pipeline level default location for storing temporary files. Support Google Cloud Storage + * locations, e.g. gs://bucket/object */ - @NotBlank private String tempLocation; + @NotBlank public String tempLocation; - /* (Dataflow Runner Only) The maximum number of workers to use for the workerpool. */ - private Integer maxNumWorkers; + /* The maximum number of workers to use for the workerpool. */ + public Integer maxNumWorkers; /* BigQuery table specification, e.g. PROJECT_ID:DATASET_ID.PROJECT_ID */ - private String deadLetterTableSpec; + public String deadLetterTableSpec; + /** Validates Dataflow runner configuration options */ public void validate() { ValidatorFactory factory = Validation.buildDefaultValidatorFactory(); Validator validator = factory.getValidator(); From 1d932ff85235ec6c852c2e0121e7c8ec32f73a95 Mon Sep 17 00:00:00 2001 From: Willem Pienaar Date: Tue, 14 Apr 2020 12:33:05 +0800 Subject: [PATCH 14/17] Add static factory methods to retrievers --- .../job/dataflow/DataflowJobManagerTest.java | 4 ++ .../feast/serving/config/FeastProperties.java | 4 +- .../serving/config/ServingServiceConfig.java | 12 ++-- .../BigQueryHistoricalRetriever.java | 62 +++++++++---------- .../redis/retriever/RedisOnlineRetriever.java | 16 +++-- .../retriever/RedisOnlineRetrieverTest.java | 9 +-- 6 files changed, 60 insertions(+), 47 deletions(-) diff --git a/core/src/test/java/feast/core/job/dataflow/DataflowJobManagerTest.java b/core/src/test/java/feast/core/job/dataflow/DataflowJobManagerTest.java index 59aec7f2a2c..340ea2dfee6 100644 --- a/core/src/test/java/feast/core/job/dataflow/DataflowJobManagerTest.java +++ b/core/src/test/java/feast/core/job/dataflow/DataflowJobManagerTest.java @@ -77,6 +77,10 @@ public void setUp() { defaults = new HashMap<>(); defaults.put("project", "project"); defaults.put("region", "region"); + defaults.put("zone", "zone"); + defaults.put("tempLocation", "tempLocation"); + defaults.put("network", "network"); + defaults.put("subnetwork", "subnetwork"); MetricsProperties metricsProperties = new MetricsProperties(); metricsProperties.setEnabled(false); dfJobManager = new DataflowJobManager(defaults, metricsProperties); diff --git a/serving/src/main/java/feast/serving/config/FeastProperties.java b/serving/src/main/java/feast/serving/config/FeastProperties.java index 7fdcdcd2321..bf3387728a7 100644 --- a/serving/src/main/java/feast/serving/config/FeastProperties.java +++ b/serving/src/main/java/feast/serving/config/FeastProperties.java @@ -269,7 +269,7 @@ public StoreProto.Store toProto() * Get the subscriptions to this specific store. The subscriptions indicate which feature sets a * store subscribes to. * - * @return List of subscriptions in the form of {@link List}. + * @return List of subscriptions. */ public List getSubscriptions() { return subscriptions; @@ -289,7 +289,7 @@ public void setSubscriptions(List subscriptions) { * unique to the store. Please see protos/feast/core/Store.proto for the store specific * configuration options * - * @return the config as a {@link Map} + * @return Returns the store specific configuration */ public Map getConfig() { return config; diff --git a/serving/src/main/java/feast/serving/config/ServingServiceConfig.java b/serving/src/main/java/feast/serving/config/ServingServiceConfig.java index 13dd9400475..ec84e6c4fef 100644 --- a/serving/src/main/java/feast/serving/config/ServingServiceConfig.java +++ b/serving/src/main/java/feast/serving/config/ServingServiceConfig.java @@ -30,6 +30,7 @@ import feast.storage.connectors.bigquery.retriever.BigQueryHistoricalRetriever; import feast.storage.connectors.redis.retriever.RedisOnlineRetriever; import io.opentracing.Tracer; +import java.util.Map; import org.slf4j.Logger; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -47,11 +48,13 @@ public ServingService servingService( Tracer tracer) throws InvalidProtocolBufferException, JsonProcessingException { ServingService servingService = null; - StoreProto.Store store = feastProperties.getActiveStore().toProto(); + FeastProperties.Store store = feastProperties.getActiveStore(); + StoreProto.Store.StoreType storeType = store.toProto().getType(); + Map config = store.getConfig(); - switch (store.getType()) { + switch (storeType) { case REDIS: - OnlineRetriever redisRetriever = new RedisOnlineRetriever(store.getRedisConfig()); + OnlineRetriever redisRetriever = RedisOnlineRetriever.create(config); servingService = new OnlineServingService(redisRetriever, specService, tracer); break; case BIGQUERY: @@ -59,8 +62,7 @@ public ServingService servingService( throw new IllegalArgumentException( "Unable to instantiate JobService which is required by BigQueryHistoricalRetriever."); } - HistoricalRetriever bqRetriever = - BigQueryHistoricalRetriever.fromConfig(store.getBigqueryConfig()); + HistoricalRetriever bqRetriever = BigQueryHistoricalRetriever.create(config); servingService = new HistoricalServingService(bqRetriever, specService, jobService); break; case CASSANDRA: diff --git a/storage/connectors/bigquery/src/main/java/feast/storage/connectors/bigquery/retriever/BigQueryHistoricalRetriever.java b/storage/connectors/bigquery/src/main/java/feast/storage/connectors/bigquery/retriever/BigQueryHistoricalRetriever.java index 0c1d02cae41..cd372511c0a 100644 --- a/storage/connectors/bigquery/src/main/java/feast/storage/connectors/bigquery/retriever/BigQueryHistoricalRetriever.java +++ b/storage/connectors/bigquery/src/main/java/feast/storage/connectors/bigquery/retriever/BigQueryHistoricalRetriever.java @@ -25,7 +25,6 @@ import com.google.cloud.storage.Blob; import com.google.cloud.storage.Storage; import com.google.cloud.storage.StorageOptions; -import feast.core.StoreProto; import feast.serving.ServingAPIProto; import feast.serving.ServingAPIProto.DatasetSource; import feast.storage.api.retriever.FeatureSetRequest; @@ -35,6 +34,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.UUID; import java.util.concurrent.*; import java.util.stream.Collectors; @@ -50,6 +50,36 @@ public abstract class BigQueryHistoricalRetriever implements HistoricalRetriever public static final long TEMP_TABLE_EXPIRY_DURATION_MS = Duration.ofDays(1).toMillis(); private static final long SUBQUERY_TIMEOUT_SECS = 900; // 15 minutes + public static HistoricalRetriever create(Map config) { + + BigQuery bigquery = BigQueryOptions.getDefaultInstance().getService(); + Storage storage = StorageOptions.getDefaultInstance().getService(); + + String jobStagingLocation = config.get("staging-location"); + if (!jobStagingLocation.contains("://")) { + throw new IllegalArgumentException( + String.format("jobStagingLocation is not a valid URI: %s", jobStagingLocation)); + } + if (jobStagingLocation.endsWith("/")) { + jobStagingLocation = jobStagingLocation.substring(0, jobStagingLocation.length() - 1); + } + if (!jobStagingLocation.startsWith("gs://")) { + throw new IllegalArgumentException( + "Store type BIGQUERY requires job staging location to be a valid and existing Google Cloud Storage URI. Invalid staging location: " + + jobStagingLocation); + } + + return builder() + .setBigquery(bigquery) + .setDatasetId(config.get("dataset_id")) + .setProjectId(config.get("project_id")) + .setJobStagingLocation(config.get("staging-location")) + .setInitialRetryDelaySecs(Integer.parseInt(config.get("bigquery-initial-retry-delay-secs"))) + .setTotalTimeoutSecs(Integer.parseInt(config.get("bigquery-total-timeout-secs"))) + .setStorage(storage) + .build(); + } + public abstract String projectId(); public abstract String datasetId(); @@ -87,36 +117,6 @@ public abstract static class Builder { public abstract BigQueryHistoricalRetriever build(); } - public static BigQueryHistoricalRetriever fromConfig(StoreProto.Store.BigQueryConfig config) { - - String jobStagingLocation = config.getStagingLocation(); - BigQuery bigquery = BigQueryOptions.getDefaultInstance().getService(); - Storage storage = StorageOptions.getDefaultInstance().getService(); - - if (!jobStagingLocation.contains("://")) { - throw new IllegalArgumentException( - String.format("jobStagingLocation is not a valid URI: %s", jobStagingLocation)); - } - if (jobStagingLocation.endsWith("/")) { - jobStagingLocation = jobStagingLocation.substring(0, jobStagingLocation.length() - 1); - } - if (!jobStagingLocation.startsWith("gs://")) { - throw new IllegalArgumentException( - "Store type BIGQUERY requires job staging location to be a valid and existing Google Cloud Storage URI. Invalid staging location: " - + jobStagingLocation); - } - - return builder() - .setBigquery(bigquery) - .setDatasetId(config.getDatasetId()) - .setProjectId(config.getProjectId()) - .setJobStagingLocation(config.getStagingLocation()) - .setInitialRetryDelaySecs(config.getInitialRetryDelaySeconds()) - .setTotalTimeoutSecs(config.getTotalTimeoutSeconds()) - .setStorage(storage) - .build(); - } - @Override public String getStagingLocation() { return jobStagingLocation(); diff --git a/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/retriever/RedisOnlineRetriever.java b/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/retriever/RedisOnlineRetriever.java index 99de7f9112c..0963731988c 100644 --- a/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/retriever/RedisOnlineRetriever.java +++ b/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/retriever/RedisOnlineRetriever.java @@ -20,7 +20,6 @@ import com.google.protobuf.InvalidProtocolBufferException; import feast.core.FeatureSetProto.EntitySpec; import feast.core.FeatureSetProto.FeatureSetSpec; -import feast.core.StoreProto.Store.RedisConfig; import feast.serving.ServingAPIProto.FeatureReference; import feast.serving.ServingAPIProto.GetOnlineFeaturesRequest.EntityRow; import feast.storage.RedisProto.RedisKey; @@ -45,15 +44,22 @@ public class RedisOnlineRetriever implements OnlineRetriever { private final RedisCommands syncCommands; - public RedisOnlineRetriever(StatefulRedisConnection connection) { + private RedisOnlineRetriever(StatefulRedisConnection connection) { this.syncCommands = connection.sync(); } - public RedisOnlineRetriever(RedisConfig config) { + public static OnlineRetriever create(Map config) { + StatefulRedisConnection connection = - RedisClient.create(RedisURI.create(config.getHost(), config.getPort())) + RedisClient.create( + RedisURI.create(config.get("host"), Integer.parseInt(config.get("port")))) .connect(new ByteArrayCodec()); - this.syncCommands = connection.sync(); + + return new RedisOnlineRetriever(connection); + } + + public static OnlineRetriever create(StatefulRedisConnection connection) { + return new RedisOnlineRetriever(connection); } /** diff --git a/storage/connectors/redis/src/test/java/feast/storage/connectors/redis/retriever/RedisOnlineRetrieverTest.java b/storage/connectors/redis/src/test/java/feast/storage/connectors/redis/retriever/RedisOnlineRetrieverTest.java index 11c216c5a0c..41bbfaa74c4 100644 --- a/storage/connectors/redis/src/test/java/feast/storage/connectors/redis/retriever/RedisOnlineRetrieverTest.java +++ b/storage/connectors/redis/src/test/java/feast/storage/connectors/redis/retriever/RedisOnlineRetrieverTest.java @@ -33,6 +33,7 @@ import feast.serving.ServingAPIProto.GetOnlineFeaturesRequest.EntityRow; import feast.storage.RedisProto.RedisKey; import feast.storage.api.retriever.FeatureSetRequest; +import feast.storage.api.retriever.OnlineRetriever; import feast.types.FeatureRowProto.FeatureRow; import feast.types.FieldProto.Field; import feast.types.ValueProto.Value; @@ -52,14 +53,14 @@ public class RedisOnlineRetrieverTest { @Mock RedisCommands syncCommands; - private RedisOnlineRetriever redisOnlineRetriever; + private OnlineRetriever redisOnlineRetriever; private byte[][] redisKeyList; @Before public void setUp() { initMocks(this); when(connection.sync()).thenReturn(syncCommands); - redisOnlineRetriever = new RedisOnlineRetriever(connection); + redisOnlineRetriever = RedisOnlineRetriever.create(connection); redisKeyList = Lists.newArrayList( RedisKey.newBuilder() @@ -135,7 +136,7 @@ public void shouldReturnResponseWithValuesIfKeysPresent() { .map(x -> KeyValue.from(new byte[1], Optional.of(x.toByteArray()))) .collect(Collectors.toList()); - redisOnlineRetriever = new RedisOnlineRetriever(connection); + redisOnlineRetriever = RedisOnlineRetriever.create(connection); when(connection.sync()).thenReturn(syncCommands); when(syncCommands.mget(redisKeyList)).thenReturn(featureRowBytes); @@ -211,7 +212,7 @@ public void shouldReturnResponseWithUnsetValuesIfKeysNotPresent() { .collect(Collectors.toList()); featureRowBytes.add(null); - redisOnlineRetriever = new RedisOnlineRetriever(connection); + redisOnlineRetriever = RedisOnlineRetriever.create(connection); when(connection.sync()).thenReturn(syncCommands); when(syncCommands.mget(redisKeyList)).thenReturn(featureRowBytes); From 6587b7279f21617450bc5accca1cfc0989a2a0c0 Mon Sep 17 00:00:00 2001 From: Willem Pienaar Date: Wed, 15 Apr 2020 14:05:37 +0800 Subject: [PATCH 15/17] Remove runner specific comment typo --- protos/feast/core/Runner.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protos/feast/core/Runner.proto b/protos/feast/core/Runner.proto index 55116f0ba34..779f4d44bea 100644 --- a/protos/feast/core/Runner.proto +++ b/protos/feast/core/Runner.proto @@ -65,7 +65,7 @@ message DataflowRunnerConfigOptions { // e.g. gs://bucket/object string tempLocation = 10; - /* (Dataflow Runner Only) The maximum number of workers to use for the workerpool. */ + /* The maximum number of workers to use for the workerpool. */ int32 maxNumWorkers = 11; /* BigQuery table specification, e.g. PROJECT_ID:DATASET_ID.PROJECT_ID */ From 204b05b575d31c7a70c73cacd1431441e1286700 Mon Sep 17 00:00:00 2001 From: Willem Pienaar Date: Wed, 15 Apr 2020 15:04:52 +0800 Subject: [PATCH 16/17] Add oneOfStrings validator annotation for configuration validation --- .../feast/core/config/FeastProperties.java | 20 +++----- .../core/validators/OneOfStringValidator.java | 51 +++++++++++++++++++ .../feast/core/validators/OneOfStrings.java | 49 ++++++++++++++++++ 3 files changed, 107 insertions(+), 13 deletions(-) create mode 100644 core/src/main/java/feast/core/validators/OneOfStringValidator.java create mode 100644 core/src/main/java/feast/core/validators/OneOfStrings.java diff --git a/core/src/main/java/feast/core/config/FeastProperties.java b/core/src/main/java/feast/core/config/FeastProperties.java index 44c6b61f9c2..941d51f68c9 100644 --- a/core/src/main/java/feast/core/config/FeastProperties.java +++ b/core/src/main/java/feast/core/config/FeastProperties.java @@ -17,10 +17,10 @@ package feast.core.config; import feast.core.config.FeastProperties.StreamProperties.FeatureStreamOptions; +import feast.core.validators.OneOfStrings; import java.util.*; import javax.annotation.PostConstruct; import javax.validation.*; -import javax.validation.constraints.AssertTrue; import javax.validation.constraints.NotBlank; import javax.validation.constraints.NotNull; import javax.validation.constraints.Positive; @@ -130,7 +130,9 @@ public feast.core.job.Runner getType() { public static class StreamProperties { /* Feature stream type. Only "kafka" is supported. */ - @NotBlank private String type; + @OneOfStrings({"kafka"}) + @NotBlank + private String type; /* Feature stream options */ @NotNull private FeatureStreamOptions options; @@ -157,16 +159,6 @@ public static class FeatureStreamOptions { } } - /** - * Validates whether stream options are correct. - * - * @return Boolean used for assertion - */ - @AssertTrue - public boolean isValidStreamTypeSelected() { - return Objects.equals(getStream().getType(), "kafka"); - } - /** Feast population job metrics */ @Getter @Setter @@ -176,7 +168,9 @@ public static class MetricsProperties { private boolean enabled; /* Metric type. Possible options: statsd */ - @NotBlank private String type; + @OneOfStrings({"statsd"}) + @NotBlank + private String type; /* Host of metric sink */ @URL private String host; diff --git a/core/src/main/java/feast/core/validators/OneOfStringValidator.java b/core/src/main/java/feast/core/validators/OneOfStringValidator.java new file mode 100644 index 00000000000..6b84e44b01c --- /dev/null +++ b/core/src/main/java/feast/core/validators/OneOfStringValidator.java @@ -0,0 +1,51 @@ +/* + * 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.validators; + +import java.util.Arrays; +import javax.validation.ConstraintValidator; +import javax.validation.ConstraintValidatorContext; + +/** Validates whether a string value is found within a collection. */ +public class OneOfStringValidator implements ConstraintValidator { + + /** Values that are permitted for a specific instance of this validator */ + String[] allowedValues; + + /** + * Initialize the OneOfStringValidator with a collection of allowed String values. + * + * @param constraintAnnotation + */ + @Override + public void initialize(OneOfStrings constraintAnnotation) { + allowedValues = constraintAnnotation.value(); + } + + /** + * Validates whether a string value is found within the collection defined in the annotation. + * + * @param value String value that should be validated + * @param context Provides contextual data and operation when applying a given constraint + * validator + * @return Boolean value indicating whether the string is found within the allowed values. + */ + @Override + public boolean isValid(String value, ConstraintValidatorContext context) { + return Arrays.asList(allowedValues).contains(value); + } +} diff --git a/core/src/main/java/feast/core/validators/OneOfStrings.java b/core/src/main/java/feast/core/validators/OneOfStrings.java new file mode 100644 index 00000000000..dba290438c8 --- /dev/null +++ b/core/src/main/java/feast/core/validators/OneOfStrings.java @@ -0,0 +1,49 @@ +/* + * 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.validators; + +import java.lang.annotation.*; +import javax.validation.Constraint; +import javax.validation.Payload; + +/** + * Annotation for String "one of" validation. Allows for the definition of a collection through an + * annotation. The collection is used to test values defined in the object. + */ +@Target({ + ElementType.METHOD, + ElementType.FIELD, + ElementType.ANNOTATION_TYPE, + ElementType.CONSTRUCTOR, + ElementType.PARAMETER +}) +@Retention(RetentionPolicy.RUNTIME) +@Documented +@Constraint(validatedBy = OneOfStringValidator.class) +public @interface OneOfStrings { + /** @return Default error message that is returned if the incorrect value is set */ + String message() default "Field value must be one of the following: {value}"; + + /** Allows for the specification of validation groups to which this constraint belongs. */ + Class[] groups() default {}; + + /** An attribute payload that can be used to assign custom payload objects to a constraint. */ + Class[] payload() default {}; + + /** @return Default value that is returned if no allowed values are configured */ + String[] value() default {}; +} From e2b43cc11950d857b30b0d10ce07f0241b603db9 Mon Sep 17 00:00:00 2001 From: Willem Pienaar Date: Thu, 16 Apr 2020 13:34:26 +0800 Subject: [PATCH 17/17] Fix broken Dataflow unit test that depends on GOOGLE_APPLICATION_CREDENTIALS --- .../core/job/dataflow/DataflowJobManager.java | 27 +++++++++++++------ .../job/dataflow/DataflowJobManagerTest.java | 11 +++++++- 2 files changed, 29 insertions(+), 9 deletions(-) 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 bd90206117f..6002133e828 100644 --- a/core/src/main/java/feast/core/job/dataflow/DataflowJobManager.java +++ b/core/src/main/java/feast/core/job/dataflow/DataflowJobManager.java @@ -18,6 +18,7 @@ import static feast.core.util.PipelineUtil.detectClassPathResourcesToStage; +import com.google.api.client.auth.oauth2.Credential; import com.google.api.client.googleapis.auth.oauth2.GoogleCredential; import com.google.api.client.googleapis.javanet.GoogleNetHttpTransport; import com.google.api.client.json.jackson2.JacksonFactory; @@ -66,16 +67,15 @@ public class DataflowJobManager implements JobManager { public DataflowJobManager( Map runnerConfigOptions, MetricsProperties metricsProperties) { + this(runnerConfigOptions, metricsProperties, getGoogleCredential()); + } - DataflowRunnerConfig config = new DataflowRunnerConfig(runnerConfigOptions); + public DataflowJobManager( + Map runnerConfigOptions, + MetricsProperties metricsProperties, + Credential credential) { - GoogleCredential credential = null; - try { - credential = GoogleCredential.getApplicationDefault().createScoped(DataflowScopes.all()); - } catch (IOException e) { - throw new IllegalStateException( - "Unable to find credential required for Dataflow monitoring API", e); - } + DataflowRunnerConfig config = new DataflowRunnerConfig(runnerConfigOptions); Dataflow dataflow = null; try { @@ -97,6 +97,17 @@ public DataflowJobManager( this.location = config.getRegion(); } + private static Credential getGoogleCredential() { + GoogleCredential credential = null; + try { + credential = GoogleCredential.getApplicationDefault().createScoped(DataflowScopes.all()); + } catch (IOException e) { + throw new IllegalStateException( + "Unable to find credential required for Dataflow monitoring API", e); + } + return credential; + } + @Override public Runner getRunnerType() { return RUNNER_TYPE; diff --git a/core/src/test/java/feast/core/job/dataflow/DataflowJobManagerTest.java b/core/src/test/java/feast/core/job/dataflow/DataflowJobManagerTest.java index 340ea2dfee6..e610f393732 100644 --- a/core/src/test/java/feast/core/job/dataflow/DataflowJobManagerTest.java +++ b/core/src/test/java/feast/core/job/dataflow/DataflowJobManagerTest.java @@ -22,6 +22,8 @@ import static org.mockito.Mockito.*; import static org.mockito.MockitoAnnotations.initMocks; +import com.google.api.client.auth.oauth2.Credential; +import com.google.api.client.googleapis.testing.auth.oauth2.MockGoogleCredential; import com.google.api.services.dataflow.Dataflow; import com.google.common.collect.Lists; import com.google.protobuf.Duration; @@ -83,7 +85,14 @@ public void setUp() { defaults.put("subnetwork", "subnetwork"); MetricsProperties metricsProperties = new MetricsProperties(); metricsProperties.setEnabled(false); - dfJobManager = new DataflowJobManager(defaults, metricsProperties); + Credential credential = null; + try { + credential = MockGoogleCredential.getApplicationDefault(); + } catch (IOException e) { + e.printStackTrace(); + } + + dfJobManager = new DataflowJobManager(defaults, metricsProperties, credential); dfJobManager = spy(dfJobManager); }