Fix flaky test TieredSpilloverCacheTests.testComputeIfAbsentConcurrently

[sgup432]

Description

Fixes flaky test.
We had added the logic as part of #14187 to move query recompute logic outside write lock. We used a Map<Key, Future> so that the first thread adds a future to the map, loads the value and rest threads wait on the future via future.get(). And then we remove the key from the map from the thread which was responsible to load the value.

After this change testComputeIfAbsentConcurrently became flaky and fails in certain scenarios where we expected that the value was loaded only once but in actual it was more than more. This happened in below scenario:

1. Thread 1 puts the future into map completeableFutureMap for key A
2. Thread 1 loads the value, takes write lock and puts the value onto onHeap cache. Release the lock.
3. Now Thread 2 comes with request for key A, tries a get() and finds null value.
4. Meanwhile Thread 1 removes the key A from map completeableFutureMap
5. Thread 2 doesn't find key A in completeableFutureMap and again loads the value. Test fails after this point.

I reproduced this by running testComputeIfAbsentConcurrently 100 times and saw it failing pretty often. To verify the fix, again ran it 100 times and it passed in all cases.

Solution

The fix is to put the future for key A inside read lock. And remove it inside write lock.
So that above scenario never occurs and Thread 2 will find the value in get() itself.

Related Issues

Resolves #14544

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• [] Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[sgup432]

@sohami Need help from you on this one.

[github-actions]

❕ Gradle check result for 5e020b8: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.75%. Comparing base (0eb39ae) to head (a8825a7).

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #14550      +/-   ##
============================================
- Coverage     71.77%   71.75%   -0.02%     
- Complexity    62191    62234      +43     
============================================
  Files          5125     5125              
  Lines        292411   292414       +3     
  Branches      42254    42255       +1     
============================================
- Hits         209868   209823      -45     
- Misses        65221    65273      +52     
+ Partials      17322    17318       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

✅ Gradle check result for a8825a7: SUCCESS