-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Potential throwing of the blocking exception #2025
Comments
where exactly in which class? |
class ReactiveElasticsearchTemplate: line 805: Function<SearchDocument, Object> entityCreator = searchDocument -> documentCallback.toEntity(searchDocument)
line 806: .block(); |
I'll have to have a deeper look at this callback, but I think that the reactive version of this callback is not needed here anyway and this could be a non-reactive call. |
Maybe change the callback to a non-reactive version? I can do it. |
The non-reactive code has some callback for this as well, this should be used or consolidated into common code. |
The callback ReadSearchDocumentCallback has a call to the maybeCallAfterConvert public API.
Wrapper wr = toEntity(...);
maybeCallAfterConvert(wr.p1(), wr.p2(),.....); |
I'll hopefully have a look at this deeper in the next days currently I'm quite busy (not doing this as my day job 😃 ) |
I'am know that is a free time job. Me too. I have the opportunity to help you develop this project. |
seems the best would be like you suggested to have the |
If you don't mind, i'll take this problem |
that has to be done with the introduction of the new client. I have a branch in my fork where I am working on this; it is planned for the version 4.4 to be able to use either the I can really work on this since Elasticsearch 7.16 is out, since then the new client is fairly stable, before that, there were many changes in packages, structure and functionality. This also means that we are writing a new reactive template and reactive client as well because we need to use the same request and response classes that the Elasticsearch client uses. For the existing code we had copies/adaptions of code from the Elasticsearch codebase, we need to get rid of that. So currently I am building a code structure where we for both the imperative and the reactive code we have implementations using the old Elasticsearch libraries and copied code or the new one. In addition to that, that will enable Spring Data Elasticsearch to eventually have other client implementations in the future (OpenSearch might be one). So this issue with the document callback needs to be addressed in this whole process of restructuring the code. Changing this now in the current code will need refactoring later. So I am not sure where you could currently help here. |
In the DefaultReactiveElasticsearch class there is a call to block(). This could potentially cause the IllegalStateException("block()/blockFirst()/blockLast()...) to be thrown. This will be thrown into EventLoop of the Netty Server.
Can make a reactive implementation of the SearchDocumentResponse to remove the blocking call?
The text was updated successfully, but these errors were encountered: