Fixes for OSGi, remove automatic creation of executor for async requests (WIP) by gflores-jahia · Pull Request #569 · graphql-java-kickstart/graphql-java-servlet · GitHub
Skip to content

Fixes for OSGi, remove automatic creation of executor for async requests (WIP)#569

Draft
gflores-jahia wants to merge 4 commits into
graphql-java-kickstart:masterfrom
Jahia:osgi-fixes-and-improvements-latest
Draft

Fixes for OSGi, remove automatic creation of executor for async requests (WIP)#569
gflores-jahia wants to merge 4 commits into
graphql-java-kickstart:masterfrom
Jahia:osgi-fixes-and-improvements-latest

Conversation

@gflores-jahia

Copy link
Copy Markdown
Contributor

WORK IN PROGRESS

Creating new PR based off of #538 and branched off latest master code.

@federicorispo

Copy link
Copy Markdown
Member

@federicorispo federicorispo self-requested a review May 25, 2024 12:13
@federicorispo federicorispo marked this pull request as draft May 25, 2024 12:14
@gflores-jahia

Copy link
Copy Markdown
Contributor Author

@federicorispo I wonder if you have an opinion regarding the original PR fix of removing ThreadPoolExecutor completely. It looks like this might be a breaking change.

It looks like that we no longer have a default async executor, and users will need to provide one if they want to handle async tasks. But because configuration and HttpInvoker is still created for every request, this to me still doesn't fix the original issue of being able to clean up thread pools. Instead of completely removing the default ThreadPoolExecutor, I wonder if you have ideas on how to properly clean them after execution.

In the meantime will need to put this on hold until we get a good consensus of proper way to fix ThreadPools constantly being created on every http request

@federicorispo

Copy link
Copy Markdown
Member

@gflores-jahia I will look in the current flow of the request handling to see if there is a clean and solid way to better handle thread after the execution. A good refactor is certainly needed

@federicorispo

federicorispo commented Jul 28, 2024

Copy link
Copy Markdown
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants