You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@edgent.apache.org by wmarshall484 <gi...@git.apache.org> on 2016/03/31 19:14:21 UTC
[GitHub] incubator-quarks pull request: [WIP] Added join and joinLast funct...
GitHub user wmarshall484 opened a pull request:
https://github.com/apache/incubator-quarks/pull/60
[WIP] Added join and joinLast functionality.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/wmarshall484/incubator-quarks join
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-quarks/pull/60.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #60
----
commit dfb63325f878c7087136d06905ac997d9dc5f392
Author: wcmarsha <wc...@us.ibm.com>
Date: 2016-03-31T17:11:20Z
Added join and joinLast functionality.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-quarks pull request: [WIP] Added join and joinLast funct...
Posted by ddebrunner <gi...@git.apache.org>.
Github user ddebrunner commented on a diff in the pull request:
https://github.com/apache/incubator-quarks/pull/60#discussion_r58104612
--- Diff: api/topology/src/main/java/quarks/topology/TStream.java ---
@@ -439,4 +440,11 @@ else if (WARNING.equals(lr.getLevel()))
* @return set of tags
*/
Set<String> getTags();
+
+ <J, U, K> TStream<J> join(Function<T, K> keyer, TWindow<U, K> window, BiFunction<T, List<U>, J> joiner);
+
+ <J, U, K> TStream<J> joinLast(Function<T, K> keyer, TStream<U> lastStream, Function<U, K> lastStreamKeyer, BiFunction<T, U, J> joiner);
+
+ <J, U> TStream<J> joinLast(TStream<U> lastStream, BiFunction<T, U, J> joiner);
--- End diff --
Why have this method? We don't provide unpartitioned variants of other methods do we?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-quarks pull request: [WIP] Added join and joinLast funct...
Posted by dlaboss <gi...@git.apache.org>.
Github user dlaboss commented on a diff in the pull request:
https://github.com/apache/incubator-quarks/pull/60#discussion_r58204284
--- Diff: api/window/src/main/java/quarks/window/Window.java ---
@@ -157,4 +158,10 @@ Licensed to the Apache Software Foundation (ASF) under one
*/
Function<T, K> getKeyFunction();
+ /**
+ * Retrieves the partitions in the window.
+ * @return A map of the window's keys and partitions.
--- End diff --
How about adding doc for the behavior / use requirements of the returned map as they relate to: is the returned map/partitions "stable" or can the state change? What happens if one attempts to change the map or the partitions it contains - is it backed by the live window's objects?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-quarks pull request: Added join and joinLast functionali...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-quarks/pull/60
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-quarks pull request: [WIP] Added join and joinLast funct...
Posted by wmarshall484 <gi...@git.apache.org>.
Github user wmarshall484 commented on a diff in the pull request:
https://github.com/apache/incubator-quarks/pull/60#discussion_r58490405
--- Diff: spi/topology/src/main/java/quarks/topology/spi/graph/TWindowTimeImpl.java ---
@@ -94,5 +94,33 @@ Licensed to the Apache Software Foundation (ASF) under one
Aggregate<T,U,K> op = new Aggregate<T,U,K>(window, batcher);
return feeder().pipe(op);
+ }
+
+ /**
+ * @return the time
+ */
+ public long getTime() {
+ return time;
+ }
+
+ /**
+ * @param time the time to set
+ */
--- End diff --
The set() methods were generated accidentally; they have been removed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-quarks pull request: [WIP] Added join and joinLast funct...
Posted by dlaboss <gi...@git.apache.org>.
Github user dlaboss commented on a diff in the pull request:
https://github.com/apache/incubator-quarks/pull/60#discussion_r58201225
--- Diff: spi/topology/src/main/java/quarks/topology/spi/graph/TWindowTimeImpl.java ---
@@ -94,5 +94,33 @@ Licensed to the Apache Software Foundation (ASF) under one
Aggregate<T,U,K> op = new Aggregate<T,U,K>(window, batcher);
return feeder().pipe(op);
+ }
+
+ /**
+ * @return the time
+ */
+ public long getTime() {
+ return time;
+ }
+
+ /**
+ * @param time the time to set
+ */
--- End diff --
windowing impl newbie wondering about the addition of the setters... the ctor requires the values be supplied. Is it really necessary to make them changeable post construction? They can't be made final? Changing the values after calling aggregate() or batch() seems to have no affect on the behavior of the windows associated with them. Maybe just WIP, maybe just need more doc stating that, maybe some class doc clarifying usage of the class - e.g., is a TWindowTimeImpl instance supposed to represent a single window instance or a factory for one or more actual windows (i.e., it's legal to call both aggregate() and batch(), or each one more than once)?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-quarks pull request: [WIP] Added join and joinLast funct...
Posted by ddebrunner <gi...@git.apache.org>.
Github user ddebrunner commented on a diff in the pull request:
https://github.com/apache/incubator-quarks/pull/60#discussion_r58104881
--- Diff: spi/topology/src/main/java/quarks/topology/spi/graph/ConnectorStream.java ---
@@ -163,6 +175,100 @@ protected Graph graph() {
}
@Override
+ public <J, U, K> TStream<J> join(Function<T, K> keyer,
+ TWindow<U, K> twindow, BiFunction<T, List<U>, J> joiner) {
+
+ if(twindow instanceof TWindowImpl){
+ TStream<U> lastStream = twindow.feeder();
--- End diff --
There's several lines of code that are common between the two implementations, can they be shared?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-quarks pull request: [WIP] Added join and joinLast funct...
Posted by wmarshall484 <gi...@git.apache.org>.
Github user wmarshall484 commented on a diff in the pull request:
https://github.com/apache/incubator-quarks/pull/60#discussion_r58492726
--- Diff: spi/topology/src/main/java/quarks/topology/spi/graph/ConnectorStream.java ---
@@ -163,6 +175,100 @@ protected Graph graph() {
}
@Override
+ public <J, U, K> TStream<J> join(Function<T, K> keyer,
+ TWindow<U, K> twindow, BiFunction<T, List<U>, J> joiner) {
+
+ if(twindow instanceof TWindowImpl){
+ TStream<U> lastStream = twindow.feeder();
+ BiFunction<List<U>,K, Object> processor = Functions.synchronizedBiFunction((list, key) -> null);
+ Window<U, K, LinkedList<U>> window = Windows.lastNProcessOnInsert(((TWindowImpl<U, K>)twindow).getSize(), twindow.getKeyFunction());
+ Aggregate<U,Object,K> op = new Aggregate<U,Object,K>(window, processor);
+ lastStream.pipe(op);
+ return this.map((tuple) -> {
+ Partition<U, K, ? extends List<U>> part = window.getPartitions().get(keyer.apply(tuple));
+ if(part == null)
+ return null;
+ J ret;
+ synchronized (part) {
+ List<U> last = part.getContents();
+ ret = joiner.apply(tuple, last);
+ }
+ return ret;
+ });
+ }
+
+ else if (twindow instanceof TWindowTimeImpl){
+ TStream<U> lastStream = twindow.feeder();
+ BiFunction<List<U>,K, Object> processor = Functions.synchronizedBiFunction((list, key) -> null);
+ long time = ((TWindowTimeImpl<U, K>)(twindow)).getTime();
+ TimeUnit unit = ((TWindowTimeImpl<U, K>)(twindow)).getUnit();
+ Window<U, K, InsertionTimeList<U>> window =
+ Windows.window(
+ alwaysInsert(),
+ scheduleEvictIfEmpty(time, unit),
+ evictOlderWithProcess(time, unit),
+ processOnInsert(),
+ twindow.getKeyFunction(),
+ insertionTimeList());
+ Aggregate<U,Object,K> op = new Aggregate<U,Object,K>(window, processor);
--- End diff --
The Aggregate isn't unconnected. With the next line
```java
lastStream.pipe(op);
```
its state is set to update based on the tuples from `twindow.feeder` stream. The `window` object which was created earlier is included in the `joiner` function via closure so its partitions can be accessed.
Including the window in the `joiner` function through a closure removes the need to create an additional oplet with two input ports. The downside is that this approach only works if the Quarks application is only running inside of a single JVM, which it is for now (but maybe not later?)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-quarks pull request: [WIP] Added join and joinLast funct...
Posted by wmarshall484 <gi...@git.apache.org>.
Github user wmarshall484 commented on a diff in the pull request:
https://github.com/apache/incubator-quarks/pull/60#discussion_r58492046
--- Diff: api/window/src/main/java/quarks/window/Window.java ---
@@ -157,4 +158,10 @@ Licensed to the Apache Software Foundation (ASF) under one
*/
Function<T, K> getKeyFunction();
+ /**
+ * Retrieves the partitions in the window.
+ * @return A map of the window's keys and partitions.
--- End diff --
The returned map is stable when synchronizing on its intrinsic lock, for example:
```java
Map<Integer, Partition<Integer, Integer, ArrayList<Integer> > > partitions = window.getPartitions();
synchronized(partitions){
// Stable operations on/with partitions
}
```
I've amended the doc to address this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-quarks pull request: [WIP] Added join and joinLast funct...
Posted by wmarshall484 <gi...@git.apache.org>.
Github user wmarshall484 commented on a diff in the pull request:
https://github.com/apache/incubator-quarks/pull/60#discussion_r58492137
--- Diff: spi/topology/src/main/java/quarks/topology/spi/graph/ConnectorStream.java ---
@@ -163,6 +175,100 @@ protected Graph graph() {
}
@Override
+ public <J, U, K> TStream<J> join(Function<T, K> keyer,
+ TWindow<U, K> twindow, BiFunction<T, List<U>, J> joiner) {
+
+ if(twindow instanceof TWindowImpl){
+ TStream<U> lastStream = twindow.feeder();
--- End diff --
Fixed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-quarks pull request: [WIP] Added join and joinLast funct...
Posted by dlaboss <gi...@git.apache.org>.
Github user dlaboss commented on a diff in the pull request:
https://github.com/apache/incubator-quarks/pull/60#discussion_r58528032
--- Diff: api/window/src/main/java/quarks/window/Window.java ---
@@ -157,4 +158,10 @@ Licensed to the Apache Software Foundation (ASF) under one
*/
Function<T, K> getKeyFunction();
+ /**
+ * Retrieves the partitions in the window.
+ * @return A map of the window's keys and partitions.
--- End diff --
Maybe you've also made adjustments but on lines 188 & 216 below in ConnectorStream, it's not synchronizing the partitions map when getting the partition of interest. Q: would using/returning a synchronized map be better?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-quarks pull request: [WIP] Added join and joinLast funct...
Posted by ddebrunner <gi...@git.apache.org>.
Github user ddebrunner commented on a diff in the pull request:
https://github.com/apache/incubator-quarks/pull/60#discussion_r58105482
--- Diff: spi/topology/src/main/java/quarks/topology/spi/graph/ConnectorStream.java ---
@@ -163,6 +175,100 @@ protected Graph graph() {
}
@Override
+ public <J, U, K> TStream<J> join(Function<T, K> keyer,
+ TWindow<U, K> twindow, BiFunction<T, List<U>, J> joiner) {
+
+ if(twindow instanceof TWindowImpl){
+ TStream<U> lastStream = twindow.feeder();
+ BiFunction<List<U>,K, Object> processor = Functions.synchronizedBiFunction((list, key) -> null);
+ Window<U, K, LinkedList<U>> window = Windows.lastNProcessOnInsert(((TWindowImpl<U, K>)twindow).getSize(), twindow.getKeyFunction());
+ Aggregate<U,Object,K> op = new Aggregate<U,Object,K>(window, processor);
+ lastStream.pipe(op);
+ return this.map((tuple) -> {
+ Partition<U, K, ? extends List<U>> part = window.getPartitions().get(keyer.apply(tuple));
+ if(part == null)
+ return null;
+ J ret;
+ synchronized (part) {
+ List<U> last = part.getContents();
+ ret = joiner.apply(tuple, last);
+ }
+ return ret;
+ });
+ }
+
+ else if (twindow instanceof TWindowTimeImpl){
+ TStream<U> lastStream = twindow.feeder();
+ BiFunction<List<U>,K, Object> processor = Functions.synchronizedBiFunction((list, key) -> null);
+ long time = ((TWindowTimeImpl<U, K>)(twindow)).getTime();
+ TimeUnit unit = ((TWindowTimeImpl<U, K>)(twindow)).getUnit();
+ Window<U, K, InsertionTimeList<U>> window =
+ Windows.window(
+ alwaysInsert(),
+ scheduleEvictIfEmpty(time, unit),
+ evictOlderWithProcess(time, unit),
+ processOnInsert(),
+ twindow.getKeyFunction(),
+ insertionTimeList());
+ Aggregate<U,Object,K> op = new Aggregate<U,Object,K>(window, processor);
--- End diff --
Can some explanation be added, it's unclear why an Aggregate is used that is unconnected.
I guess i was expecting a new Join oplet to be created & used.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---