Fix the flaky issue in ParallelInterleaveDatasetOpTest #27805
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jsimsa
requested changes
Apr 13, 2019
Yes, the roundtrip test for |
|
This fix is added to all roundtrip tests via edb84f2. |
jsimsa
requested changes
Apr 13, 2019
| @@ -536,6 +536,8 @@ TEST_P(ParameterizedConcatenateDatasetOpTest, Roundtrip) { | |||
| TF_EXPECT_OK(iterator->Save(serialization_ctx.get(), &writer)); | |||
| TF_EXPECT_OK(writer.Flush()); | |||
| VariantTensorDataReader reader(&data); | |||
| TF_ASSERT_OK(concatenate_dataset->MakeIterator(iterator_ctx.get(), | |||
There was a problem hiding this comment.
Please create a test utility (in the test base) for restoring an iterator that makes sure the iterator is reset and it its docstring explain why it is needed. Then apply this utility everywhere (instead of the current error prone pattern that expect that the test writer understand they need to call MakeIterator before Restore). Thanks.
feihugis
force-pushed
the
Debug_ParallelInterleaveDatasetOpTest
branch
from
7662c18
to
9a86868
Compare
|
Good suggestion! I change it via this commit (9a86868). Could you please have a look at the change? |
jsimsa
previously approved these changes
Apr 14, 2019
rthadur
added
comp:data
jsimsa
approved these changes
Apr 15, 2019
|
The test failure in |
tensorflow-copybara
pushed a commit
that referenced
this pull request
Apr 15, 2019
…etOpTest PiperOrigin-RevId: 243648524
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes the flaky issue in ParallelInterleaveDatasetOpTest. We reset the iterator before restoring it so that all the resources/tasks the old iterator holds can be released.
cc:@jsimsa