Remove Maven client at runtime + Provide a way to load Druid extensions through local file system #1638

guobingkun · 2015-08-18T17:17:55Z

This is a follow up of https://groups.google.com/forum/#!searchin/druid-development/proposal/druid-development/7KJCsQ9GvGo/u3XMyNLmeAIJ

This pull request does the following,

Remove maven client from downloading extensions at runtime
Provide a way to load Druid extensions and hadoop dependencies through file system.
Refactor pull-deps so that it can download extensions into extension directories.
Add documents on how to use this new extension loading mechanism

guobingkun · 2015-08-19T13:56:08Z

Close and reopen to restart the build.

fjy · 2015-08-21T18:24:44Z

docs/content/operations/including-extensions.md

+
+If you don't specify `druid.extensions.toLoad`, Druid will load all the extensions under root extension directory.
+
+### How to prepare extensions directories using `tools pull-deps`?


I feel this should be the default option as it is the easier option and be placed before the instructions to load manual setup.

I think if we keep including all the base extensions in release tarballs (like @xvrl's newer 0.8.1 builds) then this point is not too important, since most people will be using the prebuilt extensions dir. Only people wanting to use their own extensions would have to decide how to get the jars.

fjy · 2015-08-21T18:32:17Z

@guobingkun I have a few minor comments but overall I am extremely excited for this to get merged. How backwards compatible is this? It seems like it is backwards compatible and we don't need to update the 0.x version.

drcrallen · 2015-08-21T18:33:18Z

processing/src/main/java/io/druid/guice/ExtensionsConfig.java

@@ -33,10 +34,6 @@
  @NotNull
  private boolean searchCurrentClassloader = true;

-  @JsonProperty
-  @NotNull
-  private List<String> coordinates = ImmutableList.of();


Can this actually be left in, and give an error at startup if it is specified?

There doesn't seem to be a migration plan overall here. (or I could have just missed it)

I believe it's sufficient to mention this deprecation in the release note? Even if people set this property, it will be ignored and not cause any harm.

not cause any harm

Not true, if they didn't realize one of the nodes had not been updated to the "new" config, then it will not launch properly, or will have missing functionality that may not even report on bootup.

Not sure if I fully understand this. Are you worrying about people not being aware of one of the nodes hadn't been updated to the latest version? If that node fails on bootup then people will realize it's not updated properly. If that node just runs the old version silently, then failing on "specifying druid.extensions.coordinates" will not help either since that node didn't pick up this code. Or did I miss something?

If someone is updating their cluster to the latest binaries, it could be easy for them to miss the release notes that the druid.extensions.coordinates changed. And it can cause late failures.

Since the config change is not handled at start time, and there is no migration method to warn people of the config change (other than release notes) AND failure to migrate to the new config can cause node failure due to extensions not being present when they are expected to be present, that would make this PR an incompatible change.

Personally, I'm ok with eliminating the setting provided that there is an informative error on startup (or even better, an informative warning and transparent handling of migration until next major version release). Would be worth getting input from the other contributors though.

fjy · 2015-09-17T22:37:46Z

👍 But before merging we should decide on the new version of Druid and mysql packaging

xvrl · 2015-09-17T22:41:08Z

docs/content/tutorials/firewall.md

-
-5. Once all dependencies have been downloaded successfully, replicate the `repo` directory to the machine behind the firewall; e.g. `/opt/druid-<version>/repo`
-6. Create property `druid.extensions.localRepository=`*`path to repo directory`* in the *`Druid Directory`*`/config/_common/common.runtime.properties` file; e.g. `druid.extensions.localRepository=/opt/druid-<version>/repo`
+When you are behind a firewall, the IRC wikipedia channels that feed realtime data into Druid will not be accessible. To workaround this challenge, you will need to make the Wikipedia example GeoLite DB dependency available offline, see below.


This sentence is a bit confusing, making GeoLite avaible offline will not solve the IRC problem as the sentence implies.
There are two problems, one is that IRC may be firewalled, in which case there's nothing you can do. The other problem is that downloading data from maxmind may not be possible, in which case GeoLite needs to be downloaded offline.

updated the doc so that it reflects your comment.

xvrl · 2015-09-17T22:53:16Z

@cheddar unless we relicense the mysql metadata store code under GPL, I don't see how packaging it as a separate tarball would make a difference?

xvrl · 2015-09-17T23:00:11Z

Assuming this has been tested, other than my last few comments about docs, and maybe resolve the mysql tabarll issue outside of this PR, I'm 👍

drcrallen · 2015-09-17T23:15:51Z

server/src/main/java/io/druid/initialization/Initialization.java

-      try {
-        URLClassLoader loader = getClassLoaderForCoordinates(aether, coordinate, config.getDefaultVersion());
+    if (config.searchCurrentClassloader()) {
+      for (T module : ServiceLoader.load(clazz, Thread.currentThread().getContextClassLoader())) {


ServiceLoader.load checks context classloader automatically, but it probably doesn't hurt to have it here.

drcrallen · 2015-09-17T23:26:18Z

This is looking good!

As a note, it is blocked until https://groups.google.com/d/msg/druid-development/KXpLwUisJcU/eCM3GfgPBAAJ is resolved

drcrallen · 2015-10-15T17:16:39Z

@guobingkun I think this is ready to merge, can you please cleanup the merge conflicts?

guobingkun · 2015-10-15T17:51:43Z

@drcrallen Conflicts resolved.

1) Remove maven client from downloading extensions at runtime. 2) Provide a way to load Druid extensions and hadoop dependencies through file system. 3) Refactor pull-deps so that it can download extensions into extension directories. 4) Add documents on how to use this new extension loading mechanism. 5) Change the way how Druid tarball is generated. Now all the extensions + hadoop-client 2.3.0 are packaged within the Druid tarball.

Remove Maven client at runtime + Provide a way to load Druid extensions through local file system

se7entyse7en · 2015-10-30T23:58:51Z

distribution/pom.xml

+                            <goal>single</goal>
+                        </goals>
+                        <configuration>
+                            <finalName>mysql-metdata-storage</finalName>


Is this a typo?

@se7entyse7en Thanks! That's definitely a typo, I will send a PR to fix it.

drcrallen · 2015-12-09T01:42:16Z

This is weird, #2067 but this still passed travis

guobingkun force-pushed the remove_maven_client_code branch 4 times, most recently from b9b48ae to 68d7b9d Compare August 18, 2015 18:10

guobingkun closed this Aug 18, 2015

guobingkun reopened this Aug 18, 2015

guobingkun closed this Aug 19, 2015

guobingkun reopened this Aug 19, 2015

guobingkun force-pushed the remove_maven_client_code branch 3 times, most recently from ce3c742 to 8ce526a Compare August 20, 2015 14:20

guobingkun changed the title ~~Provide a way to load Druid extensions through local file system~~ Remove Maven client at runtime + Provide a way to load Druid extensions through local file system Aug 20, 2015

guobingkun force-pushed the remove_maven_client_code branch 2 times, most recently from ca4574f to 4450881 Compare August 20, 2015 16:28

fjy reviewed Aug 21, 2015
View reviewed changes

drcrallen reviewed Aug 21, 2015
View reviewed changes

xvrl reviewed Sep 17, 2015
View reviewed changes

drcrallen reviewed Sep 17, 2015
View reviewed changes

drcrallen mentioned this pull request Sep 18, 2015

Add Rackspace Cloud Files Deep Storage Extension #1719

Merged

guobingkun force-pushed the remove_maven_client_code branch 2 times, most recently from ef1d59d to 2acd649 Compare September 18, 2015 19:03

xvrl modified the milestone: 0.9.0 Sep 19, 2015

guobingkun force-pushed the remove_maven_client_code branch from 2acd649 to a18d62f Compare October 2, 2015 15:19

guobingkun force-pushed the remove_maven_client_code branch from a18d62f to 05aa0b3 Compare October 15, 2015 17:26

guobingkun force-pushed the remove_maven_client_code branch from 05aa0b3 to f4a8fa6 Compare October 20, 2015 16:21

guobingkun force-pushed the remove_maven_client_code branch from f4a8fa6 to 4914925 Compare October 21, 2015 19:24

fjy added a commit that referenced this pull request Oct 26, 2015

Merge pull request #1638 from guobingkun/remove_maven_client_code

5f23703

Remove Maven client at runtime + Provide a way to load Druid extensions through local file system

fjy merged commit 5f23703 into apache:master Oct 26, 2015

guobingkun deleted the remove_maven_client_code branch October 26, 2015 16:32

se7entyse7en reviewed Oct 30, 2015
View reviewed changes

gianm added the Incompatible label Nov 7, 2015

gianm mentioned this pull request Nov 7, 2015

Implement cache module for Redis #1927

Closed

drcrallen added the Release Notes label Jan 22, 2016

guobingkun added the Area - Documentation label Jan 26, 2016

fjy mentioned this pull request Feb 5, 2016

druid-0.9.0 release notes #2404

Closed

xvrl mentioned this pull request Feb 11, 2016

distribution tarball should be created as part of maven package phase #2440

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove Maven client at runtime + Provide a way to load Druid extensions through local file system #1638

Remove Maven client at runtime + Provide a way to load Druid extensions through local file system #1638

guobingkun commented Aug 18, 2015

guobingkun commented Aug 19, 2015

fjy Aug 21, 2015

gianm Aug 21, 2015

fjy commented Aug 21, 2015

drcrallen Aug 21, 2015

guobingkun Aug 21, 2015

drcrallen Aug 21, 2015

guobingkun Aug 24, 2015

drcrallen Aug 24, 2015

fjy commented Sep 17, 2015

xvrl Sep 17, 2015

guobingkun Sep 18, 2015

xvrl commented Sep 17, 2015

xvrl commented Sep 17, 2015

drcrallen Sep 17, 2015

drcrallen commented Sep 17, 2015

drcrallen commented Oct 15, 2015

guobingkun commented Oct 15, 2015

se7entyse7en Oct 30, 2015

guobingkun Oct 31, 2015

drcrallen commented Dec 9, 2015


		If you don't specify `druid.extensions.toLoad`, Druid will load all the extensions under root extension directory.

		### How to prepare extensions directories using `tools pull-deps`?

Remove Maven client at runtime + Provide a way to load Druid extensions through local file system #1638

Remove Maven client at runtime + Provide a way to load Druid extensions through local file system #1638

Conversation

guobingkun commented Aug 18, 2015

guobingkun commented Aug 19, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fjy commented Aug 21, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fjy commented Sep 17, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xvrl commented Sep 17, 2015

xvrl commented Sep 17, 2015

Choose a reason for hiding this comment

drcrallen commented Sep 17, 2015

drcrallen commented Oct 15, 2015

guobingkun commented Oct 15, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drcrallen commented Dec 9, 2015