-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade guava version to 31.1-jre #14767
Upgrade guava version to 31.1-jre #14767
Conversation
licenses.yaml
Outdated
license_category: binary | ||
module: java-core | ||
license_name: Apache License version 2.0 | ||
version: 9999.0-empty-to-avoid-conflict-with-guava |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this change required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently this is a transitive dependency of com.google.guava:guava:31.1-jre
. Although this is an empty artifactory I am not entirely sure of repurcusions of removing this dependency.
@@ -48,6 +48,7 @@ | |||
|
|||
/** | |||
*/ | |||
@SuppressWarnings("CheckReturnValue") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why this change was needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is causing this warning during strict compilation check.
druid/processing/src/test/java/org/apache/druid/collections/spatial/ImmutableRTreeTest.java
Line 583 in a8eaa1e
Iterables.size(points); |
server/src/main/java/org/apache/druid/client/HttpServerInventoryView.java
Show resolved
Hide resolved
@@ -265,7 +265,7 @@ public void testPollPeriodicallyAndOnDemandInterleave() throws Exception | |||
Assert.assertTrue(sqlSegmentsMetadataManager.getLatestDatabasePoll() instanceof SqlSegmentsMetadataManager.PeriodicDatabasePoll); | |||
dataSourcesSnapshot = sqlSegmentsMetadataManager.getDataSourcesSnapshot(); | |||
Assert.assertEquals( | |||
ImmutableList.of("wikipedia2", "wikipedia3", "wikipedia"), | |||
ImmutableList.of("wikipedia3", "wikipedia", "wikipedia2"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why does this order change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes to this method com.google.common.collect.Maps.newHashMapWithExpectedSize(int expectedSize)
which we're calling from here
final Map<K, V2> result = Maps.newHashMapWithExpectedSize(map.size()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like the order changed again in https://github.com/apache/druid/pull/15482/files#r1414283468. Given that we are not order sensitive it might make more sense to compare sets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking great! I agree with the changes to use latest guava api by passing in direct executor in many places where the legacy method args are no longer supported. I share questions with @abhishekagarwal87 about a lot of the changes to annotations and seemingly trivial changes other places that aren't clearly a part of this specific change. I'd prefer we keep this PR to just the base set of changes for guava itself and push any other fixups in another PR if possible! But overall, this is a very cool PR to finally see come upstream!
...test/java/org/apache/druid/indexing/common/task/AppenderatorDriverRealtimeIndexTaskTest.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/apache/druid/client/HttpServerInventoryView.java
Show resolved
Hide resolved
@@ -236,7 +236,7 @@ public void poll() | |||
{ | |||
try { | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this required for this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This ImmutableMap.copyOf(Map<? extends K, ? extends V> map)
is now overloaded with another method ImmutableMap.copyOf(Iterable<? extends Entry<? extends K, ? extends V>> entries)
. Compiler was complaining when I tried to cast the params to Map<String, List<Rule>>
hence created another variable with that type and used it to obtain ImmutableMap.
Several dependabot ignore directives are no longer relevant. Unpin them to ensure we get again get timely updates via dependabot. * support for Hadoop 2 was dropped as part of apache#14763 * Guava was upgraded to 31 as part of apache#14767 * Calcite was upgraded to 1.35 as part of apache#14510
Several dependabot ignore directives are no longer relevant. Unpin them to ensure we get again get timely updates via dependabot. * support for Hadoop 2 was dropped as part of apache#14763 * Guava was upgraded to 31 as part of apache#14767 * Calcite was upgraded to 1.35 as part of apache#14510
Currently, Druid is using Guava
16.0.1
version. This upgrade to31.1-jre
fixes the following issues.AtomicDoubleArray
class (when serialized with Java serialization) and theCompoundOrdering
class (when serialized with GWT serialization) perform eager allocation without appropriate checks on what a client has sent and whether the data size is reasonable). We don't use Java or GWT serializations. Despite being false positive they're causing red security scans on Druid distribution.google-client-api
is incompatible with the existing Guava version. This PR unblocks Update google client apis to latest version #14414