You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@seata.apache.org by "jihun4452 (via GitHub)" <gi...@apache.org> on 2025/08/17 16:18:41 UTC
[PR] bugfix: clamp getOrder to avoid tie/overflow [incubator-seata]
jihun4452 opened a new pull request, #7587:
URL: https://github.com/apache/incubator-seata/pull/7587
Fixes #7579
- [x] I have read the CONTRIBUTING.md guidelines.
- [x] I have registered the PR changes.
### Ⅰ. Describe what this PR did
- Normalize `AdapterSpringSeataInterceptor#getOrder()`:
- Prefer the explicit order set via `setOrder(...)` to avoid overflow artifacts from the proxy.
- Clamp values `>= Ordered.LOWEST_PRECEDENCE` to `Integer.MAX_VALUE - 1` to avoid a tie with Spring’s `TransactionInterceptor`.
- No public API change; only edge-case ordering is adjusted.
### Ⅱ. Does this pull request fix one issue?
Yes.
### Ⅲ. Why don't you add test cases (unit test/integration test)?
Added:
- `order_should_be_less_than_lowest_precedence`
- `order_should_passthrough_when_not_lowest`
### Ⅳ. Describe how to verify it
Run: `./mvnw -pl spring -am -Dtest=AdapterSpringSeataInterceptorTest test -DfailIfNoTests=false`
Env: macOS, JDK 17, Maven 3.9.x → BUILD SUCCESS locally.
### Ⅴ. Special notes for reviews
Some environments return an overflowed (negative) order from `ProxyInvocationHandler#getOrder()`, which broke the UT.
Preferring the explicit order and clamping the max value stabilizes execution precedence and avoids a tie with `TransactionInterceptor`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] bugfix: clamp getOrder to avoid tie/overflow [incubator-seata]
Posted by "YongGoose (via GitHub)" <gi...@apache.org>.
YongGoose commented on PR #7587:
URL: https://github.com/apache/incubator-seata/pull/7587#issuecomment-3194906461
In the code below, the execution order of the `Seata interceptor` has already been adjusted to be one lower than Spring’s `TransactionInterceptor`.
- https://github.com/apache/incubator-seata/blob/b0566047717a485ba0b62e16d8724d50ffe77e6c/spring/src/main/java/org/apache/seata/spring/annotation/GlobalTransactionScanner.java#L449
That’s why I don’t think this change is necessary.
Personally, I also prefer keeping the code clear.
If we end up having two different `order` values, it might make future maintenance more complicated.
WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] bugfix: clamp getOrder to avoid tie/overflow [incubator-seata]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #7587:
URL: https://github.com/apache/incubator-seata/pull/7587#issuecomment-3194494442
## [Codecov](https://app.codecov.io/gh/apache/incubator-seata/pull/7587?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `66.66667%` with `1 line` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 61.38%. Comparing base ([`b056604`](https://app.codecov.io/gh/apache/incubator-seata/commit/b0566047717a485ba0b62e16d8724d50ffe77e6c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`60156f2`](https://app.codecov.io/gh/apache/incubator-seata/commit/60156f2231183ff93774b7d515497ed5fabc73c5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/incubator-seata/pull/7587?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...ring/annotation/AdapterSpringSeataInterceptor.java](https://app.codecov.io/gh/apache/incubator-seata/pull/7587?src=pr&el=tree&filepath=spring%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Fspring%2Fannotation%2FAdapterSpringSeataInterceptor.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3ByaW5nL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zZWF0YS9zcHJpbmcvYW5ub3RhdGlvbi9BZGFwdGVyU3ByaW5nU2VhdGFJbnRlcmNlcHRvci5qYXZh) | 66.66% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/incubator-seata/pull/7587?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## 2.x #7587 +/- ##
=========================================
Coverage 61.38% 61.38%
Complexity 658 658
=========================================
Files 1310 1310
Lines 49578 49580 +2
Branches 5836 5837 +1
=========================================
+ Hits 30431 30435 +4
+ Misses 16428 16427 -1
+ Partials 2719 2718 -1
```
| [Files with missing lines](https://app.codecov.io/gh/apache/incubator-seata/pull/7587?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...ring/annotation/AdapterSpringSeataInterceptor.java](https://app.codecov.io/gh/apache/incubator-seata/pull/7587?src=pr&el=tree&filepath=spring%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Fspring%2Fannotation%2FAdapterSpringSeataInterceptor.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3ByaW5nL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zZWF0YS9zcHJpbmcvYW5ub3RhdGlvbi9BZGFwdGVyU3ByaW5nU2VhdGFJbnRlcmNlcHRvci5qYXZh) | `78.57% <66.66%> (+20.23%)` | :arrow_up: |
... and [5 files with indirect coverage changes](https://app.codecov.io/gh/apache/incubator-seata/pull/7587/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
[](https://app.codecov.io/gh/apache/incubator-seata/pull/7587?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] bugfix: clamp getOrder to avoid tie/overflow [incubator-seata]
Posted by "jihun4452 (via GitHub)" <gi...@apache.org>.
jihun4452 commented on PR #7587:
URL: https://github.com/apache/incubator-seata/pull/7587#issuecomment-3194928465
Thanks for pointing it out. I didn’t realize it was already covered. I’ll close this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] bugfix: clamp getOrder to avoid tie/overflow [incubator-seata]
Posted by "jihun4452 (via GitHub)" <gi...@apache.org>.
jihun4452 closed pull request #7587: bugfix: clamp getOrder to avoid tie/overflow
URL: https://github.com/apache/incubator-seata/pull/7587
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org