Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ReduceStateByKey state combiner function shouldn't use CombinableReduceFunction #51

Closed
vanekjar opened this issue Mar 20, 2017 · 0 comments
Closed

ReduceStateByKey state combiner function shouldn't use CombinableReduceFunction #51

vanekjar opened this issue Mar 20, 2017 · 0 comments
Assignees
@xitep
Labels
client-api

Comments

@vanekjar
Copy link
Contributor

Currently the state combiner is type of CombinableReduceFunction

public State apply(Iterable<State> reducedStates)

It turned out runtime needs to choose which of given states will be the final result merged into. That means the function can't be commutative.

Proposed solution is to change the signature of the state combiner to something like

public void merge(State targetState, Iterable<State> mergedStates)

This task involves change of client API as well as modification of all executors relying on this API.
@vanekjar vanekjar mentioned this issue Mar 20, 2017
Merged
@xitep xitep self-assigned this Apr 3, 2017
xitep pushed a commit that referenced this issue Apr 4, 2017
#51 Move StateFactory to the state package
b7008a3
xitep pushed a commit that referenced this issue Apr 4, 2017
#51 StateFactory interface revised
9f39cdc
xitep pushed a commit that referenced this issue Apr 4, 2017
#51 Do _not_ store storage-provider in states by default
b607bf8
xitep pushed a commit that referenced this issue Apr 4, 2017
#51 Move StateFactory to the state package
4d7d1e2
xitep pushed a commit that referenced this issue Apr 4, 2017
#51 StateFactory interface revised
0cc8642
xitep pushed a commit that referenced this issue Apr 4, 2017
#51 Replace StateCombiner with StateMerger
3cba482
@xitep xitep mentioned this issue Apr 4, 2017
Merged
xitep pushed a commit that referenced this issue Apr 4, 2017
#51 Drop obsolete TODO
e237cf5
xitep pushed a commit that referenced this issue Apr 6, 2017
#51 Do _not_ store storage-provider in states by default
d62286c
xitep pushed a commit that referenced this issue Apr 6, 2017
#51 Move StateFactory to the state package
07b9fc6
xitep pushed a commit that referenced this issue Apr 6, 2017
#51 StateFactory interface revised
525b874
xitep pushed a commit that referenced this issue Apr 6, 2017
#51 Replace StateCombiner with StateMerger
38c0579
xitep pushed a commit that referenced this issue Apr 6, 2017
#51 Drop obsolete TODO
e3efefe
xitep pushed a commit that referenced this issue Apr 8, 2017
#51 Do _not_ store storage-provider in states by default
4b8c51c
xitep pushed a commit that referenced this issue Apr 8, 2017
#51 Move StateFactory to the state package
80e1149
xitep pushed a commit that referenced this issue Apr 8, 2017
#51 StateFactory interface revised
a5024eb
xitep pushed a commit that referenced this issue Apr 8, 2017
#51 Replace StateCombiner with StateMerger
8747451
xitep pushed a commit that referenced this issue Apr 8, 2017
#51 Drop obsolete TODO
ecc5290
xitep pushed a commit that referenced this issue Apr 8, 2017
#51 [euphoria-core] Skip processing empty window merge sets
8919eb0
@xitep xitep closed this as completed in #71 Apr 8, 2017
xitep added a commit that referenced this issue Apr 8, 2017
@xitep
Merge pull request #71 from seznam/pete/state-combiner
d509d57 
#51 [euphoria-core] Introduce StateMerger
mareksimunek pushed a commit that referenced this issue May 4, 2018
#51 Do _not_ store storage-provider in states by default
ecd595f
mareksimunek pushed a commit that referenced this issue May 4, 2018
#51 Move StateFactory to the state package
435f08d
mareksimunek pushed a commit that referenced this issue May 4, 2018
#51 StateFactory interface revised
0a4f48d
mareksimunek pushed a commit that referenced this issue May 4, 2018
#51 Replace StateCombiner with StateMerger
2220654
mareksimunek pushed a commit that referenced this issue May 4, 2018
#51 Drop obsolete TODO
e65cb53
mareksimunek pushed a commit that referenced this issue May 4, 2018
#51 [euphoria-core] Skip processing empty window merge sets
60ca092
mareksimunek pushed a commit that referenced this issue May 4, 2018
@xitep
Merge pull request #71 from seznam/pete/state-combiner
0139963 
#51 [euphoria-core] Introduce StateMerger
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@xitep xitep

Labels
client-api
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@vanekjar @xitep