Uploaded image for project: 'Flink'
  1. Flink
  2. FLINK-5817

Fix test concurrent execution failure by test dir conflicts.

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.3.0, 1.2.1
    • Component/s: None
    • Labels:
      None

      Description

      Currently when different users build flink on the same machine, failure may happen because some test utilities create test file using the fixed name, which will cause file access failing when different user processing the same file at the same time.
      We have found errors from AbstractTestBase, IOManagerTest, FileCacheTest.

        Issue Links

          Activity

          Hide
          StephanEwen Stephan Ewen added a comment -

          I would encourage you to use the JUnit tool to create random temporary folders:

          @Rule
          public final TemporaryFolder tmp = new TemporaryFolder();
          

          Have a look at the org.apache.flink.util.FileUtilsTest for an example.

          Show
          StephanEwen Stephan Ewen added a comment - I would encourage you to use the JUnit tool to create random temporary folders: @Rule public final TemporaryFolder tmp = new TemporaryFolder(); Have a look at the org.apache.flink.util.FileUtilsTest for an example.
          Hide
          wenlong.lwl Wenlong Lyu added a comment -

          ok, It seems good, I will try it instead of adding a random suffix.

          Show
          wenlong.lwl Wenlong Lyu added a comment - ok, It seems good, I will try it instead of adding a random suffix.
          Hide
          StephanEwen Stephan Ewen added a comment -

          There is also a pull request that tries to change the tmp directory for tests: https://github.com/apache/flink/pull/3190

          Show
          StephanEwen Stephan Ewen added a comment - There is also a pull request that tries to change the tmp directory for tests: https://github.com/apache/flink/pull/3190
          Hide
          shijinkui shijinkui added a comment -

          Stephan Ewen Wenlong Lyu, there no overlapping between FLINK-5546 and FLINK-5817. In FLINK-5546, I'm focus on changing the default java.io.tmp system property.
          I think this issue can find out all the temporary directory creating by new File, and replace all with TemporaryFolder following Stephan's suggestion.
          You can find out all the Test. files and search the keyword "new File(", then you'll find there so much bad smell need to be re-corrected.

          Show
          shijinkui shijinkui added a comment - Stephan Ewen Wenlong Lyu , there no overlapping between FLINK-5546 and FLINK-5817 . In FLINK-5546 , I'm focus on changing the default java.io.tmp system property. I think this issue can find out all the temporary directory creating by new File, and replace all with TemporaryFolder following Stephan's suggestion. You can find out all the Test. files and search the keyword "new File(", then you'll find there so much bad smell need to be re-corrected.
          Hide
          wenlong.lwl Wenlong Lyu added a comment -

          shijinkui I think the reason why you want to change the java.io.tmp property is to solve the ut problem. But I think the best way is to avoid UTs depends on the system property, which will make building more clear. And when we make it, where is no need to set java.io.tmp by pom any more.

          Show
          wenlong.lwl Wenlong Lyu added a comment - shijinkui I think the reason why you want to change the java.io.tmp property is to solve the ut problem. But I think the best way is to avoid UTs depends on the system property, which will make building more clear. And when we make it, where is no need to set java.io.tmp by pom any more.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user wenlong88 opened a pull request:

          https://github.com/apache/flink/pull/3341

          FLINK-5817Fix test concurrent execution failure by test dir conflicts.

          use TemporaryFold to create temp files and folds for UT

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/wenlong88/flink jira-5817-2

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/flink/pull/3341.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #3341



          Show
          githubbot ASF GitHub Bot added a comment - GitHub user wenlong88 opened a pull request: https://github.com/apache/flink/pull/3341 FLINK-5817 Fix test concurrent execution failure by test dir conflicts. use TemporaryFold to create temp files and folds for UT You can merge this pull request into a Git repository by running: $ git pull https://github.com/wenlong88/flink jira-5817-2 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/3341.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #3341
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user NicoK commented on the issue:

          https://github.com/apache/flink/pull/3341

          Thanks, this looks like a really nice addition and simplifies the code a lot.

          Show
          githubbot ASF GitHub Bot added a comment - Github user NicoK commented on the issue: https://github.com/apache/flink/pull/3341 Thanks, this looks like a really nice addition and simplifies the code a lot.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user StephanEwen commented on the issue:

          https://github.com/apache/flink/pull/3341

          Good change, thank you. merging this...

          Show
          githubbot ASF GitHub Bot added a comment - Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/3341 Good change, thank you. merging this...
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/flink/pull/3341

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/3341
          Hide
          StephanEwen Stephan Ewen added a comment -

          Fixed in

          • 1.2.1 via d1b148cf30ebd10815912bca2f2b94b2b56a2388 and fc5a20a81f71b81850292bba2643ef37acfb6c2f
          • 1.3.0 via 709fa1d95b7dbbcfdd1124de7d6e073834ca75cf and 1456f0a7084f45056ea9b09e3f85b1aae6b11c6e
          Show
          StephanEwen Stephan Ewen added a comment - Fixed in 1.2.1 via d1b148cf30ebd10815912bca2f2b94b2b56a2388 and fc5a20a81f71b81850292bba2643ef37acfb6c2f 1.3.0 via 709fa1d95b7dbbcfdd1124de7d6e073834ca75cf and 1456f0a7084f45056ea9b09e3f85b1aae6b11c6e

            People

            • Assignee:
              wenlong.lwl Wenlong Lyu
              Reporter:
              wenlong.lwl Wenlong Lyu
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development