[QTL] Implement LookupExtractorFactory of namespaced lookup #2716

sirpkt · 2016-03-24T02:28:59Z

This patch adds LookupExtractorFactory of namespaced lookup.
With this, NamespacedExtractor can be used with LookupDimensionSpec.
Done as a first step to merge #2524 with QTL.

Migrated to #2926

drcrallen · 2016-03-24T18:19:57Z

...amespace-lookup/src/main/java/io/druid/query/extraction/NamespaceLookupExtractorFactory.java

+        try {
+          Thread.sleep(10);
+        } catch (InterruptedException e) {
+          e.printStackTrace();


Should log that it was interrupted (probably as error) and return false.

drcrallen · 2016-03-25T00:12:31Z

...amespace-lookup/src/main/java/io/druid/query/extraction/NamespaceLookupExtractorFactory.java

+import javax.annotation.Nullable;
+import java.util.List;
+
+@JsonTypeName("name")


suggest making this namespace to match io.druid.query.extraction.NamespacedExtractor

drcrallen · 2016-03-25T17:39:09Z

I realized I hadn't written this down yet, but I'm thinking this PR might be able to be the switching point for the extension to allow the recommended lookups to go through the dynamic configuration instead of through static configs for this extension. (mentioned in #2735 (comment) )

If you think such a task is too far out of scope for this PR, please let me know and we can limit the scope here to a minimal change, and have a potentially more sweeping change come later.

sirpkt · 2016-03-28T02:08:21Z

@drcrallen Ok I'll try to eliminate static configuration of namespace lookup.
As I know, druid-namespace-lookup and druid-kaka-extraction-namespace are the only two extension modules related with lookup, right?

drcrallen · 2016-03-28T19:43:52Z

@sirpkt The only druid.io modules, yes.

… eliminate static configurations for lookup from namespecd lookup extensions - druid-namespace-lookup and druid-kafka-extraction-namespace are modified - However, druid-namespace-lookup still has configuration about ON/OFF HEAP cache manager selection, which is not namespace wide configuration but node wide configuration as multiple namespace shares the same cache manager

…nges

sirpkt · 2016-04-01T00:44:19Z

Static config for lookup is eliminated.
Only druid.query.extraction.namespace.cache.type is remained to select type of cache between on/off heap as it is module-wide not per-namespace config.

drcrallen · 2016-04-05T17:54:54Z

...on-namespace/src/main/java/io/druid/query/extraction/namespace/KafkaExtractionNamespace.java


  @JsonCreator
  public KafkaExtractionNamespace(
      @NotNull @JsonProperty(value = "kafkaTopic", required = true) final String kafkaTopic,
-      @NotNull @JsonProperty(value = "namespace", required = true) final String namespace
+      @NotNull @JsonProperty(value = "kafkaProperites", required = true) final Properties kafkaProperties


Does this need to be required? also required=true is just a hint, you still have to enforce not null checks in constructor to be ACTUALLY required.

nishantmonu51 · 2016-04-18T12:57:49Z

...fka-extraction-namespace/src/main/java/io/druid/server/namespace/KafkaExtractionManager.java

-import java.util.concurrent.ConcurrentLinkedQueue;
-import java.util.concurrent.ConcurrentMap;
-import java.util.concurrent.Executors;
+import java.util.concurrent.*;


could you only import the required classes ?

b-slim · 2016-04-20T21:48:56Z

@sirpkt and @drcrallen what's the status of this PR ?

sirpkt · 2016-04-22T04:26:21Z

@b-slim waiting for the commit of #2800.
This will be rebased based on it.

drcrallen · 2016-04-22T05:49:58Z

@b-slim excluding the kafka extension, the rest should be about ready.

b-slim · 2016-04-27T15:11:04Z

@drcrallen and @sirpkt can we split this to make review straightforward ?

drcrallen · 2016-05-02T18:14:42Z

@sirpkt Please check out sirpkt#2 which is a merge to master and reverting of the kafka stuff

Merge master lookups

sirpkt · 2016-05-03T06:05:49Z

merge sirpkt#2 and rebased.

drcrallen · 2016-05-03T14:25:31Z

@b-slim this should be ready for final review (and merge)

fjy · 2016-05-05T17:10:08Z

@b-slim can we finish this up?

b-slim · 2016-05-05T17:13:12Z

@fjy it is in the pipeline...

b-slim · 2016-05-05T17:55:53Z

...fka-extraction-namespace/src/main/java/io/druid/server/namespace/KafkaExtractionManager.java

+ */
+public class KafkaExtractionManager
+{
+  private static final Logger log = new Logger(KafkaExtractionManager.class);


can we call all the static variable with upper case please ?

log is log across all of the Druid code base...

b-slim · 2016-05-05T19:09:04Z

@drcrallen let me know when this is ready.

drcrallen · 2016-05-05T20:00:39Z

@b-slim the non-kafka stuff should be ready

drcrallen · 2016-05-05T20:35:39Z

@sirpkt I'm going to move this PR to a MMX branch. Since most of the reviewers are in my timezone I can have faster modifications while they are awake.

drcrallen · 2016-05-05T20:35:47Z

Moved to #2926

sirpkt · 2016-05-05T23:12:35Z

@drcrallen OK

drcrallen added this to the 0.9.1 milestone Mar 24, 2016

drcrallen added Feature Discuss labels Mar 24, 2016

drcrallen reviewed Mar 24, 2016
View reviewed changes

drcrallen self-assigned this Mar 24, 2016

drcrallen reviewed Mar 25, 2016
View reviewed changes

drcrallen mentioned this pull request Mar 25, 2016

Move lookup docs that are in druid-proper back into lookups.md #2735

Merged

sirpkt force-pushed the qtl_namespace_lookup branch from 7f918f3 to 14f4c88 Compare March 31, 2016 01:15

update KafkaExtractionNamespaceTest to reflect argument signature cha…

7d75062

…nges

drcrallen reviewed Apr 5, 2016
View reviewed changes

drcrallen removed their assignment Apr 12, 2016

nishantmonu51 reviewed Apr 18, 2016
View reviewed changes

sirpkt added 2 commits April 27, 2016 10:05

Merge remote-tracking branch 'upstream/master' into qtl_namespace_lookup

379af21

resolve compile error by supporting LookupIntrospectHandler

5b35e42

This was referenced Apr 27, 2016

[QTL] Make URI Exctraction Namespace take more sane arguments #2738

Merged

[QTL] Immediate future plans #2889

Closed

Merge remote-tracking branch 'druid/master' into mergeMasterLookups

688e7c1

sirpkt added 2 commits May 3, 2016 11:54

Merge pull request #2 from metamx/mergeMasterLookups

10fc4f8

Merge master lookups

Merge remote-tracking branch 'upstream/master' into qtl_namespace_lookup

f2b6864

b-slim reviewed May 5, 2016
View reviewed changes

drcrallen mentioned this pull request May 5, 2016

[QTL] Implement LookupExtractorFactory of namespaced lookup #2926

Merged

drcrallen closed this May 5, 2016

sirpkt deleted the qtl_namespace_lookup branch May 30, 2016 09:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[QTL] Implement LookupExtractorFactory of namespaced lookup #2716

[QTL] Implement LookupExtractorFactory of namespaced lookup #2716

sirpkt commented Mar 24, 2016 •

edited by drcrallen

Loading

drcrallen Mar 24, 2016

sirpkt Mar 25, 2016

drcrallen Mar 25, 2016

sirpkt Mar 25, 2016

drcrallen commented Mar 25, 2016

sirpkt commented Mar 28, 2016

drcrallen commented Mar 28, 2016

sirpkt commented Apr 1, 2016

drcrallen Apr 5, 2016

nishantmonu51 Apr 18, 2016

b-slim commented Apr 20, 2016

sirpkt commented Apr 22, 2016

drcrallen commented Apr 22, 2016

b-slim commented Apr 27, 2016

drcrallen commented May 2, 2016

sirpkt commented May 3, 2016

drcrallen commented May 3, 2016

fjy commented May 5, 2016

b-slim commented May 5, 2016

b-slim May 5, 2016

cheddar May 5, 2016

b-slim commented May 5, 2016

drcrallen commented May 5, 2016

drcrallen commented May 5, 2016

drcrallen commented May 5, 2016

sirpkt commented May 5, 2016

[QTL] Implement LookupExtractorFactory of namespaced lookup #2716

[QTL] Implement LookupExtractorFactory of namespaced lookup #2716

Conversation

sirpkt commented Mar 24, 2016 • edited by drcrallen Loading

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

drcrallen commented Mar 25, 2016

sirpkt commented Mar 28, 2016

drcrallen commented Mar 28, 2016

sirpkt commented Apr 1, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

b-slim commented Apr 20, 2016

sirpkt commented Apr 22, 2016

drcrallen commented Apr 22, 2016

b-slim commented Apr 27, 2016

drcrallen commented May 2, 2016

sirpkt commented May 3, 2016

drcrallen commented May 3, 2016

fjy commented May 5, 2016

b-slim commented May 5, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

b-slim commented May 5, 2016

drcrallen commented May 5, 2016

drcrallen commented May 5, 2016

drcrallen commented May 5, 2016

sirpkt commented May 5, 2016

sirpkt commented Mar 24, 2016 •

edited by drcrallen

Loading