[
https://issues.apache.org/jira/browse/KAFKA-6467?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16341253#comment-16341253
]
ASF GitHub Bot commented on KAFKA-6467:
---------------------------------------
hachikuji closed pull request #4459: KAFKA-6467: Enforce layout of dependencies
within a connect plugin to be deterministic
URL: https://github.com/apache/kafka/pull/4459
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/PluginUtils.java
b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/PluginUtils.java
index 9ef7c3a40e3..c1774e971aa 100644
---
a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/PluginUtils.java
+++
b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/PluginUtils.java
@@ -34,6 +34,7 @@
import java.util.List;
import java.util.Locale;
import java.util.Set;
+import java.util.TreeSet;
/**
* Connect plugin utility methods.
@@ -208,7 +209,7 @@ public static boolean isClassFile(Path path) {
*/
public static List<Path> pluginUrls(Path topPath) throws IOException {
boolean containsClassFiles = false;
- Set<Path> archives = new HashSet<>();
+ Set<Path> archives = new TreeSet<>();
LinkedList<DirectoryEntry> dfs = new LinkedList<>();
Set<Path> visited = new HashSet<>();
diff --git
a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/PluginUtilsTest.java
b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/PluginUtilsTest.java
index f9532a6d8cb..61ece1c779e 100644
---
a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/PluginUtilsTest.java
+++
b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/PluginUtilsTest.java
@@ -26,6 +26,7 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collections;
import java.util.List;
@@ -158,6 +159,22 @@ public void testPluginUrlsWithJars() throws Exception {
assertUrls(expectedUrls, PluginUtils.pluginUrls(pluginPath));
}
+ @Test
+ public void testOrderOfPluginUrlsWithJars() throws Exception {
+ createBasicDirectoryLayout();
+ // Here this method is just used to create the files. The result is
not used.
+ createBasicExpectedUrls();
+
+ List<Path> actual = PluginUtils.pluginUrls(pluginPath);
+ // 'simple-transform.jar' is created first. In many cases, without
sorting within the
+ // PluginUtils, this jar will be placed before
'another-transform.jar'. However this is
+ // not guaranteed because a DirectoryStream does not maintain a
certain order in its
+ // results. Besides this test case, sorted order in every call to
assertUrls below.
+ int i =
Arrays.toString(actual.toArray()).indexOf("another-transform.jar");
+ int j =
Arrays.toString(actual.toArray()).indexOf("simple-transform.jar");
+ assertTrue(i < j);
+ }
+
@Test
public void testPluginUrlsWithZips() throws Exception {
createBasicDirectoryLayout();
@@ -262,9 +279,8 @@ private void createBasicDirectoryLayout() throws
IOException {
}
private void assertUrls(List<Path> expected, List<Path> actual) {
- List<Path> actualCopy = new ArrayList<>(actual);
Collections.sort(expected);
- Collections.sort(actualCopy);
- assertEquals(expected, actualCopy);
+ // not sorting 'actual' because it should be returned sorted from
withing the PluginUtils.
+ assertEquals(expected, actual);
}
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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
> Enforce layout of dependencies within a Connect plugin to be deterministic
> --------------------------------------------------------------------------
>
> Key: KAFKA-6467
> URL: https://issues.apache.org/jira/browse/KAFKA-6467
> Project: Kafka
> Issue Type: Bug
> Components: KafkaConnect
> Affects Versions: 1.0.0, 0.11.0.2
> Reporter: Konstantine Karantasis
> Assignee: Konstantine Karantasis
> Priority: Blocker
> Fix For: 1.0.1, 0.11.0.3
>
>
> In principle, Connect plugins that intend to load their dependencies in
> isolation should not contain any conflicts among the classes they package as
> dependencies. In other words, the order in which a plugin's dependencies are
> laid out and passed to its plugin class loader should not matter.
> However, in practice, there are rare and suboptimal situations where a plugin
> needs to bundle a few packages with conflicting dependencies because it
> doesn't control packaging of third-party modules. In those cases depending on
> a deterministic ordering within the class loader's path can help the Connect
> plugin enforce loading of the desired classes as needed. (For example, see
> [HDFS connector with MapR
> libs|https://github.com/confluentinc/kafka-connect-hdfs/issues/270] or [HDFS
> connector with Hive's extended
> jar|https://github.com/confluentinc/kafka-connect-hdfs/issues/261])
> To achieve such ordering, this improvement suggests ordering a plugin's
> dependencies in a nested directory structure by sorting such paths
> alphanumerically. This way the deterministic order is implicit (no extra
> configuration is required) and a specific dependency can be put earlier or
> later in the class loader's path with appropriate naming of its package path
> (e.g. within the plugin's directory).
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
