Transaction improvements + use new propagation context API #2189
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dstepanov
changed the title
dstepanov
changed the title
graemerocher
pushed a commit
to micronaut-projects/micronaut-core
that referenced
this pull request
May 17, 2023
Initial version of the propagation API. Main points: - The propagated context is always immutable and consist of different propagated elements similar to Kotlin Coroutines context - Propagated element can implement `ThreadPropagatedContextElement` to setup/restore thread locals - The propagated state is never captured but the context needs to be extended and pushed down the downstream - The capturing API is not needed anymore, this should eliminate a lot of overhead from the reactive code - The context is automatically propagated to the Kotlin coroutines - In the Reactor context should be propagated as whole with utility method to extract/extend it (The Reactor context needs to be modified manually) - The implementation is using try-resources to propagate the contest without extra overhead, this might change in the future to support scoped local, but at this moment, I think this is the best solution. I want to merge this first PR to have the orther work mergable. Next PRs will remove the existing instrumentation propagation, add docs etc. There are already examples how it should be used: - Tracing micronaut-projects/micronaut-tracing#281 - Micronaut Data micronaut-projects/micronaut-data#2189 (This is a big PR because of the changes in the TX management and removal of forked Spring TX code)
dstepanov
force-pushed
the
prop
branch
2 times, most recently
from
3044125
to
3021d01
Compare
|
@radovanradic Would you have any idea why |
Investigated it here #2245, not sure if this fix makes sense |
|
I need to exclude Sonar from the TCK projects as it includes warning for the incorrect usecases |
dstepanov
requested review from
graemerocher and
radovanradic
and removed request for
graemerocher
graemerocher
requested changes
Jun 6, 2023
...main/java/io/micronaut/data/connection/jdbc/exceptions/CannotGetJdbcConnectionException.java
Outdated
Show resolved
Hide resolved
data-connection/src/main/java/io/micronaut/data/connection/ConnectionOperationsRegistry.java
Show resolved
Hide resolved
data-connection/src/main/java/io/micronaut/data/connection/annotation/Connection.java
Outdated
Show resolved
Hide resolved
data-connection/src/main/java/io/micronaut/data/connection/annotation/Connection.java
Outdated
Show resolved
Hide resolved
...connection/src/main/java/io/micronaut/data/connection/interceptor/ConnectionInterceptor.java
Outdated
Show resolved
Hide resolved
data-tx/src/main/java/io/micronaut/transaction/TransactionExecution.java
Outdated
Show resolved
Hide resolved
| // prepareTransactionalConnection(connection, definition); | ||
|
|
||
| if (!onComplete.isEmpty()) { | ||
| Collections.reverse(onComplete); |
There was a problem hiding this comment.
can we avoid having to reverse this collection
data-tx-jdbc/src/main/java/io/micronaut/transaction/jdbc/DataSourceTransactionStatus.java
Show resolved
Hide resolved
| @@ -6,7 +6,7 @@ | |||
|
|
|||
| import static org.junit.jupiter.api.Assertions.*; | |||
|
|
|||
| @MicronautTest | |||
| @MicronautTest(transactional = false) | |||
There was a problem hiding this comment.
why was this disabled?
Comment on lines
-13
to
14
| private final SynchronousTransactionManager<Connection> transactionManager; | ||
| private final TransactionOperations<Session> transactionManager; | ||
|
|
There was a problem hiding this comment.
need to add more documentation to breaks about this change and how programmatic transactions have changed
There was a problem hiding this comment.
The previous way is still supported (SynchronousTransactionManager<Session>) but I would like to eliminate SynchronousTransactionManager because of the detached TX is not the best for propagation
graemerocher
approved these changes
Jun 6, 2023
graemerocher
approved these changes
Jun 6, 2023
|
SonarCloud Quality Gate failed. |
1 task
timyates
added a commit
to micronaut-projects/micronaut-crac
that referenced
this pull request
Jun 19, 2023
timyates
added a commit
to micronaut-projects/micronaut-crac
that referenced
this pull request
Jun 19, 2023
…v4.0.0-m9 (#134) * fix(deps): update dependency io.micronaut.data:micronaut-data-bom to v4.0.0-m9 * Fix for Data M9. Classes were moved in micronaut-projects/micronaut-data#2189 --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Tim Yates <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR:
@Connectionannotation that works in a similar way as@Transaction, allowing to have one open connection/session for multiple repository operations (The annotation is conflicting with JDBCConnection, maybe it should be something else like@WithConnectionWDYT?)Resolves #1875