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-1521] Chained operators respect reuse #392

Closed
wants to merge 1 commit into from

Conversation

zentol
Copy link
Contributor

@zentol zentol commented Feb 13, 2015

No description provided.

@StephanEwen
Copy link
Contributor

While this change makes it technically more correct, I think it will have a massive negative impact on performance. Nobody can be expected to turn on the object reuse switch without being a seriously advanced user. But everybody will experience the performance drop.

Since no user ever ran into this problem, I think we should NOT change it at this point.

@fhueske
Copy link
Contributor

fhueske commented Feb 13, 2015

I agree with Stephan to not copy records for chained mappers.
However, we should clearly document the behavior of the different modes or at least the behavior of the default mode.

@gyfora
Copy link
Contributor

gyfora commented Feb 14, 2015

The problem here I think that an error caused by the reusing mapper could be very hard to detect. So some users might have it but they don't realise.

For streaming we copy the input at every chained function call just to be on the safe side, but we'll have to reintroduce the object reuse mode.

@StephanEwen
Copy link
Contributor

At this point, we also have to worry about efficiency and performance.
This is also something that the user can easily mitigate in the UDF, at a fraction of the cost in most cases.

I vote postpone the fix until we get some tangible user feedback concerning this issue.

@fhueske
Copy link
Contributor

fhueske commented Feb 17, 2015

+1
I opened FLINK-1565 for documenting object reuse behavior.

@zentol
Copy link
Contributor Author

zentol commented Feb 19, 2015

Alright, I`m closing this PR then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants