Coalesce rows #89

tims · 2019-01-18T12:19:21Z

/wip

tims · 2019-01-18T12:21:33Z

@pradithya @woop @budi This is the PR I mentioned to you in chat. Take a look if you're interested in the implementation details. It's still needs a few changes.

eg

change the import spec options field to be sourceOptions, but I want to wait for another PR so we have less merge conflicts.
group by entityname +entitykey, instead of FeatureRowKey, but this is dependent on removing history from the serving stores first.

tims · 2019-01-18T16:04:56Z

/wip
/hold

pradithya · 2019-01-21T05:22:04Z

ingestion/src/main/java/feast/ingestion/options/JobOptions.java

+
+public class JobOptions implements Options {
+
+  private long sampleLimit;


how all these configuration will affect the ingestion?

pradithya · 2019-01-21T05:28:09Z

change the import spec options field to be sourceOptions, but I want to wait for another PR so we have less merge conflicts.

Is it #87 ?

group by entityname +entitykey, instead of FeatureRowKey, but this is dependent on removing history from the serving stores first.

is it possible without #87 ?

pradithya · 2019-01-21T09:15:30Z

ingestion/src/main/java/feast/ingestion/transform/CoalesceFeatureRows.java

+public class CoalesceFeatureRows extends
+    PTransform<PCollection<FeatureRow>, PCollection<FeatureRow>> {
+
+  private static final Comparator<Timestamp> TIMESTAMP_COMPARATOR = Comparator


we can use Timestamps.compare(t1, t2) for this

pradithya · 2019-01-21T14:31:50Z

ingestion/src/main/java/feast/ingestion/ImportJob.java

@@ -170,6 +175,12 @@ public void expand() {
                    ParDo.of(new RoundEventTimestampsDoFn())),
            pFeatureRows.getErrors());

+    if (jobOptions.isCoalesceRowsEnabled()) {
+      pFeatureRows = pFeatureRows.apply("foo", new CoalescePFeatureRows(


"foo" can be replaced with "Coalesce Feature Row"

syvlabs · 2019-01-23T03:35:19Z

ingestion/src/main/java/feast/ingestion/options/JobOptions.java

+    return sampleLimit;
+  }
+
+  public void setSampleimit(long sampleLimit) {


typo on method name

… key in the global window

…ing history from serving stores

…ptions

…e store

tims · 2019-01-27T15:16:15Z

/hold cancel

/assign pradithya please review

tims · 2019-01-27T15:20:10Z

I've included not writing to the feature stores as requested.

tims · 2019-01-27T15:20:34Z

Related to #87

tims · 2019-01-27T15:22:37Z

Biggest change externally is to the ImportSpec.

From


message ImportSpec {
  string type = 1;
  map<string, string> options = 2;
  repeated string entities = 3;
  Schema schema = 4;
}

To:


message ImportSpec {
  string type = 1;
  map<string, string> sourceOptions = 2;
  map<string, string> jobOptions = 5;
  repeated string entities = 3;
  Schema schema = 4;
}

tims · 2019-01-27T15:26:08Z

To turn on coalesceRows you need to pass jobOptions: { coalesceRows.enabled: "true" }

You can also set, coalesceRows.delaySeconds, and coalesceRows.timeoutSeconds, but these settings are only relevant for streaming.

The delay indicates how many seconds the watermark must advance before the rows are flushed, default is 10 seconds.
They timeout indicates how many seconds the watermark must advance before the previous state is cleared. The default is 0, which indicates it will never be cleared.

tims · 2019-01-27T19:59:29Z

should this be on or off by default?

pradithya · 2019-01-28T02:09:58Z

should this be on or off by default?

I think it should be on by default.
What is the behavior when it's off?

Is this in-scope for 0.1.0?

tims · 2019-01-30T03:26:10Z

/assign zhilingc

Can you run with some of your existing workloads?

zhilingc · 2019-02-10T17:15:33Z

@tims done, I think we're good to merge it in

zhilingc · 2019-02-10T17:15:39Z

/approve

zhilingc · 2019-02-10T17:15:54Z

sorry, sausage fingers :(

feast-ci-bot · 2019-02-10T17:15:57Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zhilingc

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [zhilingc]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

tims · 2019-02-11T14:59:14Z

/lgtm

feast-ci-bot · 2019-02-11T14:59:14Z

@tims: you cannot LGTM your own PR.

In response to this:

/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

tims · 2019-02-11T15:01:06Z

boo... @pradithya can you lgtm this?

pradithya · 2019-02-11T15:03:24Z

/lgtm

feast-ci-bot added the size/XXL label Jan 18, 2019

feast-ci-bot added the do-not-merge/hold label Jan 18, 2019

pradithya reviewed Jan 21, 2019

View reviewed changes

syvlabs reviewed Jan 23, 2019

View reviewed changes

tims force-pushed the coalescerows branch from 7e88982 to a1b4107 Compare January 24, 2019 13:22

tims added 8 commits January 27, 2019 22:57

ptransform to coalesce feature rows in batch and streaming

2400927

add coalesce feature rows transform, to build state of latest row per…

262cd09

… key in the global window

coalesce for serving on entity name and key only, preparing for remov…

d34e298

…ing history from serving stores

change to rounding granularities after feature row coalesce

83d33a7

rename importSpec.options to be importspec.sourceOptions and add jobO…

07104d4

…ptions

only write out the latest row for bigtable and remove postgres featur…

581762e

…e store

do not write historical values to redis serving store

b6e05b1

formatting and fix test

301d7fc

tims force-pushed the coalescerows branch from a1b4107 to 301d7fc Compare January 27, 2019 15:03

core importspec validator test

d8ef749

feast-ci-bot removed the do-not-merge/hold label Jan 27, 2019

tims added 2 commits January 27, 2019 23:17

fix typo on option name

4371ce2

use Timestamps.compare

8fe8b5c

remove redudant proto message

39c11da

tims mentioned this pull request Jan 27, 2019

Refactor stores #110

Merged

tims self-assigned this Jan 30, 2019

feast-ci-bot assigned zhilingc Jan 30, 2019

tims and others added 3 commits February 8, 2019 15:36

default coalescerows to true

ca9332e

Update go protos and cli tests

4f70114

Update python sdk for new import spec

a734340

zhilingc closed this Feb 10, 2019

zhilingc reopened this Feb 10, 2019

feast-ci-bot added the approved label Feb 10, 2019

feast-ci-bot assigned pradithya Feb 11, 2019

feast-ci-bot added the lgtm label Feb 11, 2019

feast-ci-bot merged commit 5a6edc4 into feast-dev:master Feb 11, 2019

tims mentioned this pull request Feb 12, 2019

Coalesce FeatureRows for improved "latest" value consistency in serving stores #88

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Coalesce rows #89

Coalesce rows #89

tims commented Jan 18, 2019

tims commented Jan 18, 2019

tims commented Jan 18, 2019

pradithya Jan 21, 2019

pradithya commented Jan 21, 2019

pradithya Jan 21, 2019

pradithya Jan 21, 2019

syvlabs Jan 23, 2019

tims commented Jan 27, 2019

tims commented Jan 27, 2019

tims commented Jan 27, 2019

tims commented Jan 27, 2019

tims commented Jan 27, 2019

tims commented Jan 27, 2019

pradithya commented Jan 28, 2019 •

edited

Loading

tims commented Jan 30, 2019

zhilingc commented Feb 10, 2019

zhilingc commented Feb 10, 2019

zhilingc commented Feb 10, 2019

feast-ci-bot commented Feb 10, 2019

tims commented Feb 11, 2019

feast-ci-bot commented Feb 11, 2019

tims commented Feb 11, 2019

pradithya commented Feb 11, 2019


		public class JobOptions implements Options {

		private long sampleLimit;

Coalesce rows #89

Coalesce rows #89

Conversation

tims commented Jan 18, 2019

tims commented Jan 18, 2019

tims commented Jan 18, 2019

pradithya Jan 21, 2019

Choose a reason for hiding this comment

pradithya commented Jan 21, 2019

pradithya Jan 21, 2019

Choose a reason for hiding this comment

pradithya Jan 21, 2019

Choose a reason for hiding this comment

syvlabs Jan 23, 2019

Choose a reason for hiding this comment

tims commented Jan 27, 2019

tims commented Jan 27, 2019

tims commented Jan 27, 2019

tims commented Jan 27, 2019

tims commented Jan 27, 2019

tims commented Jan 27, 2019

pradithya commented Jan 28, 2019 • edited Loading

tims commented Jan 30, 2019

zhilingc commented Feb 10, 2019

zhilingc commented Feb 10, 2019

zhilingc commented Feb 10, 2019

feast-ci-bot commented Feb 10, 2019

tims commented Feb 11, 2019

feast-ci-bot commented Feb 11, 2019

tims commented Feb 11, 2019

pradithya commented Feb 11, 2019

pradithya commented Jan 28, 2019 •

edited

Loading