Skip to content

Conversation

@StefanRRichter
Copy link
Contributor

…TCase#testMapState

What is the purpose of the change

This PR fixes a potential NPE in AbstractQueryableStateTestBase#testMapState by extending the condition to retry the query under test.

Copy link
Contributor

@kl0u kl0u left a comment

Choose a reason for hiding this comment

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

Thanks for the work @StefanRRichter ! This definitely avoids the NPE so +1 to merge, but given that we do future.get(deadline.timeLeft().toMillis(), TimeUnit.MILLISECONDS), the NPE will become an assertion error because we did not manage to get the value within the timeout, right? This is because of the while (deadline.hasTimeLeft() && !success) {...}.

@StefanRRichter
Copy link
Contributor Author

Thanks for the review @kl0u. I think the NPE can be caused by some consistency/visibility problem that I think can happen in queryable state, so giving it a retry will probably yield a successful result eventually. Merging.

@asfgit asfgit closed this in b9351fc Nov 5, 2018
asfgit pushed a commit that referenced this pull request Nov 5, 2018
asfgit pushed a commit that referenced this pull request Nov 5, 2018
JTaky pushed a commit to JTaky/flink that referenced this pull request Dec 26, 2018
ashangit pushed a commit to criteo-forks/flink that referenced this pull request Dec 31, 2018
tisonkun pushed a commit to tisonkun/flink that referenced this pull request Jan 17, 2019
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.

3 participants