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-4960] Allow the AbstractStreamOperatorTestHarness to test scaling down #2747

Closed
wants to merge 2 commits into from

Conversation

kl0u
Copy link
Contributor

@kl0u kl0u commented Nov 3, 2016

As the title describes, this PR just adds a method in the AbstractStreamOperatorTestHarness that allows multiple partial operator states takes with the snapshot() method, to be combined into a single one. This, combined with the already existing functionality, will allow testing arbitrary rescaling scenarios.

@kl0u
Copy link
Contributor Author

kl0u commented Nov 3, 2016

R: @aljoscha

Copy link
Contributor

@aljoscha aljoscha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general I like this PR a lot! 👍

I had some comments inline that should be addressed, but then this LGTM.

* on different instances of {@link AbstractStreamOperatorTestHarness} (each one representing one subtask)
* and repacks them into a single {@link OperatorStateHandles} so that the parallelism of the test
* can change arbitrarily (i.e. be able to scale both up and down).
* <p/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A new paragraph should start with <p>. <p/> should never be used in Javadoc, AFAIK.

Collection<KeyGroupsStateHandle> rawKeyedState = handle.getRawKeyedState();


if ((managedOperatorState != null && managedOperatorState.size() > 1) ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this restriction necessary? I think it would also work if there are several entries in the list and we just add them all together.

@kl0u
Copy link
Contributor Author

kl0u commented Nov 3, 2016

@aljoscha thanks for the comments. I integrated them.

@aljoscha
Copy link
Contributor

aljoscha commented Nov 4, 2016

I merged this. Thanks for your work! 😃

Could you please close this and the Jira issue?

@kl0u
Copy link
Contributor Author

kl0u commented Nov 4, 2016

Thanks @aljoscha . Will do that.

@kl0u kl0u closed this Nov 4, 2016
@kl0u kl0u deleted the scaling_down_tests branch March 15, 2017 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants