Parallel Task Catalog Loading #20446
Conversation
jklamer
force-pushed
the
jklamer/ParrallelTaskCatalogLoading
branch
from
9742d10 to
0b3a4e2
Compare
core/trino-main/src/main/java/io/trino/connector/WorkerDynamicCatalogManager.java
Outdated
Show resolved
Hide resolved
core/trino-main/src/main/java/io/trino/connector/WorkerDynamicCatalogManager.java
Outdated
Show resolved
Hide resolved
core/trino-main/src/main/java/io/trino/connector/WorkerDynamicCatalogManager.java
Outdated
Show resolved
Hide resolved
jklamer
force-pushed
the
jklamer/ParrallelTaskCatalogLoading
branch
from
0b3a4e2 to
bd4afb8
Compare
core/trino-main/src/main/java/io/trino/connector/WorkerDynamicCatalogManager.java
Outdated
Show resolved
Hide resolved
core/trino-main/src/main/java/io/trino/connector/WorkerDynamicCatalogManager.java
Outdated
Show resolved
Hide resolved
core/trino-main/src/main/java/io/trino/connector/WorkerDynamicCatalogManager.java
Outdated
Show resolved
Hide resolved
| executor)) | ||
| .collect(toImmutableList())); | ||
|
|
||
| var ignored = catalogsShutDown.run(() -> |
There was a problem hiding this comment.
Don't assign to a variable if it isn't going to be used.
There was a problem hiding this comment.
This was the result of some error prone check with this as the suggested fix. I'll see what I can do to remove.
There was a problem hiding this comment.
With the changing of the logging structure I just made each shutdown async and I block on none of them. Let me know if that's a little too radical or not.
There was a problem hiding this comment.
oh it still yelled even when submitting inside a forEach. Gonna try the annotation approach
There was a problem hiding this comment.
I had to do something very silly here. Let me know what you think.
core/trino-main/src/main/java/io/trino/connector/WorkerDynamicCatalogManager.java
Outdated
Show resolved
Hide resolved
core/trino-main/src/main/java/io/trino/connector/WorkerDynamicCatalogManager.java
Outdated
Show resolved
Hide resolved
jklamer
force-pushed
the
jklamer/ParrallelTaskCatalogLoading
branch
from
bd4afb8 to
4518e4e
Compare
core/trino-main/src/main/java/io/trino/connector/WorkerDynamicCatalogManager.java
Show resolved
Hide resolved
jklamer
force-pushed
the
jklamer/ParrallelTaskCatalogLoading
branch
2 times, most recently
from
f8068f8 to
7d451ca
Compare
core/trino-main/src/main/java/io/trino/connector/WorkerDynamicCatalogManager.java
Outdated
Show resolved
Hide resolved
| catalogLoadingExceptions.add(e); | ||
| } | ||
| } | ||
| catch (TimeoutException | InterruptedException e) { |
There was a problem hiding this comment.
The thread interrupted state is being swallowed here. It needs to be restored or the method needs to throw an exception.
There was a problem hiding this comment.
Not sure I know enough about restoring the interrupted thread here to know how I might do that. Right now the behavior is to put the exception in the list and throw if there are any exceptions loading any of the catalogs. Is that sub optimal? Or should I just throw immediately in this case (and with what runtime exception)?
There was a problem hiding this comment.
Believe this is handled now.
jklamer
force-pushed
the
jklamer/ParrallelTaskCatalogLoading
branch
from
7d451ca to
fd47661
Compare
jklamer
force-pushed
the
jklamer/ParrallelTaskCatalogLoading
branch
from
fd47661 to
0d3acbd
Compare
jklamer
force-pushed
the
jklamer/ParrallelTaskCatalogLoading
branch
from
0d3acbd to
d83f65f
Compare
Description
Make Catalog loading for tasks
Additional context and related issues
Release notes
( X) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text: