You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@uniffle.apache.org by GitBox <gi...@apache.org> on 2022/09/02 09:51:40 UTC
[GitHub] [incubator-uniffle] kaijchen opened a new pull request, #197: Synchronise read access to ShuffleBuffer#size
kaijchen opened a new pull request, #197:
URL: https://github.com/apache/incubator-uniffle/pull/197
### What changes were proposed in this pull request?
Synchronise read access to ShuffleBuffer#size.
### Why are the changes needed?
To make synchronization consistent.
Because all write accesses to `ShuffleBuffer#size` are synchronised.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
CI.
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] kaijchen closed pull request #197: Synchronise read access to ShuffleBuffer#size
Posted by GitBox <gi...@apache.org>.
kaijchen closed pull request #197: Synchronise read access to ShuffleBuffer#size
URL: https://github.com/apache/incubator-uniffle/pull/197
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] codecov-commenter commented on pull request #197: Synchronise read access to ShuffleBuffer#size
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #197:
URL: https://github.com/apache/incubator-uniffle/pull/197#issuecomment-1235327799
# [Codecov](https://codecov.io/gh/apache/incubator-uniffle/pull/197?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#197](https://codecov.io/gh/apache/incubator-uniffle/pull/197?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3908a6d) into [master](https://codecov.io/gh/apache/incubator-uniffle/commit/331ebb2a7cd12dfa5d780fc4b0b7d11e9917d343?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (331ebb2) will **decrease** coverage by `0.03%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #197 +/- ##
============================================
- Coverage 58.41% 58.38% -0.04%
+ Complexity 1273 1272 -1
============================================
Files 158 158
Lines 8448 8448
Branches 784 784
============================================
- Hits 4935 4932 -3
- Misses 3260 3262 +2
- Partials 253 254 +1
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-uniffle/pull/197?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...rg/apache/uniffle/server/buffer/ShuffleBuffer.java](https://codecov.io/gh/apache/incubator-uniffle/pull/197/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL3NlcnZlci9idWZmZXIvU2h1ZmZsZUJ1ZmZlci5qYXZh) | `92.72% <ø> (ø)` | |
| [...org/apache/uniffle/server/ShuffleFlushManager.java](https://codecov.io/gh/apache/incubator-uniffle/pull/197/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL3NlcnZlci9TaHVmZmxlRmx1c2hNYW5hZ2VyLmphdmE=) | `76.66% <0.00%> (-1.67%)` | :arrow_down: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] frankliee commented on pull request #197: Synchronise read access to ShuffleBuffer#size
Posted by GitBox <gi...@apache.org>.
frankliee commented on PR #197:
URL: https://github.com/apache/incubator-uniffle/pull/197#issuecomment-1246292616
Do you find any bugs ?
It seems that there is no multi-threaded conflicts because the two APIs only read a long type.
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] frankliee commented on pull request #197: Synchronise read access to ShuffleBuffer#size
Posted by GitBox <gi...@apache.org>.
frankliee commented on PR #197:
URL: https://github.com/apache/incubator-uniffle/pull/197#issuecomment-1246307602
> For example, is is possible to read stale `size` if context switch happens before line 66:
>
> https://github.com/apache/incubator-uniffle/blob/ec00ad91c72dc23b3e81b5d9abe815438b0f5bcf/server/src/main/java/org/apache/uniffle/server/buffer/ShuffleBuffer.java#L61-L67
Reading long type (x86_64 word) is already atomic with or without synchronized.
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] kaijchen commented on pull request #197: Synchronise read access to ShuffleBuffer#size
Posted by GitBox <gi...@apache.org>.
kaijchen commented on PR #197:
URL: https://github.com/apache/incubator-uniffle/pull/197#issuecomment-1246606201
> Like discussion at dev mail list, we will freeze the code and cut 0.6 version branch in September 15, we will not merge this pr before I cut 0.6 version branch, are you ok?
No problem. As it's not causing any issue right now.
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] kaijchen commented on pull request #197: Synchronise read access to ShuffleBuffer#size
Posted by GitBox <gi...@apache.org>.
kaijchen commented on PR #197:
URL: https://github.com/apache/incubator-uniffle/pull/197#issuecomment-1246298592
For example, is is possible to read stale `size` if context switch happens before line 66:
https://github.com/apache/incubator-uniffle/blob/ec00ad91c72dc23b3e81b5d9abe815438b0f5bcf/server/src/main/java/org/apache/uniffle/server/buffer/ShuffleBuffer.java#L61-L67
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on pull request #197: Synchronise read access to ShuffleBuffer#size
Posted by GitBox <gi...@apache.org>.
jerqi commented on PR #197:
URL: https://github.com/apache/incubator-uniffle/pull/197#issuecomment-1246587630
Like discussion at dev mail list, we will freeze the code and cut 0.6 version branch in September 15, we will not merge this pr before I cut 0.6 version branch, are you ok?
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] kaijchen commented on pull request #197: Synchronise read access to ShuffleBuffer#size
Posted by GitBox <gi...@apache.org>.
kaijchen commented on PR #197:
URL: https://github.com/apache/incubator-uniffle/pull/197#issuecomment-1246310697
> Reading long type (x86_64 word) is already atomic with or without synchronized.
Here `size` means the sum of size of all blocks.
It is possible that when you read it, that `blocks` has been updated while `size` hasn't.
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org