Skip to content

Commit fdbc376

Browse files
author
Oleksii Moskalenko
authored
Keep StoreProto inside JobStore to decouple JobCoordination from SpecService internals (#852)
* keep store proto in JobStore * lint python * pr comments * more comments
1 parent e76392c commit fdbc376

15 files changed

Lines changed: 227 additions & 189 deletions

‎core/src/main/java/feast/core/dao/JobRepository.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,6 @@ public interface JobRepository extends JpaRepository<Job, String> {
4040

4141
List<Job> findByFeatureSetJobStatusesIn(List<FeatureSetJobStatus> featureSetsJobStatuses);
4242

43-
// find jobs by feast store name
44-
List<Job> findByStoresName(String storeName);
43+
// find jobs that have at least one store with given name
44+
List<Job> findByJobStoresIdStoreName(String storeName);
4545
}

‎core/src/main/java/feast/core/job/ConsolidatedJobStrategy.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,12 @@ public Job getOrCreateJob(Source source, Set<Store> stores) {
5050
.findFirstBySourceTypeAndSourceConfigAndStoreNameAndStatusNotInOrderByLastUpdatedDesc(
5151
source.getType(), source.getConfig(), null, JobStatus.getTerminalStates())
5252
.orElseGet(
53-
() ->
54-
Job.builder()
55-
.setSource(source)
56-
.setStores(stores)
57-
.setFeatureSetJobStatuses(new HashSet<>())
58-
.build());
53+
() -> {
54+
Job job =
55+
Job.builder().setSource(source).setFeatureSetJobStatuses(new HashSet<>()).build();
56+
job.setStores(stores);
57+
return job;
58+
});
5959
}
6060

6161
@Override

‎core/src/main/java/feast/core/job/JobPerStoreStrategy.java‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,16 @@ public Job getOrCreateJob(Source source, Set<Store> stores) {
5555
.findFirstBySourceTypeAndSourceConfigAndStoreNameAndStatusNotInOrderByLastUpdatedDesc(
5656
source.getType(), source.getConfig(), store.getName(), JobStatus.getTerminalStates())
5757
.orElseGet(
58-
() ->
59-
Job.builder()
60-
.setSource(source)
61-
.setStoreName(store.getName())
62-
.setStores(stores)
63-
.setFeatureSetJobStatuses(new HashSet<>())
64-
.build());
58+
() -> {
59+
Job job =
60+
Job.builder()
61+
.setSource(source)
62+
.setStoreName(store.getName())
63+
.setFeatureSetJobStatuses(new HashSet<>())
64+
.build();
65+
job.setStores(stores);
66+
return job;
67+
});
6568
}
6669

6770
@Override

‎core/src/main/java/feast/core/model/Job.java‎

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -76,16 +76,8 @@ public JobBuilder setSource(Source source) {
7676
private String sourceConfig;
7777

7878
// Sinks
79-
@ManyToMany
80-
@JoinTable(
81-
name = "jobs_stores",
82-
joinColumns = @JoinColumn(name = "job_id"),
83-
inverseJoinColumns = @JoinColumn(name = "store_name"),
84-
indexes = {
85-
@Index(name = "idx_jobs_stores_job_id", columnList = "job_id"),
86-
@Index(name = "idx_jobs_stores_store_name", columnList = "store_name")
87-
})
88-
private Set<Store> stores;
79+
@OneToMany(mappedBy = "job", cascade = CascadeType.ALL)
80+
private Set<JobStore> jobStores = new HashSet<>();
8981

9082
@Deprecated
9183
@Column(name = "store_name")
@@ -144,6 +136,30 @@ public void addAllFeatureSets(Set<FeatureSet> featureSets) {
144136
}
145137
}
146138

139+
/**
140+
* Materialize stores from protos stored in {@link JobStore}
141+
*
142+
* @return set of {@link Store}
143+
*/
144+
public Set<Store> getStores() {
145+
return getJobStores().stream()
146+
.map(JobStore::getStoreProto)
147+
.map(Store::fromProto)
148+
.collect(Collectors.toSet());
149+
}
150+
151+
/**
152+
* Copy stores as protos to JobStores {@link JobStore} to keep job's version of allocated stores.
153+
*
154+
* @param stores allocated set of {@link Store}
155+
*/
156+
public void setStores(Set<Store> stores) {
157+
jobStores = new HashSet<>();
158+
for (Store store : stores) {
159+
jobStores.add(new JobStore(this, store));
160+
}
161+
}
162+
147163
/**
148164
* Convert a job model to ingestion job proto
149165
*
@@ -177,21 +193,21 @@ public IngestionJobProto.IngestionJob toProto() throws InvalidProtocolBufferExce
177193
public Job clone() {
178194
Job job =
179195
Job.builder()
180-
.setStores(getStores())
181196
.setStoreName(getStoreName())
182197
.setSourceConfig(getSourceConfig())
183198
.setSourceType(getSourceType())
184199
.setFeatureSetJobStatuses(new HashSet<>())
185200
.setRunner(getRunner())
186201
.setStatus(JobStatus.UNKNOWN)
187202
.build();
203+
job.setStores(getStores());
188204
job.addAllFeatureSets(getFeatureSets());
189205
return job;
190206
}
191207

192208
@Override
193209
public int hashCode() {
194-
return Objects.hash(getSource(), this.stores, this.runner);
210+
return Objects.hash(getSource(), getStores(), this.runner);
195211
}
196212

197213
@Override
@@ -204,7 +220,7 @@ public boolean equals(Object obj) {
204220
return false;
205221
} else if (!getSource().equals(other.getSource())) {
206222
return false;
207-
} else if (!stores.equals(other.stores)) {
223+
} else if (!getStores().equals(other.getStores())) {
208224
return false;
209225
}
210226
return true;

‎core/src/main/java/feast/core/model/JobStore.java‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
* Copyright 2018-2020 The Feast Authors
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* https://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package feast.core.model;
18+
19+
import com.google.common.base.Objects;
20+
import com.google.protobuf.InvalidProtocolBufferException;
21+
import feast.proto.core.StoreProto;
22+
import java.io.ByteArrayOutputStream;
23+
import java.io.IOException;
24+
import java.io.Serializable;
25+
import javax.persistence.*;
26+
import javax.persistence.Entity;
27+
import lombok.AllArgsConstructor;
28+
import lombok.EqualsAndHashCode;
29+
import lombok.Getter;
30+
import lombok.Setter;
31+
32+
/**
33+
* Represents {@link Store}s attached to one {@link Job}. Keeps copy of Store's proto to detect
34+
* changes in original Store.
35+
*/
36+
@Entity
37+
@Table(
38+
name = "jobs_stores",
39+
indexes = {
40+
@Index(name = "idx_jobs_stores_job_id", columnList = "job_id"),
41+
@Index(name = "idx_jobs_stores_store_name", columnList = "store_name")
42+
})
43+
@Getter
44+
@Setter
45+
public class JobStore {
46+
@Embeddable
47+
@EqualsAndHashCode
48+
@AllArgsConstructor
49+
public static class JobStoreKey implements Serializable {
50+
public JobStoreKey() {}
51+
52+
@Column(name = "job_id")
53+
String jobId;
54+
55+
@Column(name = "store_name")
56+
String storeName;
57+
}
58+
59+
@EmbeddedId private JobStoreKey id = new JobStoreKey();
60+
61+
@ManyToOne
62+
@MapsId("jobId")
63+
@JoinColumn(name = "job_id")
64+
private Job job;
65+
66+
@Column(name = "store_proto", nullable = false)
67+
@Lob
68+
private byte[] storeProto;
69+
70+
public JobStore() {}
71+
72+
public JobStore(Job job, Store store) {
73+
this.job = job;
74+
this.id.storeName = store.getName();
75+
try {
76+
setStoreProto(store.toProto());
77+
} catch (InvalidProtocolBufferException e) {
78+
throw new RuntimeException("Couldn't convert Store to proto. Reason: %s", e.getCause());
79+
}
80+
}
81+
82+
public StoreProto.Store getStoreProto() {
83+
try {
84+
return StoreProto.Store.parseFrom(this.storeProto);
85+
} catch (InvalidProtocolBufferException e) {
86+
return StoreProto.Store.newBuilder().build();
87+
}
88+
}
89+
90+
public void setStoreProto(StoreProto.Store storeProto) {
91+
ByteArrayOutputStream output = new ByteArrayOutputStream();
92+
try {
93+
storeProto.writeTo(output);
94+
} catch (IOException e) {
95+
throw new RuntimeException(
96+
String.format("Couldn't write StoreProto to byteArray: %s", e.getCause()));
97+
}
98+
99+
this.storeProto = output.toByteArray();
100+
}
101+
102+
@Override
103+
public boolean equals(Object o) {
104+
if (this == o) return true;
105+
if (o == null || getClass() != o.getClass()) return false;
106+
JobStore jobStore = (JobStore) o;
107+
return Objects.equal(this.id, jobStore.id)
108+
&& Objects.equal(this.storeProto, jobStore.storeProto);
109+
}
110+
111+
@Override
112+
public int hashCode() {
113+
return Objects.hashCode(this.id, this.storeProto);
114+
}
115+
}

‎core/src/main/java/feast/core/model/Store.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@
4747
@AllArgsConstructor
4848
@Entity
4949
@Table(name = "stores")
50-
public class Store extends AbstractTimestampEntity {
50+
public class Store {
5151

5252
// Name of the store. Must be unique
5353
@Id

‎core/src/main/java/feast/core/service/JobCoordinatorService.java‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -227,10 +227,6 @@ private boolean jobRequiresUpgrade(Job job, Set<Store> stores) {
227227
return true;
228228
}
229229

230-
if (stores.stream().anyMatch(s -> s.getLastUpdated().after(job.getCreated()))) {
231-
return true;
232-
}
233-
234230
return false;
235231
}
236232

‎core/src/main/java/feast/core/service/JobService.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ public ListIngestionJobsResponse listJobs(ListIngestionJobsRequest request)
108108
// multiple filters can apply together in an 'and' operation
109109
if (!filter.getStoreName().isEmpty()) {
110110
// find jobs by name
111-
List<Job> jobs = this.jobRepository.findByStoresName(filter.getStoreName());
111+
List<Job> jobs = this.jobRepository.findByJobStoresIdStoreName(filter.getStoreName());
112112
Set<String> jobIds = jobs.stream().map(Job::getId).collect(Collectors.toSet());
113113
matchingJobIds = this.mergeResults(matchingJobIds, jobIds);
114114
}

‎core/src/main/resources/db/migration/V2.4__Store_Timestamps.sql‎

Lines changed: 0 additions & 2 deletions
This file was deleted.

‎core/src/main/resources/db/migration/V2.4__Store_proto.sql‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTER TABLE jobs_stores ADD COLUMN store_proto oid not null;

0 commit comments

Comments
 (0)