[FLINK-8941][network][serializer] improve SpanningRecordSerializationTest and ensure unique spilling files #5709
Conversation
…ordSerializationTest
Although the spilling files were chosen with random names of 20 bytes, it could rarely happen that these collide. In that case, have another try (at most 10) at selecting a unique file name.
There was a problem hiding this comment.
Collision on 20 length bytes? 255^20? Please replace
it could rarely happen
in the commit massage with more accurate statement:
it could never happen in this universe
Besides I do not see how any of those two commits could fix the reported problem :(
|
I agree with @pnowojski. To me it seems quite unlikely that |
|
I'd actually like to keep this collision detection since it does not hurt although it may not fix the problem. Both commits are actually somewhat shots in the dark trying to tackle the original problem which I could not really explain otherwise. |
|
Alright. Thanks for your contribution @NicoK and the review @pnowojski. Merging this PR. |
Although the spilling files were chosen with random names of 20 bytes, it could rarely happen that these collide. In that case, have another try (at most 10) at selecting a unique file name. This closes apache#5709.
Although the spilling files were chosen with random names of 20 bytes, it could rarely happen that these collide. In that case, have another try (at most 10) at selecting a unique file name. This closes #5709.
Although the spilling files were chosen with random names of 20 bytes, it could rarely happen that these collide. In that case, have another try (at most 10) at selecting a unique file name. This closes apache#5709.
4 participants
What is the purpose of the change
This PR contains two commits trying to tackle FLINK-8941 (which I could not reproduce).
Brief change log
SpanningRecordSerializationTestextend fromTestLoggerTemporaryFolderinSpanningRecordSerializationTestfor spilling filesSpillingAdaptiveSpanningRecordDeserializerdoes not work on an existing file, e.g. from another instance running on the same machine - allow 10 retries with 20 random bytes file names and fail otherwiseVerifying this change
This change is already covered by existing tests, such as
SpanningRecordSerializationTest.Does this pull request potentially affect one of the following parts:
@Public(Evolving): noDocumentation