[Bug] Fix potenial bug when the index reading offset is greater than data length

[zuston]

What changes were proposed in this pull request?

Fix potenial bug when the index reading offset equals to data length

Why are the changes needed?

If the reading length == data length in FixedSizeSegmentSplitter, it should involve this block. And if its length > data length. it should abort it.

Does this PR introduce any user-facing change?

No

How was this patch tested?

UTs

[zuston]

@leixm Could you help check whether this is a bug? CC @jerqi

[leixm]

Here's an extra judgment.

if (bufferOffset > 0) {
      ShuffleDataSegment sds = new ShuffleDataSegment(fileOffset, bufferOffset, bufferSegments);
      dataFileSegments.add(sds);
    }

[zuston]

You could use the test case to check this @leixm . It will fail.

[jerqi]

cc @kaijchen

[jerqi]

Do you find this problem in your online environment? Maybe we should also add some logs.

[leixm]

You could use the test case to check this @leixm . It will fail.

You're right, it's a potenial bug.

[zuston]

Do you find this problem in your online environment?

No. I just found this bug when browsing recent commits.

Maybe we should also add some logs.

Good ideas. I will add some logs

[codecov-commenter]

Codecov Report

Merging #320 (56a0026) into master (1b53455) will increase coverage by 0.06%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master     #320      +/-   ##
============================================
+ Coverage     61.07%   61.13%   +0.06%     
- Complexity     1493     1497       +4     
============================================
  Files           185      185              
  Lines          9327     9329       +2     
  Branches        903      903              
============================================
+ Hits           5696     5703       +7     
+ Misses         3326     3322       -4     
+ Partials        305      304       -1     

Impacted Files	Coverage Δ
...iffle/common/segment/FixedSizeSegmentSplitter.java	95.55% <100.00%> (+5.31%)	⬆️
...che/uniffle/client/impl/ShuffleReadClientImpl.java	90.09% <0.00%> (+2.72%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[kaijchen]

Thanks @zuston for the improvement, this change looks good.
Please remove the [WIP] tag in title if there is nothing to add.

[jerqi]

LGTM @leixm @kaijchen Do you have any other suggestion?

[kaijchen]

LGTM

[leixm]

LGTM.

[jerqi]

Merged. Thanks @zuston @kaijchen @leixm