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 2021/04/29 08:47:57 UTC
[GitHub] [spark] wForget opened a new pull request #32395: [SPARK-35270][SQL][CORE] Remove the use of guava to fix Hadoop 3.2.2 guava conflict.
wForget opened a new pull request #32395:
URL: https://github.com/apache/spark/pull/32395
### What changes were proposed in this pull request?
Remove the use of guava to fix Hadoop 3.2.2 guava conflict.
### Why are the changes needed?
Hadoop 3.2.2 uses Guava 27, the change is for the guava version upgrade.
### Does this PR introduce _any_ user-facing change?
no
### How was this patch tested?
Modify the guava version to 27.0-jre, and then compile.
--
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:
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] srowen commented on a change in pull request #32395: [SPARK-35270][SQL][CORE] Remove the use of guava to fix Hadoop 3.2.2 guava conflict.
Posted by GitBox <gi...@apache.org>.
srowen commented on a change in pull request #32395:
URL: https://github.com/apache/spark/pull/32395#discussion_r623062758
##########
File path: common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/RemoteBlockPushResolver.java
##########
@@ -760,19 +762,19 @@ public boolean equals(Object o) {
return false;
}
AppShuffleId that = (AppShuffleId) o;
- return shuffleId == that.shuffleId && Objects.equal(appId, that.appId);
+ return shuffleId == that.shuffleId && Objects.equals(appId, that.appId);
}
@Override
public int hashCode() {
- return Objects.hashCode(appId, shuffleId);
+ return Objects.hash(appId, shuffleId);
}
@Override
public String toString() {
- return Objects.toStringHelper(this)
- .add("appId", appId)
- .add("shuffleId", shuffleId)
+ return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE)
Review comment:
It isn't super important here I think, but does this result in the same string?
--
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:
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] github-actions[bot] closed pull request #32395: [SPARK-35270][SQL][CORE] Remove the use of guava in order to upgrade guava version to 27
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #32395:
URL: https://github.com/apache/spark/pull/32395
--
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] AmplabJenkins commented on pull request #32395: [SPARK-35270][SQL][CORE] Remove the use of guava to fix Hadoop 3.2.2 guava conflict.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32395:
URL: https://github.com/apache/spark/pull/32395#issuecomment-829055165
Can one of the admins verify this patch?
--
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:
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] sunchao commented on pull request #32395: [SPARK-35270][SQL][CORE] Remove the use of guava to fix Hadoop 3.2.2 guava conflict.
Posted by GitBox <gi...@apache.org>.
sunchao commented on pull request #32395:
URL: https://github.com/apache/spark/pull/32395#issuecomment-829393525
@wForget Spark master branch has already moved to use shaded Hadoop client by default (see [SPARK-33212](https://issues.apache.org/jira/browse/SPARK-33212) which effectively isolated itself from the Guava version on the Hadoop side. Did you actually see a Guava conflict issue?
--
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:
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] wForget commented on pull request #32395: [SPARK-35270][SQL][CORE] Remove the use of guava in order to upgrade guava version to 27
Posted by GitBox <gi...@apache.org>.
wForget commented on pull request #32395:
URL: https://github.com/apache/spark/pull/32395#issuecomment-833178638
@HyukjinKwon I have enabled it, how to rerun these checks?
--
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:
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] srowen commented on pull request #32395: [SPARK-35270][SQL][CORE] Remove the use of guava in order to upgrade guava version to 27
Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #32395:
URL: https://github.com/apache/spark/pull/32395#issuecomment-852136354
I think the concern about changing behavior still stands?
--
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:
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] github-actions[bot] closed pull request #32395: [SPARK-35270][SQL][CORE] Remove the use of guava in order to upgrade guava version to 27
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #32395:
URL: https://github.com/apache/spark/pull/32395
--
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 #32395: [SPARK-35270][SQL][CORE] Remove the use of guava to fix Hadoop 3.2.2 guava conflict.
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #32395:
URL: https://github.com/apache/spark/pull/32395#issuecomment-829395958
+1 for @sunchao 's comment. Also, Apache Spark 3.2 is moving toward to Hadoop 3.3.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:
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] HyukjinKwon commented on pull request #32395: [SPARK-35270][SQL][CORE] Remove the use of guava in order to upgrade guava version to 27
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #32395:
URL: https://github.com/apache/spark/pull/32395#issuecomment-831020675
@wForget can you enable GitHub Actions in your forked repository? https://github.com/apache/spark/pull/32395/checks?check_run_id=2465058510
--
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:
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 #32395: [SPARK-35270][SQL][CORE] Remove the use of guava to fix Hadoop 3.2.2 guava conflict.
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #32395:
URL: https://github.com/apache/spark/pull/32395#issuecomment-829798261
Ya, `different hash` bites us always; at Scala version changes and at Spark versions change.
For this case, I'm not sure, but I'll leave this up to your decisions, @srowen and @sunchao .
BTW, it seems that we need to revise the wrong title and PR description about `Hadoop 3.2.2`. Could you make this PR neutral from Hadoop, @wForget .
--
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:
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] pan3793 commented on pull request #32395: [SPARK-35270][SQL][CORE] Remove the use of guava in order to upgrade guava version to 27
Posted by GitBox <gi...@apache.org>.
pan3793 commented on pull request #32395:
URL: https://github.com/apache/spark/pull/32395#issuecomment-886292379
any update?
--
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] HyukjinKwon commented on pull request #32395: [SPARK-35270][SQL][CORE] Remove the use of guava in order to upgrade guava version to 27
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #32395:
URL: https://github.com/apache/spark/pull/32395#issuecomment-833195384
Did you do something like https://github.com/apache/spark/pull/32400#issuecomment-831051189 too? If it's done, feel free to rebase which should retrigger the test.
--
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:
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] srowen commented on pull request #32395: [SPARK-35270][SQL][CORE] Remove the use of guava to fix Hadoop 3.2.2 guava conflict.
Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #32395:
URL: https://github.com/apache/spark/pull/32395#issuecomment-829508036
It may be unnecessary for the reason above; it still probably wouldn't hurt to just move these to standard JDK classes. I have a little bit of worry about changing behavior, with a possibly different hash or toString, though
--
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:
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] sunchao commented on pull request #32395: [SPARK-35270][SQL][CORE] Remove the use of guava to fix Hadoop 3.2.2 guava conflict.
Posted by GitBox <gi...@apache.org>.
sunchao commented on pull request #32395:
URL: https://github.com/apache/spark/pull/32395#issuecomment-829509170
@srowen yes agreed - it's better to avoid Guava usage in general if it's not necessary.
--
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:
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] github-actions[bot] commented on pull request #32395: [SPARK-35270][SQL][CORE] Remove the use of guava in order to upgrade guava version to 27
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #32395:
URL: https://github.com/apache/spark/pull/32395#issuecomment-960309924
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!
--
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] github-actions[bot] closed pull request #32395: [SPARK-35270][SQL][CORE] Remove the use of guava in order to upgrade guava version to 27
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #32395:
URL: https://github.com/apache/spark/pull/32395
--
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] wForget commented on a change in pull request #32395: [SPARK-35270][SQL][CORE] Remove the use of guava in order to upgrade guava version to 27
Posted by GitBox <gi...@apache.org>.
wForget commented on a change in pull request #32395:
URL: https://github.com/apache/spark/pull/32395#discussion_r623641360
##########
File path: common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/RemoteBlockPushResolver.java
##########
@@ -760,19 +762,19 @@ public boolean equals(Object o) {
return false;
}
AppShuffleId that = (AppShuffleId) o;
- return shuffleId == that.shuffleId && Objects.equal(appId, that.appId);
+ return shuffleId == that.shuffleId && Objects.equals(appId, that.appId);
}
@Override
public int hashCode() {
- return Objects.hashCode(appId, shuffleId);
+ return Objects.hash(appId, shuffleId);
}
@Override
public String toString() {
- return Objects.toStringHelper(this)
- .add("appId", appId)
- .add("shuffleId", shuffleId)
+ return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE)
Review comment:
The results are different. Using Objects to return is like "AppShuffleId{appId=appId, shuffleId=100}", using ToStringBuilder to return is like "RemoteBlockPushResolver.AppShuffleId[appId=appId,shuffleId=100]". Will it cause some problems?
--
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:
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] wForget commented on pull request #32395: [SPARK-35270][SQL][CORE] Remove the use of guava to fix Hadoop 3.2.2 guava conflict.
Posted by GitBox <gi...@apache.org>.
wForget commented on pull request #32395:
URL: https://github.com/apache/spark/pull/32395#issuecomment-829855196
@sunchao @dongjoon-hyun @srowen
Sorry, my description here is not accurate. The conflict caused by the program introduced multiple versions of guava, I tried to modify the guava version to 27 and found a compilation problem.
--
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:
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] pan3793 commented on pull request #32395: [SPARK-35270][SQL][CORE] Remove the use of guava in order to upgrade guava version to 27
Posted by GitBox <gi...@apache.org>.
pan3793 commented on pull request #32395:
URL: https://github.com/apache/spark/pull/32395#issuecomment-851187049
Seems `spark-core` already shaded guava, and for Hadoop 3.2, since spark already moved to Hadoop Shaded Client, I only see Curator depends on guava, from https://cwiki.apache.org/confluence/display/CURATOR/TN13 , I think it's ok to bundle a high version of guava in Spark hadoop-3.2 binary dist?
--
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:
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] srowen commented on a change in pull request #32395: [SPARK-35270][SQL][CORE] Remove the use of guava in order to upgrade guava version to 27
Posted by GitBox <gi...@apache.org>.
srowen commented on a change in pull request #32395:
URL: https://github.com/apache/spark/pull/32395#discussion_r623829657
##########
File path: common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/RemoteBlockPushResolver.java
##########
@@ -760,19 +762,19 @@ public boolean equals(Object o) {
return false;
}
AppShuffleId that = (AppShuffleId) o;
- return shuffleId == that.shuffleId && Objects.equal(appId, that.appId);
+ return shuffleId == that.shuffleId && Objects.equals(appId, that.appId);
}
@Override
public int hashCode() {
- return Objects.hashCode(appId, shuffleId);
+ return Objects.hash(appId, shuffleId);
}
@Override
public String toString() {
- return Objects.toStringHelper(this)
- .add("appId", appId)
- .add("shuffleId", shuffleId)
+ return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE)
Review comment:
Like with hash function changes, it shouldn't matter to programs. But if some program did rely on it, directly or accidentally, this might break. It's a tough call - how much is the change worth? overall it's an OK improvement but yeah I'm hesitant for just this reason. It's more the hash change than this one.
--
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:
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] github-actions[bot] commented on pull request #32395: [SPARK-35270][SQL][CORE] Remove the use of guava in order to upgrade guava version to 27
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #32395:
URL: https://github.com/apache/spark/pull/32395#issuecomment-960309924
--
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] sunchao edited a comment on pull request #32395: [SPARK-35270][SQL][CORE] Remove the use of guava to fix Hadoop 3.2.2 guava conflict.
Posted by GitBox <gi...@apache.org>.
sunchao edited a comment on pull request #32395:
URL: https://github.com/apache/spark/pull/32395#issuecomment-829393525
@wForget Spark master branch has already moved to use shaded Hadoop client by default (see [SPARK-33212](https://issues.apache.org/jira/browse/SPARK-33212)) which effectively isolated itself from the Guava version on the Hadoop side. Did you actually see a Guava conflict issue?
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org