[GitHub] [flink] wzhero1 commented on issue #9336: [FLINK-13548][Deployment/YARN]Support priority of the Flink YARN application
wzhero1 commented on issue #9336: [FLINK-13548][Deployment/YARN]Support priority of the Flink YARN application URL: https://github.com/apache/flink/pull/9336#issuecomment-526441235 Thanks for the supplementary test of YARN priority @tillrohrmann . I think it looks good to merge. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [flink] wzhero1 commented on issue #9336: [FLINK-13548][Deployment/YARN]Support priority of the Flink YARN application
wzhero1 commented on issue #9336: [FLINK-13548][Deployment/YARN]Support priority of the Flink YARN application URL: https://github.com/apache/flink/pull/9336#issuecomment-526078399 Besides, it seems that using mocks to test `AbstractYarnClusterDescriptor#startAppMaster()` is very difficult. That method relies on too many dependencies which cannot be mocked properly, e.g. `setupSingleLocalResource()` relies on a local jar and copies it to HDFS. Could we just test the configuration parse in this commit (like other existing test cases), and refactor the `AbstractYarnClusterDescriptor#startAppMaster()` in another commit? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [flink] wzhero1 commented on issue #9336: [FLINK-13548][Deployment/YARN]Support priority of the Flink YARN application
wzhero1 commented on issue #9336: [FLINK-13548][Deployment/YARN]Support priority of the Flink YARN application URL: https://github.com/apache/flink/pull/9336#issuecomment-526014631 @tillrohrmann @walterddr Thank you very much for the discussion~ I think maybe there are two places we should test. 1. the configuration we configured in Cli could be correctly parsed into YARN dynamic configuration. I found in `YARNSessionCapacitySchedulerITCase#testVCoresAreSet...()`, the method would getFlinkConfig by quering the yarn cluster. I added a priority config in this method, and it could correctly be parsed, which means the flinkConfiguration has the property when `AbstractYarnClusterDescriptor#startAppMaster()`, so we can infer the property can be well configured in the `ApplicationSubmissionContext`(and debugging can confirm this Inference ). Also I found `FlinkYarnSessionCliTest#testConfigurationBeforeDeployment()` I added before is redundant so I have deleted this test. https://github.com/apache/flink/blob/17e2747e575cc4d9847be76b5ce9a75a7b6707f7/flink-yarn-tests/src/test/java/org/apache/flink/yarn/YARNSessionCapacitySchedulerITCase.java#L254 2. after sumbitting the application, we should get the priority report from `ApplicationReport` to check whether the priority config takes effect. `YARNSessionCapacitySchedulerITCase#testVCoresAreSet...()` also provides the way to check the `ApplicationReport`. But in hadoop-2.4.1 api, the `ApplicationReport` does not provide the `getPriority()` method(the method is supported after 2.8.0). So in current flink(hadoop version:2.4.1), we could not to test the `ApplicationReport`. Since currently `ApplicationSubmissionContext` is wrapped in the `AbstractYarnClusterDescriptor#startAppMaster` and hard to get and test, so I think maybe it is proper to test as I talked above. I sumbitted a new commit as I said, looking forward to your better ideas, thanks~ This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [flink] wzhero1 commented on issue #9336: [FLINK-13548][Deployment/YARN]Support priority of the Flink YARN application
wzhero1 commented on issue #9336: [FLINK-13548][Deployment/YARN]Support priority of the Flink YARN application URL: https://github.com/apache/flink/pull/9336#issuecomment-522445398 @walterddr Thanks for the test about ApplicationReport~ I have updated the doc and added a test in `FlinkYarnSessionCliTest` to check whether the command line is correctly converted into flinkConfiguration before deployment. Also I think the ApplicationReport test may be integrated into Flink later after the hadoop version is upgraded after 2.8.x. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [flink] wzhero1 commented on issue #9336: [FLINK-13548][Deployment/YARN]Support priority of the Flink YARN application
wzhero1 commented on issue #9336: [FLINK-13548][Deployment/YARN]Support priority of the Flink YARN application URL: https://github.com/apache/flink/pull/9336#issuecomment-522332605 @walterddr Thanks for your detailed research. I will change the doc and add a test in `FlinkYarnSessionCliTest` to test `setPriority` operation is valid. And after that we can submit a version to provide this yarn config parameter first. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [flink] wzhero1 commented on issue #9336: [FLINK-13548][Deployment/YARN]Support priority of the Flink YARN application
wzhero1 commented on issue #9336: [FLINK-13548][Deployment/YARN]Support priority of the Flink YARN application URL: https://github.com/apache/flink/pull/9336#issuecomment-521954354 ok, I will consider to add a test in flink-yarn-tests, maybe in YarnConfigurationITCase. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [flink] wzhero1 commented on issue #9336: [FLINK-13548][Deployment/YARN]Support priority of the Flink YARN application
wzhero1 commented on issue #9336: [FLINK-13548][Deployment/YARN]Support priority of the Flink YARN application URL: https://github.com/apache/flink/pull/9336#issuecomment-521953939 ok, I will consider to add a test in flink-yarn-tests, maybe in YarnConfigurationITCase. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [flink] wzhero1 commented on issue #9336: [FLINK-13548][Deployment/YARN]Support priority of the Flink YARN application
wzhero1 commented on issue #9336: [FLINK-13548][Deployment/YARN]Support priority of the Flink YARN application URL: https://github.com/apache/flink/pull/9336#issuecomment-519463581 @walterddr I found CI failed by timeout problem, how could I rebuild the CI~ This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [flink] wzhero1 commented on issue #9336: [FLINK-13548][Deployment/YARN]Support priority of the Flink YARN application
wzhero1 commented on issue #9336: [FLINK-13548][Deployment/YARN]Support priority of the Flink YARN application URL: https://github.com/apache/flink/pull/9336#issuecomment-519360009 @walterddr Thanks for your review, I have modified YARN config description according to the comments. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
