You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/03/11 14:04:23 UTC
[GitHub] [spark] Yikun opened a new pull request #35820: [SPARK-38525][K8S][TESTS] Check resource after yaml resource creation
Yikun opened a new pull request #35820:
URL: https://github.com/apache/spark/pull/35820
### What changes were proposed in this pull request?
Check resource after yaml resource creation to make sure resource exsiting.
### Why are the changes needed?
The yaml create is async, we'd better to make sure the resource can be got from server.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
```
[info] VolcanoSuite:
[info] - Run SparkPi with volcano scheduler (11 seconds, 468 milliseconds)
[info] - SPARK-38187: Run SparkPi Jobs with minCPU (30 seconds, 636 milliseconds)
[info] - SPARK-38187: Run SparkPi Jobs with minMemory (31 seconds, 544 milliseconds)
[info] - SPARK-38188: Run SparkPi jobs with 2 queues (only 1 enabled) (13 seconds, 333 milliseconds)
[info] - SPARK-38188: Run SparkPi jobs with 2 queues (all enabled) (25 seconds, 486 milliseconds)
[info] - SPARK-38423: Run driver job to validate priority order (16 seconds, 488 milliseconds)
[info] Run completed in 2 minutes, 14 seconds.
[info] Total number of tests run: 6
[info] Suites: completed 2, aborted 0
[info] Tests: succeeded 6, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.
[success] Total time: 163 s (02:43), completed 2022-3-11 21:40:53
```
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on pull request #35820: [SPARK-38525][K8S][TESTS] Check resource after yaml resource creation
Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #35820:
URL: https://github.com/apache/spark/pull/35820#issuecomment-1065200434
The test passed also based on https://github.com/apache/spark/pull/35819
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun edited a comment on pull request #35820: [SPARK-38525][K8S][TESTS] Check resource after yaml resource creation
Posted by GitBox <gi...@apache.org>.
Yikun edited a comment on pull request #35820:
URL: https://github.com/apache/spark/pull/35820#issuecomment-1065838723
> `deleteYAMLResource` is also async and we don't do double-check.
deleteYAMLResource is [sync operation](https://github.com/apache/spark/blob/c91c2e9afec0d5d5bbbd2e155057fe409c5bb928/resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/VolcanoTestsSuite.scala#L66-L70), I think it is need, to make sure keep the independence of each test.
> In addition, what is the behavior of Volcano, for the pod group submission with non-existing queue name?
Just like native sheduler, the driver will be pending status.
> If Volcano hangs, we don't need this patch at all.
Yes, this patch is not only for `queue` (such as we also create priorities resource) or make test passed, It's just for more protective in my view.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun edited a comment on pull request #35820: [SPARK-38525][K8S][TESTS] Check resource after yaml resource creation
Posted by GitBox <gi...@apache.org>.
Yikun edited a comment on pull request #35820:
URL: https://github.com/apache/spark/pull/35820#issuecomment-1065838723
> `deleteYAMLResource` is also async and we don't do double-check.
`deleteYAMLResource` is [sync operation](https://github.com/apache/spark/blob/c91c2e9afec0d5d5bbbd2e155057fe409c5bb928/resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/VolcanoTestsSuite.scala#L66-L70), I think it is need, to make sure keep the independence of each test. Just like [other test resource cleanup](https://github.com/apache/spark/blob/c91c2e9afec0d5d5bbbd2e155057fe409c5bb928/resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/KubernetesSuite.scala#L585) in K8S IT.
> In addition, what is the behavior of Volcano, for the pod group submission with non-existing queue name?
Just like native sheduler, the driver will be pending status.
> If Volcano hangs, we don't need this patch at all.
Yes, without this patch, all tests are also passed as before. This patch is not only for `queue` (such as we also create priorities resource), It's just for more protective in my view. so, I will +1 for your choice, merge or not merge is both fine for me.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun edited a comment on pull request #35820: [SPARK-38525][K8S][TESTS] Check resource after yaml resource creation
Posted by GitBox <gi...@apache.org>.
Yikun edited a comment on pull request #35820:
URL: https://github.com/apache/spark/pull/35820#issuecomment-1065838723
> `deleteYAMLResource` is also async and we don't do double-check.
`deleteYAMLResource` is [sync operation](https://github.com/apache/spark/blob/c91c2e9afec0d5d5bbbd2e155057fe409c5bb928/resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/VolcanoTestsSuite.scala#L66-L70), I think it is need, to make sure keep the independence of each test. Just like [other test resource cleanup](https://github.com/apache/spark/blob/c91c2e9afec0d5d5bbbd2e155057fe409c5bb928/resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/KubernetesSuite.scala#L585).
> In addition, what is the behavior of Volcano, for the pod group submission with non-existing queue name?
Just like native sheduler, the driver will be pending status.
> If Volcano hangs, we don't need this patch at all.
Yes, this patch is not only for `queue` (such as we also create priorities resource) or make test passed, It's just for more protective in my view. so,
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun edited a comment on pull request #35820: [SPARK-38525][K8S][TESTS] Check resource after yaml resource creation
Posted by GitBox <gi...@apache.org>.
Yikun edited a comment on pull request #35820:
URL: https://github.com/apache/spark/pull/35820#issuecomment-1065838723
> `deleteYAMLResource` is also async and we don't do double-check.
`deleteYAMLResource` is [sync operation](https://github.com/apache/spark/blob/c91c2e9afec0d5d5bbbd2e155057fe409c5bb928/resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/VolcanoTestsSuite.scala#L66-L70), I think it is need, to make sure keep the independence of each test. Just like [other test resource cleanup](https://github.com/apache/spark/blob/c91c2e9afec0d5d5bbbd2e155057fe409c5bb928/resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/KubernetesSuite.scala#L585) in K8S IT.
> In addition, what is the behavior of Volcano, for the pod group submission with non-existing queue name?
Just like native sheduler, the driver will be pending status.
> If Volcano hangs, we don't need this patch at all.
Yes, this patch is not only for `queue` (such as we also create priorities resource) or make test passed, It's just for more protective in my view. so,
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun closed pull request #35820: [SPARK-38525][K8S][TESTS] Check resource after yaml resource creation
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #35820:
URL: https://github.com/apache/spark/pull/35820
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun edited a comment on pull request #35820: [SPARK-38525][K8S][TESTS] Check resource after yaml resource creation
Posted by GitBox <gi...@apache.org>.
Yikun edited a comment on pull request #35820:
URL: https://github.com/apache/spark/pull/35820#issuecomment-1065838723
> `deleteYAMLResource` is also async and we don't do double-check.
`deleteYAMLResource` is [sync operation](https://github.com/apache/spark/blob/c91c2e9afec0d5d5bbbd2e155057fe409c5bb928/resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/VolcanoTestsSuite.scala#L66-L70), I think it is need, to make sure keep the independence of each test. Just like [other test resource cleanup](https://github.com/apache/spark/blob/c91c2e9afec0d5d5bbbd2e155057fe409c5bb928/resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/KubernetesSuite.scala#L585) in K8S IT.
> In addition, what is the behavior of Volcano, for the pod group submission with non-existing queue name?
Just like native sheduler, the driver will be pending status.
> If Volcano hangs, we don't need this patch at all.
Yes, this patch is not only for `queue` (such as we also create priorities resource), It's just for more protective in my view. so,
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun edited a comment on pull request #35820: [SPARK-38525][K8S][TESTS] Check resource after yaml resource creation
Posted by GitBox <gi...@apache.org>.
Yikun edited a comment on pull request #35820:
URL: https://github.com/apache/spark/pull/35820#issuecomment-1065838723
> `deleteYAMLResource` is also async and we don't do double-check.
`deleteYAMLResource` is [sync operation](https://github.com/apache/spark/blob/c91c2e9afec0d5d5bbbd2e155057fe409c5bb928/resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/VolcanoTestsSuite.scala#L66-L70), I think it is need, to make sure keep the independence of each test. Just like [other test resource cleanup](https://github.com/apache/spark/blob/c91c2e9afec0d5d5bbbd2e155057fe409c5bb928/resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/KubernetesSuite.scala#L585) in K8S IT.
> In addition, what is the behavior of Volcano, for the pod group submission with non-existing queue name?
Just like native sheduler, the driver will be pending status.
> If Volcano hangs, we don't need this patch at all.
Yes, without this patch, all tests are also passed as before. This patch is not only for `queue` (such as we also create priorities resource), It's just for more protective in my view. so, I will give +1 for your choice, merge or not merge is both fine for me. : )
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun edited a comment on pull request #35820: [SPARK-38525][K8S][TESTS] Check resource after yaml resource creation
Posted by GitBox <gi...@apache.org>.
Yikun edited a comment on pull request #35820:
URL: https://github.com/apache/spark/pull/35820#issuecomment-1065838723
> `deleteYAMLResource` is also async and we don't do double-check.
`deleteYAMLResource` is [sync operation](https://github.com/apache/spark/blob/c91c2e9afec0d5d5bbbd2e155057fe409c5bb928/resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/VolcanoTestsSuite.scala#L66-L70), I think it is need, to make sure keep the independence of each test. Just like [other test resource cleanup](https://github.com/apache/spark/blob/c91c2e9afec0d5d5bbbd2e155057fe409c5bb928/resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/KubernetesSuite.scala#L585) in K8S IT.
> In addition, what is the behavior of Volcano, for the pod group submission with non-existing queue name?
Just like native sheduler, the driver will be pending status.
> If Volcano hangs, we don't need this patch at all.
Yes, without this patch, all tests are also passed as before. This patch is not only for `queue` (such as we also create priorities resource), It's just for more protective in my view. so, I will +1 for your choice, merge or not merge is fine for me.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #35820: [SPARK-38525][K8S][TESTS] Check resource after yaml resource creation
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35820:
URL: https://github.com/apache/spark/pull/35820#issuecomment-1065865732
Thank you. In that case, let me close this because pending is enough. Sorry for closing.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun edited a comment on pull request #35820: [SPARK-38525][K8S][TESTS] Check resource after yaml resource creation
Posted by GitBox <gi...@apache.org>.
Yikun edited a comment on pull request #35820:
URL: https://github.com/apache/spark/pull/35820#issuecomment-1065838723
> `deleteYAMLResource` is also async and we don't do double-check.
`deleteYAMLResource` is [sync operation](https://github.com/apache/spark/blob/c91c2e9afec0d5d5bbbd2e155057fe409c5bb928/resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/VolcanoTestsSuite.scala#L66-L70), I think it is need, to make sure keep the independence of each test. Just like [other test resource cleanup](https://github.com/apache/spark/blob/c91c2e9afec0d5d5bbbd2e155057fe409c5bb928/resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/KubernetesSuite.scala#L585) in K8S IT.
> In addition, what is the behavior of Volcano, for the pod group submission with non-existing queue name?
Just like native sheduler, the driver will be pending status.
> If Volcano hangs, we don't need this patch at all.
Yes, without this patch, all tests are also passed as before. This patch is not only for `queue` (such as we also create priorities resource), It's just for more protective in my view. so, I will +1 for your choice, merge or not merge is both fine for me. : )
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Yikun commented on pull request #35820: [SPARK-38525][K8S][TESTS] Check resource after yaml resource creation
Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #35820:
URL: https://github.com/apache/spark/pull/35820#issuecomment-1065838723
> `deleteYAMLResource` is also async and we don't do double-check.
deleteYAMLResource is [sync operation](https://github.com/apache/spark/blob/master/resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/VolcanoTestsSuite.scala#L63), I think it is need, to make sure keep the independence of each test.
> In addition, what is the behavior of Volcano, for the pod group submission with non-existing queue name?
Just like native sheduler, the driver will be pending status.
> If Volcano hangs, we don't need this patch at all.
Yes, this patch is not only for `queue` (such as we also create priorities resource) or make test passed, It's just for more protective in my view.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org