Skip to content
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

Considering making the Completion interface extend AutoCloseable #247

Open
thonra opened this issue Dec 3, 2024 · 2 comments
Open

Considering making the Completion interface extend AutoCloseable #247

thonra opened this issue Dec 3, 2024 · 2 comments

Comments

@thonra
Copy link

thonra commented Dec 3, 2024

Background and Intention

When using asynchronous processing completion, if a completion leak occurs, the system will be unable to consume furthur records which is quite dangerous. Therefore, avoiding completion leaks during development is a critical issue that requires attention.

However, in the context of collaboration and maintenance, as the complexity of the codebase increases and maintainers change over time, the likelihood of developers overlooking completion leaks also rises (e.g., complete being called across different locations in the call chain). I am considering whether there is a way to increase developers' awareness of completion leaks or to automatically check them.

Considerations

Perhaps one of the most effective approaches would be for developers to perform AST-based code analysis on their respective projects and integrate it into the CI pipeline. However, this is not something Decaton project can guarantee, and due to the cost, it is often not carried out.

Therefore, I am considering whether Completion could be annotated at the code level as something that must be completed, or whether it could be automatically completed at the appropriate time (though I haven't thought of a reasonable implementation for this).

Eventually, one idea I have is for Completion to extend the AutoCloseable interface. While this doesn't have any practical function, it would allow most developers' IDEs to detect the implementation of the AutoCloseable interface through their inspector tools, triggering warnings and serving as a pre-warning stuff.

I’d like to hear your thoughts on this. Thank you!

While this doesn't have any practical function ->

  • In sync complete scenarios, after the process method returns, the completion will be completed by decaton, so there is no need to control the completion process
  • In async complete scenarios, the completion is often completed during a callback, and completing it prematurely can cause issues.
@ocadaruma
Copy link
Member

Thank you for the feedback.

the likelihood of developers overlooking completion leaks also rises

100% agree, and that's the exact reason we introduced deferred-completion timeout (#96 (comment), #99).

Eventually, one idea I have is for Completion to extend the AutoCloseable interface.

IMO this doesn't make much sense, because IIRC IDE (Intellij at least) can only detect the missing close() for very simple case, and also, to warn that, AutoCloseable must be instantiated by user's code (rather than retrieving from ProcessingContext).

Another option to avoid deferred completion leak is to use virtual threads. (#224)
As you may know, as long as we don't defer completions, it is guaranteed to be completed by Decaton runtime when DecatonProcessor#process returns.

By using virtual threads, users can make processors "async" (i.e. doesn't occupy OS thread) while implementing processors in "synchronous" manner.

Does deferred-completion-timeout or virtual-threads help for your case?

@thonra
Copy link
Author

thonra commented Dec 4, 2024

Thanks for the discussions provided, giving me a great context of the efforts made by decaton team in resolving this issue.

Agree that virtual threads are the ultimate solution to this problem. 👍
(Though in practice, since virtual thread is officially introduced in JDK 21, it may be challenging for production projects with numerous upstream and downstream dependencies to upgrade to JDK 21 in the short to medium-long term)

For the timeout, when the frequency of completion leaks is low, they indeed have a good mitigating effect.

Thank you very much for your insights, especially the constructive approach of using virtual threads. I think virtual thread is the future of async programming in Java and worth being tried by projects.

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

No branches or pull requests

2 participants