You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by shijinkui <gi...@git.apache.org> on 2017/01/23 07:45:15 UTC
[GitHub] flink pull request #3190: [FLINK-5546][build] When multiple users run test, ...
GitHub user shijinkui opened a pull request:
https://github.com/apache/flink/pull/3190
[FLINK-5546][build] When multiple users run test, /tmp/cacheFile conf\u2026
Unit test will create file in java.io.tmpdir, default it's `/tmp` which has capacity limit.
Create temporary in project target directory is reasonable, delete them as `mvn clean`
- [X] General
- The pull request references the related JIRA issue ("[FLINK-5546] When multiple users run test, /tmp/cacheFile conflicts")
- The pull request addresses only one issue
- Each commit in the PR has a meaningful commit message (including the JIRA id)
- [X] Documentation
- Documentation has been added for new functionality
- Old documentation affected by the pull request has been updated
- JavaDoc for public methods has been added
- [X] Tests & Build
- Functionality added by the pull request is covered by tests
- `mvn clean verify` has been executed successfully locally or a Travis build has passed
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/shijinkui/flink FLINK-5546-tmp
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/3190.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 #3190
----
commit 3ec804a5f1de61f00ac49b1d4ed0ef005123daae
Author: shijinkui <sh...@huawei.com>
Date: 2017-01-23T07:37:44Z
[FLINK-5546][build] When multiple users run test, /tmp/cacheFile conflicts
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3190: [FLINK-5546][build] java.io.tmpdir setted as project buil...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/3190
Here is a related issue: https://issues.apache.org/jira/browse/FLINK-5817
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3190: [FLINK-5546][build] java.io.tmpdir setted as project buil...
Posted by shijinkui <gi...@git.apache.org>.
Github user shijinkui commented on the issue:
https://github.com/apache/flink/pull/3190
> Can we just use the ${project.build.directory} as java.io.tmpdir ?
@wenlong88 Sorry for late reply.
It's good question. If use `${project.build.directory}` without sub directory `tmp`, the UT will create various directories, maybe the directories overlap with other dir, such as `classes`\uff0f`surefire-reports` and so on.
Using a special dir `tmp` can avoid the probability of directory conflict.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3190: [FLINK-5546][build] java.io.tmpdir setted as project buil...
Posted by wenlong88 <gi...@git.apache.org>.
Github user wenlong88 commented on the issue:
https://github.com/apache/flink/pull/3190
Can we just use the `${project.build.directory}` as `java.io.tmpdir` ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3190: [FLINK-5546][build] java.io.tmpdir setted as project buil...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/3190
All right, I am convinced now that this is a helpful change.
Merging this...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3190: [FLINK-5546][build] java.io.tmpdir setted as project buil...
Posted by shijinkui <gi...@git.apache.org>.
Github user shijinkui commented on the issue:
https://github.com/apache/flink/pull/3190
@greghogan That make sense. Let JUnit to create root dir and delete temporary dir recursively.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3190: [FLINK-5546][build] java.io.tmpdir setted as project buil...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/3190
Yes that makes sense.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3190: [FLINK-5546][build] java.io.tmpdir setted as project buil...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/3190
Travis failed on flink-core:
```
Failed tests:
GenericCsvInputFormatTest.readWithEmptyField:639 Test failed due to a IOException: No such file or directory
GenericCsvInputFormatTest.readWithHeaderLine:703 Test failed due to a IOException: No such file or directory
GenericCsvInputFormatTest.readWithHeaderLineAndInvalidIntermediate:740 Test failed due to a IOException: No such file or directory
GenericCsvInputFormatTest.readWithParseQuotedStrings:672 Test failed due to a IOException: No such file or directory
GenericCsvInputFormatTest.testLongLongLong:312 Test erroneous
GenericCsvInputFormatTest.testReadInvalidContents:489 Test failed due to a IOException: No such file or directory
GenericCsvInputFormatTest.testReadInvalidContentsLenient:514 Test failed due to a IOException: No such file or directory
GenericCsvInputFormatTest.testReadInvalidContentsLenientWithSkipping:544 Test failed due to a IOException: No such file or directory
GenericCsvInputFormatTest.testReadNoPosAll:119 Test failed due to a IOException: No such file or directory
GenericCsvInputFormatTest.testReadNoPosAllDeflate:159 Test failed due to a IOException: No such file or directory
GenericCsvInputFormatTest.testReadNoPosAllGzip:199 Test failed due to a IOException: No such file or directory
GenericCsvInputFormatTest.testReadNoPosFirstN:235 Test failed due to a IOException: No such file or directory
GenericCsvInputFormatTest.testReadTooShortInput:434 Test failed due to a IOException: No such file or directory
GenericCsvInputFormatTest.testReadTooShortInputLenient:459 Test failed due to a IOException: No such file or directory
GenericCsvInputFormatTest.testSparseParse:276 Test erroneous
GenericCsvInputFormatTest.testSparseParseWithIndices:351 Test erroneous
GenericCsvInputFormatTest.testSparseParseWithIndicesMultiCharDelimiter:406 Test erroneous
Tests in error:
GenericCsvInputFormatTest.testReadWithCharset:568 » IO No such file or directo...
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3190: [FLINK-5546][build] java.io.tmpdir setted as project buil...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/3190
@shijinkui This still has the problem that tests don't run any more from within the IDE. That is a big problem for developers.
As an alternative, we can always fix the tests that do not use random directories directly. With that approach, we get both concurrent builds and good IDE experience.
Maybe someone else can share an opinion here as well...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3190: [FLINK-5546][build] java.io.tmpdir setted as project buil...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/3190
@shijinkui The FileCacheDeleteValidationTest should still be fixed now. When I tested it, I combined the changed from FLINK-5817 and this pull request.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3190: [FLINK-5546][build] java.io.tmpdir setted as project buil...
Posted by shijinkui <gi...@git.apache.org>.
Github user shijinkui commented on the issue:
https://github.com/apache/flink/pull/3190
> The �${project.build.directory}` is not automatically cleaned up. The/tmp` directory is natural to clean up.
hi, Stephan, thank for your reply.
When restarting os, tmp dir will be clean up automatically. And �${project.build.directory}/tmp` will be clean up when executing `mvn clean`. IMO, `${project.build.directory}` target dir can avoid multiple users building projects to share the same `/tmp` problem thoroughly.
In another word, I think project building's boundary is its target directory. Beyond target dir should be not reasonable. Keep project building independent.
In addition, correct all the temporary creating style with `TemporaryFolder` and add random dir prefix, that will modify more test file. Maybe we can resolve the `TemporaryFolder` problem in another thread.
Thanks
Happy New Year!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3190: [FLINK-5546][build] java.io.tmpdir setted as project buil...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/3190
Hi! `mvn test` works from the command line.
IntelliJ right-click on a test and run does often not work, it only works if a console build was done before and the `.../target/tmp/` directory still exists. If it does not exist, yet, or not any more (because of a `mvn clean`), then all tests fail with "temp directory does not exist". I think this confuses developers a lot, many will not know easily what to do to make the tests work in such a case.
I hope you understand that I am placing importance on the developer experience.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3190: [FLINK-5546][build] java.io.tmpdir setted as project buil...
Posted by shijinkui <gi...@git.apache.org>.
Github user shijinkui commented on the issue:
https://github.com/apache/flink/pull/3190
" /tmp/cacheFile (Permission denied)" exception of unit test can be replay.
1. on linux env, `sudo - userA`, clone the flink code, start to `mvn clean test verify`
2. on the linux machine, `sudo - userB`, clone the flink codebase, same start to `mvn clean test verify`
Because two use share the same `/tmp` directory which have no random sub directory.
IMO, `/tmp` change to `${project.build.directory}`, at same time have random suffix. This can resolve the problem thoroughly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3190: [FLINK-5546][build] java.io.tmpdir setted as project buil...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/3190
Sorry, I have to actually step back on this one.
I merged it into a feature branch and played around a bit with this, and it turns out it is not possible any more to execute tests from within the IDE. That is something we cannot have.
I don't know how to make this seamlessly across CLI builds and manual IDE tests. I think we are back at fixing the individual tests...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3190: [FLINK-5546][build] java.io.tmpdir setted as project buil...
Posted by shijinkui <gi...@git.apache.org>.
Github user shijinkui commented on the issue:
https://github.com/apache/flink/pull/3190
> Here is a related issue: https://issues.apache.org/jira/browse/FLINK-5817
I sounds good. I want to change the default system property `java.io.tmpdir` to be in the `target` directory. This can resolve most of such temporary directory conflict problem.
I only want to quickly fix such problem which block our production unit test, either this PR or FLINK-5817. Now we have to disable such module's unit test to avoid such error.
In FLINK-5817, I think we can replace all the manually temporary directory creating with `TemporaryFolder`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3190: [FLINK-5546][build] java.io.tmpdir setted as project buil...
Posted by shijinkui <gi...@git.apache.org>.
Github user shijinkui commented on the issue:
https://github.com/apache/flink/pull/3190
hi, @StephanEwen I have re-submit this pull request base on current master branch which had merged FLINK-5817.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3190: [FLINK-5546][build] java.io.tmpdir setted as project buil...
Posted by shijinkui <gi...@git.apache.org>.
Github user shijinkui commented on the issue:
https://github.com/apache/flink/pull/3190
@StephanEwen I have use TemporaryFolder to replace creating File manually.
There are some tips:
1. TemporaryFolder should invoke `create()` in setup manually
2. in `shutdown()` should invoke `tmp.delete()` to delete all the dir and files recursively.
3. file name add random string
the generated files like this in root directory `flink/flink-runtime/target/tmp`:
```
tmp/
\u2514\u2500\u2500 junit4955582781992164088
\u251c\u2500\u2500 1fde53fa-7f7c-4f55-921c-04d078601cfd
\u2502�� \u2514\u2500\u2500 flink-dist-cache-664fc7de-51af-4fc1-86a0-3d8c76bb65df
\u251c\u2500\u2500 30326e4a-205f-4982-a920-e790a7f52730
\u2502�� \u2514\u2500\u2500 flink-dist-cache-d3eb183b-ca0c-4b23-877f-adf1540d6563
\u2502�� \u2514\u2500\u2500 b217df314c282e23301ba8f8351fca5c
\u2502�� \u2514\u2500\u2500 cacheFile1e7f5410-2326-4916-8a50-986d49393a7f
\u251c\u2500\u2500 35b19e4e-1515-4f93-813e-d4a40b09baeb
\u2502�� \u2514\u2500\u2500 flink-dist-cache-af593973-73a0-4996-8323-40978a703317
\u251c\u2500\u2500 c754952f-fe8d-465f-9424-1303e734e990
\u2502�� \u2514\u2500\u2500 flink-dist-cache-18f1984b-84ca-4db1-be2c-d2a3bc0a5fe3
\u251c\u2500\u2500 cacheFile1e7f5410-2326-4916-8a50-986d49393a7f
\u2514\u2500\u2500 fde2c7bc-8bb1-43ec-8e12-21510322b2b1
\u2514\u2500\u2500 flink-dist-cache-b34f1e0b-79a7-4cf7-ab3f-7a7a93a04d73
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3190: [FLINK-5546][build] java.io.tmpdir setted as project buil...
Posted by shijinkui <gi...@git.apache.org>.
Github user shijinkui commented on the issue:
https://github.com/apache/flink/pull/3190
Get it. Run single test, having no temp created, it should use the default java.io.tmpdir property.
Let me check that.
In the base test class have a double check about the target
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3190: [FLINK-5546][build] java.io.tmpdir setted as project buil...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/3190
@shijinkui I think there are already good temp file utils in JUnit itself.
Do we need to change anything in the build when we fix the tests?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3190: [FLINK-5546][build] java.io.tmpdir setted as project buil...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/3190
I agree that we should simply fix the tests; if we do that the proposed change here is redundant anyway.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3190: [FLINK-5546][build] java.io.tmpdir setted as project buil...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/3190
Many tests assume that the `tmp` directory exists.
I would actually prefer to fix the tests, rather than re-assignign the temp directory. All tests should use a random subdirectory in the temp directory. It is quite convenient to do via JUnit:
```java
@Rule
public final TemporaryFolder tmp = new TemporaryFolder();
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3190: [FLINK-5546][build] java.io.tmpdir setted as project buil...
Posted by shijinkui <gi...@git.apache.org>.
Github user shijinkui commented on the issue:
https://github.com/apache/flink/pull/3190
> I would actually prefer to fix the tests, rather than re-assignign the temp directory. All tests should use a random subdirectory in the temp directory. It is quite convenient to do via JUnit:
@StephanEwen That make sense. Have tmp file util in flink-test-utils-junit.
@zentol @fhueske added maven-antrun-plugin for pre-create tmp directory.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3190: [FLINK-5546][build] java.io.tmpdir setted as project buil...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/3190
I don't see how this could solve the issue in the JIRA; it describes 2 tests from the same module failing since they use the same directory. This PR doesn't change that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3190: [FLINK-5546][build] java.io.tmpdir setted as project buil...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/3190
The �${project.build.directory}` is not automatically cleaned up. The `/tmp` directory is natural to clean up.
I would still suggest to simply fix the tests that have no random directory...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3190: [FLINK-5546][build] java.io.tmpdir setted as project buil...
Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/3190
It does if two users build and test from different code directories on the same machine.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3190: [FLINK-5546][build] java.io.tmpdir setted as project buil...
Posted by greghogan <gi...@git.apache.org>.
Github user greghogan commented on the issue:
https://github.com/apache/flink/pull/3190
@shijinkui: [delete](http://junit.org/junit4/javadoc/latest/org/junit/rules/TemporaryFolder.html#delete()) is "usually not called directly, since it is automatically applied by the Rule".
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3190: [FLINK-5546][build] java.io.tmpdir setted as project buil...
Posted by shijinkui <gi...@git.apache.org>.
Github user shijinkui commented on the issue:
https://github.com/apache/flink/pull/3190
@StephanEwen The FileCacheDeleteValidationTest had been fixed in FLINK-5817. This PR have rollback it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---