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

templating #25

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

templating #25

wants to merge 1 commit into from

Conversation

auriium
Copy link
Contributor

@auriium auriium commented Apr 10, 2021

I'd like to add a feature like this

CentralisedFuture<Transaction> provideTransact();

to the code. I'm fairly sure my current implementation is wrong or bad or something bad will be found about it, but the point here is not top notch code, but to show that a feature is required since i'm not sure if i can implement it myself

This feature would be useful in the context of methods that provide completablefutures and use the completablefuture's builder style methods as you can do something like

future.thenApply(transaction -> { //do something safely with transaction }).thenCombine(// another future that starts with a thenApply)

This would make writing api-using code a lot less tedious, although i do not know of the safety of this method since someone could probably block and cache the transaction in memory causing pain and suffering, so a safer implementation of this is probably in order

@auriium
Copy link
Contributor Author

auriium commented Apr 10, 2021

I mean it's kind of just for what if you want to do transactions somewhere in the middle and not at the end of a block of connected futures?

@A248
Copy link
Contributor

A248 commented Apr 11, 2021

The problem with this concept is that the DataCenter is responsible for closing the transaction when it's finished.

When a Function<Transaction, R> or Consumer is used, the DataCenter can pass the Transaction to the closure, close the transaction, and return the resulting future. This ensures a safe lifetime of the transaction, the only potential for mishap being if someone were to store the Transaction in a field or return the Transaction from the Function (as you are doing in the implementation of this PR)

Another idea would be to have the caller close the Transaction. However, that would be error-prone as the caller would need to ensure Transaction.close is called somewhere in the calling code. With chained CompletableFutures, it is easy for this to go wrong. Moreover, AutoCloseable would help little here - there is no such thing as an asynchronous version of try-with-resources.

@auriium
Copy link
Contributor Author

auriium commented Apr 11, 2021

The problem with this concept is that the DataCenter is responsible for closing the transaction when it's finished.

When a Function<Transaction, R> or Consumer is used, the DataCenter can pass the Transaction to the closure, close the transaction, and return the resulting future. This ensures a safe lifetime of the transaction, the only potential for mishap being if someone were to store the Transaction in a field or return the Transaction from the Function (as you are doing in the implementation of this PR)

Another idea would be to have the caller close the Transaction. However, that would be error-prone as the caller would need to ensure Transaction.close is called somewhere in the calling code. With chained CompletableFutures, it is easy for this to go wrong. Moreover, AutoCloseable would help little here - there is no such thing as an asynchronous version of try-with-resources.

I did note the issue of the transaction being readily available. I have a solution for that, and it is a CompletableFuture implementation that can not perform a blocking get / join, which would keep the user from storing references to the transaction. However, the issue of transactions being closable is another issue which i have to address, which i will do inside of beetle - I'm planning on reimplementing the future interface from scratch for visibility purposes and will create a ClosableFuture implementation.

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