You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "yyy1000 (via GitHub)" <gi...@apache.org> on 2023/12/05 16:31:37 UTC
[PR] Spark: Use Awaitility instead of Thread.sleep [iceberg]
yyy1000 opened a new pull request, #9224:
URL: https://github.com/apache/iceberg/pull/9224
From the comment, https://github.com/apache/iceberg/issues/7154#issuecomment-1785003366
There're some places can be replaced with Awaitlity.
Here I only change TestCopyOnWriteDelete.java, because I don't know whether I'm in the correct path.
After reviewing, I can inspect other places.
:)
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Use Awaitility instead of Thread.sleep [iceberg]
Posted by "yyy1000 (via GitHub)" <gi...@apache.org>.
yyy1000 commented on PR #9224:
URL: https://github.com/apache/iceberg/pull/9224#issuecomment-1853153204
Hi, I want to make an update of this PR:
The CI failed indicated that :
```
Caused by:
org.apache.iceberg.exceptions.CommitFailedException: Base metadata location
'file:/tmp/hive436356614419280460/table/metadata/00034-1cf322a5-e28e-4bbe-a4b1-aa2d3dad3d00.metadata.json'
is not same as the current table metadata location 'file:/tmp/hive436356614419280460/table/metadata/00035-ed8bc810-ddbf-4b41-b38d-9bb2efdbdc4a.metadata.json' for default.table
```
However, if I fall back to the old version using 'sleep()', it still shows the same error after `WARN Tasks: Retrying task after failure: Base metadata location is not the same`.
So I'm thinking the error is not not because I change 'sleep' to 'await'
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Use Awaitility instead of Thread.sleep [iceberg]
Posted by "yyy1000 (via GitHub)" <gi...@apache.org>.
yyy1000 commented on PR #9224:
URL: https://github.com/apache/iceberg/pull/9224#issuecomment-2273799426
> @yyy1000 sorry I completely missed that this PR exists. Could you please rebase and fix the merge conflicts? It would be great to get this in. Also Let's first only focus on Spark 3.5 changes. We can open a separate PR later to backport to changes to earlier Spark versions
@nastra Cool, could do it next few days!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Use Awaitility instead of Thread.sleep [iceberg]
Posted by "yyy1000 (via GitHub)" <gi...@apache.org>.
yyy1000 commented on PR #9224:
URL: https://github.com/apache/iceberg/pull/9224#issuecomment-1843230107
Hi, @nk1506
It seems that my code can't pass the unit tests.
Is it because of the default waiting time is not enough? Do you have any ideas?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Use Awaitility instead of Thread.sleep [iceberg]
Posted by "nk1506 (via GitHub)" <gi...@apache.org>.
nk1506 commented on code in PR #9224:
URL: https://github.com/apache/iceberg/pull/9224#discussion_r1417129615
##########
spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestCopyOnWriteDelete.java:
##########
@@ -113,9 +116,10 @@ public synchronized void testDeleteWithConcurrentTableRefresh() throws Exception
executorService.submit(
() -> {
for (int numOperations = 0; numOperations < Integer.MAX_VALUE; numOperations++) {
- while (barrier.get() < numOperations * 2) {
- sleep(10);
- }
+ final int finalNumOperations = numOperations;
Review Comment:
Could u plz run `spotlessApply` and check if it is getting aligned properly?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Use Awaitility instead of Thread.sleep [iceberg]
Posted by "yyy1000 (via GitHub)" <gi...@apache.org>.
yyy1000 commented on PR #9224:
URL: https://github.com/apache/iceberg/pull/9224#issuecomment-1845461829
> > Hi, @nk1506 It seems that my code can't pass the unit tests. Is it because of the default waiting time is not enough? Do you have any ideas?
>
> Please refer [here](https://github.com/apache/iceberg/blob/6a9d3c77977baff4295ee2dde0150d73c8c46af1/aws/src/integration/java/org/apache/iceberg/aws/glue/TestGlueCatalogLock.java#L146) . Also it is advisable to set the `atMost` too.
Oh, got it. I should use condition 'or' but not 'and'. :) Now the unit test can run in my local env.
Awaitliity use default timeout of 10 seconds, and I think it is enough in this case. https://github.com/awaitility/awaitility/wiki/Usage#defaults
So adding a 'atMost' seems not necessary here. What do you think?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Use Awaitility instead of Thread.sleep [iceberg]
Posted by "yyy1000 (via GitHub)" <gi...@apache.org>.
yyy1000 commented on PR #9224:
URL: https://github.com/apache/iceberg/pull/9224#issuecomment-1842988192
> Please change for other spark version too like 3.2,3.3,3.4.
Sure, I just change a single version for review to verify I'm doing right. :)
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Use Awaitility instead of Thread.sleep [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9224:
URL: https://github.com/apache/iceberg/pull/9224#issuecomment-2412570880
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Use Awaitility instead of Thread.sleep [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #9224: Spark: Use Awaitility instead of Thread.sleep
URL: https://github.com/apache/iceberg/pull/9224
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Use Awaitility instead of Thread.sleep [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on PR #9224:
URL: https://github.com/apache/iceberg/pull/9224#issuecomment-2273204616
@yyy1000 sorry I completely missed that this PR exists. Could you please rebase and fix the merge conflicts? It would be great to get this in. Also Let's first only focus on Spark 3.5 changes. We can open a separate PR later to backport to changes to earlier Spark versions
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Use Awaitility instead of Thread.sleep [iceberg]
Posted by "nk1506 (via GitHub)" <gi...@apache.org>.
nk1506 commented on PR #9224:
URL: https://github.com/apache/iceberg/pull/9224#issuecomment-1844833641
> Hi, @nk1506 It seems that my code can't pass the unit tests. Is it because of the default waiting time is not enough? Do you have any ideas?
Please refer [here](https://github.com/apache/iceberg/blob/6a9d3c77977baff4295ee2dde0150d73c8c46af1/aws/src/integration/java/org/apache/iceberg/aws/glue/TestGlueCatalogLock.java#L146) . Also it is advisable to set the `atMost` too.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org