[ 
https://issues.apache.org/jira/browse/DRILL-5089?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15942160#comment-15942160
 ] 

ASF GitHub Bot commented on DRILL-5089:
---------------------------------------

Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/795#discussion_r108052279
  
    --- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/store/SchemaTreeProvider.java
 ---
    @@ -119,6 +127,74 @@ public SchemaPlus createRootSchema(SchemaConfig 
schemaConfig) {
         }
       }
     
    +
    +  public SchemaPlus createPartialRootSchema(final String userName, final 
SchemaConfigInfoProvider provider,
    +                                            final String storage) {
    +    final String schemaUser = isImpersonationEnabled ? userName : 
ImpersonationUtil.getProcessUserName();
    +    final SchemaConfig schemaConfig = SchemaConfig.newBuilder(schemaUser, 
provider).build();
    +    final SchemaPlus rootSchema = 
SimpleCalciteSchema.createRootSchema(false);
    +    Set<String> storageSet = Sets.newHashSet();
    +    storageSet.add(storage);
    +    addNewStoragesToRootSchema(schemaConfig, rootSchema, storageSet);
    +    schemaTreesToClose.add(rootSchema);
    +    return rootSchema;
    +  }
    +
    +  public SchemaPlus addPartialRootSchema(final String userName, final 
SchemaConfigInfoProvider provider,
    +                                            Set<String> storages, 
SchemaPlus rootSchema) {
    +    final String schemaUser = isImpersonationEnabled ? userName : 
ImpersonationUtil.getProcessUserName();
    +    final SchemaConfig schemaConfig = SchemaConfig.newBuilder(schemaUser, 
provider).build();
    +    addNewStoragesToRootSchema(schemaConfig, rootSchema, storages);
    +    schemaTreesToClose.add(rootSchema);
    +    return rootSchema;
    +  }
    +
    +  private void expandSecondLevelSchema(SchemaPlus parent) {
    +    List<SchemaPlus> secondLevelSchemas = Lists.newArrayList();
    +    for (String firstLevelSchemaName : parent.getSubSchemaNames()) {
    +      SchemaPlus firstLevelSchema = 
parent.getSubSchema(firstLevelSchemaName);
    +      for (String secondLevelSchemaName : 
firstLevelSchema.getSubSchemaNames()) {
    +        
secondLevelSchemas.add(firstLevelSchema.getSubSchema(secondLevelSchemaName));
    +      }
    +    }
    +
    +    for (SchemaPlus schema : secondLevelSchemas) {
    +      AbstractSchema drillSchema;
    +      try {
    +        drillSchema = schema.unwrap(AbstractSchema.class);
    +      } catch (ClassCastException e) {
    +        throw new RuntimeException(String.format("Schema '%s' is not 
expected under root schema", schema.getName()));
    +      }
    +      SubSchemaWrapper wrapper = new SubSchemaWrapper(drillSchema);
    +      parent.add(wrapper.getName(), wrapper);
    +    }
    +  }
    +
    +  public void addNewStoragesToRootSchema(SchemaConfig schemaConfig, 
SchemaPlus rootSchema, Set<String> storages) {
    +    try {
    +      for(String name: storages) {
    +        try {
    +          StoragePlugin plugin = dContext.getStorage().getPlugin(name);
    +          if(plugin == null) {
    +            logger.error("Got null for storage plugin of name: " + name);
    +            continue;
    +          }
    +          plugin.registerSchemas(schemaConfig, rootSchema);
    +          expandSecondLevelSchema(rootSchema);
    +        }
    +        catch (ExecutionSetupException ex) {
    --- End diff --
    
    Presumably this method adds storages based on need: we add a storage only 
if a query references it. If so, then we need to pass the exception along. If 
we don't, the user will simply be told that "foo.bar" does not exist when, in 
fact, perhaps it does exist but is configured wrong.
    
    And, note that we do catch an IOException and translate it to a user 
exception below.
    
    So, maybe
    
    * Remove this try/catch block.
    * Change the following catch from IOException to just Exception.
    
    The result will be that we fail the query for all errors, not just IO 
errors. Again, this is the right choice if we are only loading the schemas 
needed by the query.


> Skip initializing all enabled storage plugins for every query
> -------------------------------------------------------------
>
>                 Key: DRILL-5089
>                 URL: https://issues.apache.org/jira/browse/DRILL-5089
>             Project: Apache Drill
>          Issue Type: Improvement
>          Components: Query Planning & Optimization
>    Affects Versions: 1.9.0
>            Reporter: Abhishek Girish
>            Assignee: Chunhui Shi
>            Priority: Critical
>
> In a query's lifecycle, at attempt is made to initialize each enabled storage 
> plugin, while building the schema tree. This is done regardless of the actual 
> plugins involved within a query. 
> Sometimes, when one or more of the enabled storage plugins have issues - 
> either due to misconfiguration or the underlying datasource being slow or 
> being down, the overall query time taken increases drastically. Most likely 
> due the attempt being made to register schemas from a faulty plugin.
> For example, when a jdbc plugin is configured with SQL Server, and at one 
> point the underlying SQL Server db goes down, any Drill query starting to 
> execute at that point and beyond begin to slow down drastically. 
> We must skip registering unrelated schemas (& workspaces) for a query. 



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Reply via email to