| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
@gflores-jahia Thank you for your work on this! I convert it to draft: ping me when it will be ready for a final review! |
Sorry, something went wrong.
|
@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 |
Sorry, something went wrong.
|
@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 |
Sorry, something went wrong.
|
I've made a deep analysis of the codebase and I confirmed the presence of a thread/memory leak in the OsgiGraphQLHttpServlet. To find a clean solution to this issue, I've investigated the current flow in the GraphQLHttpServlet. When the servlet receives an HTTP request, the doRequest method is called, which contains the following call chain: return getConfiguration().getHttpRequestHandler().handle(request, response);Here's a breakdown of the process:
The main issue with the OsgiGraphQLHttpServlet's behavior is that the getConfiguration method calls OsgiSchemaBuilder#buildConfiguration() instead of returning an already created configuration instance. This PR introduces:
Now that I have a better understanding of the process, I wouldn't remove the implicit ThreadPool creation because, if the GraphQLConfiguration is correctly created (as in the GraphQLHttpServlet and OsgiGraphQLHttpServlet, thanks to this PR), it will be automatically handled by the GraphQLConfiguration itself. Additionally, the GraphQLConfiguration#with(Executor asyncExecutor) method allows developers to pass their custom executor. My only remaining doubt is when the GraphQLConfiguration created by the GraphQLConfigurationProvider is built. I've only seen the with methods of the Builder, but I haven't seen the build method that creates the GraphQLConfiguration object: is the GraphQLConfiguration.Builder#build() call delegated to something else? |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
WORK IN PROGRESS
Creating new PR based off of #538 and branched off latest master code.