[QTL] Query time lookup cluster wide config #1576
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -46,4 +46,6 @@ public Void insertOrUpdate( | |||
| public void createTaskTables(); | |||
|
|
|||
| public void createAuditTable(); | |||
|
|
|||
| public void createNamespacesTable(); | |||
There was a problem hiding this comment.
I'm considering moving this to a module, so don't get caught up on it being in mainline druid
drcrallen
force-pushed
the
queryTimeLookup_announce
branch
from
8e6d2b5
to
2cd6e31
Compare
| @@ -234,17 +238,17 @@ | |||
| <dependency> | |||
| <groupId>com.google.inject</groupId> | |||
| <artifactId>guice</artifactId> | |||
| <version>4.0-beta</version> | |||
| <version>4.0</version> | |||
There was a problem hiding this comment.
Will remove these version changes shortly.
drcrallen
force-pushed
the
queryTimeLookup_announce
branch
3 times, most recently
from
5e1ed09
to
ab97768
Compare
drcrallen
force-pushed
the
queryTimeLookup_announce
branch
from
ab97768
to
6b29191
Compare
drcrallen
force-pushed
the
queryTimeLookup_announce
branch
from
6b29191
to
34eff84
Compare
drcrallen
force-pushed
the
queryTimeLookup_announce
branch
from
34eff84
to
e5f02d0
Compare
|
Closing for now |
|
|
||
|
|
||
| # Lookups Dynamic Config (EXPERIMENTAL) | ||
| Thse configuration options control the behavior of the Lookup dynamic configuration described in the [lookups page](../querying/lookups.html) |
|
have finished reviewing this PR -
I am 👍 on this and think this can be merged once 1 is taken care of unless @b-slim has any more comments. |
drcrallen
force-pushed
the
queryTimeLookup_announce
branch
from
2c7a90e
to
70905c5
Compare
|
Squashed |
| @@ -26,7 +26,7 @@ | |||
| <parent> | |||
| <groupId>io.druid</groupId> | |||
| <artifactId>druid</artifactId> | |||
| <version>0.9.1-SNAPSHOT</version> | |||
| <version>0.9.0-SNAPSHOT</version> | |||
There was a problem hiding this comment.
I'll undo these, give me a sec
drcrallen
force-pushed
the
queryTimeLookup_announce
branch
4 times, most recently
from
0211eef
to
2f36d73
Compare
| LifecycleModule.register(binder, LookupReferencesManager.class); | ||
| JsonConfigProvider.bind(binder, PROPERTY_BASE, LookupListeningAnnouncerConfig.class); | ||
| Jerseys.addResource(binder, LookupListeningResource.class); | ||
| LifecycleModule.register(binder, LookupReferencesManager.class); |
There was a problem hiding this comment.
duplicated LifecycleModule.register(binder, LookupReferencesManager.class)
There was a problem hiding this comment.
good call, artifact of merging module definitions. Fixed
drcrallen
force-pushed
the
queryTimeLookup_announce
branch
from
2f36d73
to
5da9a28
Compare
|
👍 please make sure to have the issue linked to the future improvement especially about initialization. |
This was referenced Mar 18, 2016
nishantmonu51
added a commit
that referenced
this pull request
Mar 18, 2016
[QTL] Query time lookup cluster wide config
seoeun25
pushed a commit
to seoeun25/incubator-druid
that referenced
this pull request
Jan 10, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the next PR for QTL where cluster wide configuration is possible.
This approach uses POSTs from the coordinator to populate QTL information in all interested nodes. Interested nodes "register" via the Announcer (currently ZK based). But all communication occurs through HTTP directly between the coordinator and the node.
The PR lists over 4k lines, but a LOT of that is tests.
TODO:Ready to mergeProposal for coordinator and node endpointsSquash (Leaving change log while reviews are occurring)DocsPunted until extensions are updatedMigrate coordinator code to a module.Figure out a better way to handle the listening handling. (see Add listening-announcer extension #2286 )added in this PR in coreMigrate lookup config to auditable config type.Rebase on Promoting LookupExtractor state and LookupExtractorFactory to be a first class druid state object. #2291Move core stuff and extension stuff to its own commitMake sure put functions as expected. (I don't think deleting actually works in this version)Make sure initialization behaves as expectedActually run the thing locallyReconcile need for static config vs dynamic config. Ex: what happens if someone requests a delete or get of a lookup defined in runtime.properties?No static configMore unit tests for delete funcitonMake timeouts configurabledoneDocument historical node endpoints as advanced featureDocument experimental update aspectAdded javadoc forreplacesaddition toLookupExtractorFactoryKNOWN ISSUES:
Currently if someone manually goes in and modifies lookups on a lookup node, the cluster wide config will attempt to re-populate that node. The way to get around this behavior is to add the lookups to the node independent of cluster-wide config.
The lookup coordinator does try to delete lookups that were removed from the config.
THERE IS A MAJOR CAVEAT HERE: If a node's network is disconnected during this delete (or it is otherwise temporarily unavailable) there is no good way for the coordinator to know that, when it can communicate with the node again, if the
some_lookupis has is supposed to be there as a custom config, or ifsome_lookupis there from a failed delete.Such an occurrence should be RARE in the current IMPL though.