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.

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

[FLINK-1505] Separate reader API from result consumption #428

Closed
wants to merge 1 commit into from

Conversation

uce
Copy link
Contributor

@uce uce commented Feb 20, 2015

@gyfora, can you please rebase on this branch and verify that everything is still working as expected for you?

This PR separates the reader API (record and buffer readers) from result consumption (input gate). The buffer reader was a huge component with mixed responsibilities both as the runtime component to set up input channels for intermediate result consumption and as a lower-level user API to consume buffers/events.

The separation makes it easier for users of the API (e.g. flink-streaming) to extend the handling of low-level buffers and events. Gyula's initial feedback confirmed this.

In view of FLINK-1568, this PR makes it also easier to test the result consumption logic for failure scenarios.

I will rebase #356 on this branch.

@gyfora
Copy link
Contributor

gyfora commented Feb 20, 2015

Thank you!
I will try it over the weekend and give you feedback.

@gyfora
Copy link
Contributor

gyfora commented Feb 20, 2015

We have rebased and tried, it works fine for us, thank you. I will add several minor changes afterwards but thats just for our convenience.

So from my side
+1

@senorcarbone
Copy link
Contributor

+1 from me as well, works fine! Nice decoupling :)

@uce
Copy link
Contributor Author

uce commented Feb 23, 2015

Ok, thanks. I will merge this then.

@asfgit asfgit closed this in 5232c56 Feb 23, 2015
@uce uce deleted the flink-1505-input_gate branch February 25, 2015 10:19
marthavk pushed a commit to marthavk/flink that referenced this pull request Jun 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants