jnturton commented on a change in pull request #2388:
URL: https://github.com/apache/drill/pull/2388#discussion_r781226257
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/SchemaTreeProvider.java
##########
@@ -154,17 +156,23 @@ public SchemaPlus createFullRootSchema(SchemaConfig
schemaConfig) {
.build(logger);
}
}
+ */
@Override
public void close() throws Exception {
+ //TODO jnturton: clean up
+ /*
List<AutoCloseable> toClose = Lists.newArrayList();
for(SchemaPlus tree : schemaTreesToClose) {
addSchemasToCloseList(tree, toClose);
}
AutoCloseables.close(toClose);
+ */
}
+ //TODO jnturton: clean up
+ /*
private static void addSchemasToCloseList(final SchemaPlus tree, final
List<AutoCloseable> toClose) {
for(String subSchemaName : tree.getSubSchemaNames()) {
addSchemasToCloseList(tree.getSubSchema(subSchemaName), toClose);
Review comment:
@vvysotskyi under this PR, the construction of the schema tree becomes
lazy since createFullRootSchema is avoided. The code at this line
unfortunately undoes the benefits by eagerly constructing the entire schema
tree only in order to be able to close schema objects. I am not happy about
skipping calls to `close()` on schemas, do you think I should look to add code
to DynamicRootSchemas that returns only schemas that have already been
constructed and initialised so that the code here has a suitable collection to
close? E.g. the `getSubSchemaNames()`, which returns every storage config
name, gets replaced with something like `getInitialisedSubSchemas()`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@drill.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
