Synchronise read access to ShuffleBuffer#size

[kaijchen]

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.

[codecov-commenter]

Codecov Report

Merging #197 (3908a6d) into master (331ebb2) will decrease coverage by 0.03%. The diff coverage is n/a.

@@             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	Coverage Δ
...rg/apache/uniffle/server/buffer/ShuffleBuffer.java	92.72% <ø> (ø)
...org/apache/uniffle/server/ShuffleFlushManager.java	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

[frankliee]

Do you find any bugs ? It seems that there is no multi-threaded conflicts because the two APIs only read a long type.

[kaijchen]

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

[frankliee]

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.

[kaijchen]

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.

[jerqi]

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?

[kaijchen]

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.