Add test coverage for SpecialKeyRangeAsyncImpl::getRange #7671

sfc-gh-clin · 2022-07-22T21:23:36Z

Extend the SpecialKeySpaceCorrectness workload to test getRange on SpecialKeyRangeAsyncImpl
Fix the bug found by the added test that the cache is invalid when we have multiple async ranges to read through

Code-Reviewer Section

The general guidelines can be found here.

Please check each of the following things and check all boxes before accepting a PR.

The PR has a description, explaining both the problem and the solution.
The description mentions which forms of testing were done and the testing seems reasonable.
Every function/class/actor that was touched is reasonably well documented.

For Release-Branches

If this PR is made against a release-branch, please also check the following:

This change/bugfix is a cherry-pick from the next younger branch (younger release-branch or main if this is the youngest branch)
There is a good reason why this PR needs to go into a release branch and this reason is documented (either in the description above or in a linked GitHub issue)

…lify the code

foundationdb-ci · 2022-07-22T21:28:58Z

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Commit ID: fdd298f
Duration 0:05:12
Result: ❌ FAILED
Error: Error while executing command: if [[ $(git diff --shortstat 2> /dev/null | tail -n1) == "" ]]; then echo "CODE FORMAT CLEAN"; else echo "CODE FORMAT NOT CLEAN"; echo; echo "THE FOLLOWING FILES NEED TO BE FORMATTED"; echo; git ls-files -m; echo; exit 1; fi. Reason: exit status 1
Build Logs (available for 30 days)

foundationdb-ci · 2022-07-22T21:29:11Z

Result of foundationdb-pr on Linux CentOS 7

Commit ID: fdd298f
Duration 0:05:29
Result: ❌ FAILED
Error: Error while executing command: if [[ $(git diff --shortstat 2> /dev/null | tail -n1) == "" ]]; then echo "CODE FORMAT CLEAN"; else echo "CODE FORMAT NOT CLEAN"; echo; echo "THE FOLLOWING FILES NEED TO BE FORMATTED"; echo; git ls-files -m; echo; exit 1; fi. Reason: exit status 1
Build Logs (available for 30 days)

fdb-windows-ci · 2022-07-22T21:58:16Z

Doxense CI Report for Windows 10

Commit ID: fdd298f
Result: ✔️ SUCCEEDED
Build Logs (available for 30 days)

fdb-windows-ci · 2022-07-22T22:27:55Z

Doxense CI Report for Windows 10

Commit ID: ed44c98
Result: ✔️ SUCCEEDED
Build Logs (available for 30 days)

foundationdb-ci · 2022-07-22T23:02:03Z

Result of foundationdb-pr-macos on macOS BigSur 11.5.2

Commit ID: ed44c98
Duration 0:42:28
Result: ✅ SUCCEEDED
Error: N/A
Build Logs (available for 30 days)

foundationdb-ci · 2022-07-22T23:27:43Z

Result of foundationdb-pr on Linux CentOS 7

Commit ID: ed44c98
Duration 1:08:07
Result: ✅ SUCCEEDED
Error: N/A
Build Logs (available for 30 days)

foundationdb-ci · 2022-07-23T00:43:38Z

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Commit ID: ed44c98
Duration 2:23:59
Result: ❌ FAILED
Error: Error while executing command: make -C tests -kj run. Reason: exit status 2
Build Logs (available for 30 days)

…ync-read-impls

fdb-windows-ci · 2022-07-23T05:39:02Z

Doxense CI Report for Windows 10

Commit ID: 3aa36fa
Result: ❌ FAILED
Build Logs (available for 30 days)

foundationdb-ci · 2022-07-23T05:39:35Z

Result of foundationdb-pr-macos on macOS BigSur 11.5.2

Commit ID: 3aa36fa
Duration 0:43:04
Result: ✅ SUCCEEDED
Error: N/A
Build Logs (available for 30 days)

foundationdb-ci · 2022-07-23T06:00:46Z

Result of foundationdb-pr on Linux CentOS 7

Commit ID: 3aa36fa
Duration 1:04:41
Result: ✅ SUCCEEDED
Error: N/A
Build Logs (available for 30 days)

foundationdb-ci · 2022-07-23T07:20:14Z

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Commit ID: 3aa36fa
Duration 2:24:07
Result: ❌ FAILED
Error: Error while executing command: make -C tests -kj run. Reason: exit status 2
Build Logs (available for 30 days)

fdb-windows-ci · 2022-07-26T21:28:08Z

Doxense CI Report for Windows 10

Commit ID: 95624d9
Result: ❌ FAILED
Build Logs (available for 30 days)

foundationdb-ci · 2022-07-26T21:59:04Z

Result of foundationdb-pr-macos on macOS BigSur 11.5.2

Commit ID: 95624d9
Duration 0:43:43
Result: ✅ SUCCEEDED
Error: N/A
Build Logs (available for 30 days)

foundationdb-ci · 2022-07-26T22:39:26Z

Result of foundationdb-pr on Linux CentOS 7

Commit ID: 95624d9
Duration 1:24:01
Result: ✅ SUCCEEDED
Error: N/A
Build Logs (available for 30 days)

foundationdb-ci · 2022-07-26T22:57:24Z

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Commit ID: 95624d9
Duration 1:42:03
Result: ❌ FAILED
Error: Error while executing command: make -C tests test_ycsb.run. Reason: exit status 2
Build Logs (available for 30 days)

fdbclient/SpecialKeySpace.actor.cpp

… to simplify the code" This reverts commit fdd298f.

fdb-windows-ci · 2022-08-02T17:57:50Z

Doxense CI Report for Windows 10

Commit ID: d9d02d4
Result: ❌ FAILED
Build Logs (available for 30 days)

foundationdb-ci · 2022-08-02T19:04:14Z

Result of foundationdb-pr on Linux CentOS 7

Commit ID: d9d02d4
Duration 1:21:45
Result: ✅ SUCCEEDED
Error: N/A
Build Logs (available for 30 days)

foundationdb-ci · 2022-08-02T19:22:29Z

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Commit ID: d9d02d4
Duration 1:39:58
Result: ✅ SUCCEEDED
Error: N/A
Build Logs (available for 30 days)

* Add getRange test coverage for SpecialKeyRangeAsyncImpl * Fix the bug in SpecialKeyRangeAsyncImpl found by the test * Refactor ConflictingKeysImpl::getRange to use containedRanges to simplify the code * Fix file format * Initialize SpecialKeyRangeAsyncImpl cache with correct end key * Add release notes * Revert "Refactor ConflictingKeysImpl::getRange to use containedRanges to simplify the code" This reverts commit fdd298f.

…#7778) * Validate subrange reads in simulation (#7597) * Add extra validation to special key space reads in simulation * Fix bugs turned up by validating subrange reads * Change to validateSpecialSubrangeRead It is in general not safe to expect that a read from the special key space returns the same results if performed again, since the transaction may be being modified concurrently. * Add comment * Add comment * Add test coverage for SpecialKeyRangeAsyncImpl::getRange (#7671) * Add getRange test coverage for SpecialKeyRangeAsyncImpl * Fix the bug in SpecialKeyRangeAsyncImpl found by the test * Refactor ConflictingKeysImpl::getRange to use containedRanges to simplify the code * Fix file format * Initialize SpecialKeyRangeAsyncImpl cache with correct end key * Add release notes * Revert "Refactor ConflictingKeysImpl::getRange to use containedRanges to simplify the code" This reverts commit fdd298f. Co-authored-by: Andrew Noyes <[email protected]>

* Add getRange test coverage for SpecialKeyRangeAsyncImpl * Fix the bug in SpecialKeyRangeAsyncImpl found by the test * Refactor ConflictingKeysImpl::getRange to use containedRanges to simplify the code * Fix file format * Initialize SpecialKeyRangeAsyncImpl cache with correct end key * Add release notes * Revert "Refactor ConflictingKeysImpl::getRange to use containedRanges to simplify the code" This reverts commit fdd298f.

foundationdb-ci · 2022-08-09T16:06:03Z

Result of foundationdb-pr-macos on macOS BigSur 11.5.2

Commit ID: d9d02d4
Duration 0:41:36
Result: ✅ SUCCEEDED
Error: N/A
Build Logs (available for 30 days)

* Add getRange test coverage for SpecialKeyRangeAsyncImpl * Fix the bug in SpecialKeyRangeAsyncImpl found by the test * Refactor ConflictingKeysImpl::getRange to use containedRanges to simplify the code * Fix file format * Initialize SpecialKeyRangeAsyncImpl cache with correct end key * Add release notes * Revert "Refactor ConflictingKeysImpl::getRange to use containedRanges to simplify the code" This reverts commit fdd298f.

…apple#7778) * Validate subrange reads in simulation (apple#7597) * Add extra validation to special key space reads in simulation * Fix bugs turned up by validating subrange reads * Change to validateSpecialSubrangeRead It is in general not safe to expect that a read from the special key space returns the same results if performed again, since the transaction may be being modified concurrently. * Add comment * Add comment * Add test coverage for SpecialKeyRangeAsyncImpl::getRange (apple#7671) * Add getRange test coverage for SpecialKeyRangeAsyncImpl * Fix the bug in SpecialKeyRangeAsyncImpl found by the test * Refactor ConflictingKeysImpl::getRange to use containedRanges to simplify the code * Fix file format * Initialize SpecialKeyRangeAsyncImpl cache with correct end key * Add release notes * Revert "Refactor ConflictingKeysImpl::getRange to use containedRanges to simplify the code" This reverts commit fdd298f. Co-authored-by: Andrew Noyes <[email protected]>

sfc-gh-clin added 3 commits July 22, 2022 12:34

Add getRange test coverage for SpecialKeyRangeAsyncImpl

17ef9b7

Fix the bug in SpecialKeyRangeAsyncImpl found by the test

1ba3409

Refactor ConflictingKeysImpl::getRange to use containedRanges to simp…

fdd298f

…lify the code

sfc-gh-clin requested a review from sfc-gh-anoyes July 22, 2022 21:27

sfc-gh-clin marked this pull request as ready for review July 22, 2022 21:27

Fix file format

ed44c98

sfc-gh-clin added 2 commits July 22, 2022 21:55

Initialize SpecialKeyRangeAsyncImpl cache with correct end key

f9c2780

Merge branch 'main' of github.com:apple/foundationdb into add-test-as…

3aa36fa

…ync-read-impls

sfc-gh-clin self-assigned this Jul 25, 2022

Add release notes

95624d9

sfc-gh-anoyes reviewed Aug 2, 2022

View reviewed changes

fdbclient/SpecialKeySpace.actor.cpp Show resolved Hide resolved

Revert "Refactor ConflictingKeysImpl::getRange to use containedRanges…

d9d02d4

… to simplify the code" This reverts commit fdd298f.

sfc-gh-anoyes self-requested a review August 2, 2022 17:45

sfc-gh-anoyes approved these changes Aug 2, 2022

View reviewed changes

sfc-gh-clin requested a review from sfc-gh-anoyes August 2, 2022 17:47

sfc-gh-clin merged commit 48e46cb into apple:main Aug 2, 2022

sfc-gh-clin deleted the add-test-async-read-impls branch August 2, 2022 19:04

sfc-gh-clin mentioned this pull request Aug 3, 2022

Add test coverage for SpecialKeyRangeAsyncImpl::getRange(release-7.1) #7778

Merged

5 tasks

sfc-gh-clin mentioned this pull request Aug 3, 2022

Add test coverage for SpecialKeyRangeAsyncImpl::getRange (snowflake-7.1) #7779

Merged

5 tasks

sfc-gh-clin mentioned this pull request Aug 3, 2022

Add test coverage for SpecialKeyRangeAsyncImpl::getRange (release-7.0) #7781

Merged

5 tasks

sfc-gh-clin added a commit to sfc-gh-clin/foundationdb that referenced this pull request Aug 5, 2022

Cherry-pick pull request apple#7671

406f937

sfc-gh-clin mentioned this pull request Aug 5, 2022

Cherry-pick pull request #7671 #7800

Merged

5 tasks

sfc-gh-clin added a commit to sfc-gh-clin/foundationdb that referenced this pull request Aug 5, 2022

Cherry-pick pull request apple#7671

a069900

sfc-gh-clin added a commit that referenced this pull request Aug 9, 2022

Cherry-pick pull request #7671 (#7800)

be8b344

Add test coverage for SpecialKeyRangeAsyncImpl::getRange #7671

Add test coverage for SpecialKeyRangeAsyncImpl::getRange #7671

Uh oh!

Conversation

sfc-gh-clin commented Jul 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code-Reviewer Section

For Release-Branches

Uh oh!

foundationdb-ci commented Jul 22, 2022

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Uh oh!

foundationdb-ci commented Jul 22, 2022

Result of foundationdb-pr on Linux CentOS 7

Uh oh!

fdb-windows-ci commented Jul 22, 2022

Doxense CI Report for Windows 10

Uh oh!

fdb-windows-ci commented Jul 22, 2022

Doxense CI Report for Windows 10

Uh oh!

foundationdb-ci commented Jul 22, 2022

Result of foundationdb-pr-macos on macOS BigSur 11.5.2

Uh oh!

foundationdb-ci commented Jul 22, 2022

Result of foundationdb-pr on Linux CentOS 7

Uh oh!

foundationdb-ci commented Jul 23, 2022

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Uh oh!

fdb-windows-ci commented Jul 23, 2022

Doxense CI Report for Windows 10

Uh oh!

foundationdb-ci commented Jul 23, 2022

Result of foundationdb-pr-macos on macOS BigSur 11.5.2

Uh oh!

foundationdb-ci commented Jul 23, 2022

Result of foundationdb-pr on Linux CentOS 7

Uh oh!

foundationdb-ci commented Jul 23, 2022

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Uh oh!

fdb-windows-ci commented Jul 26, 2022

Doxense CI Report for Windows 10

Uh oh!

foundationdb-ci commented Jul 26, 2022

Result of foundationdb-pr-macos on macOS BigSur 11.5.2

Uh oh!

foundationdb-ci commented Jul 26, 2022

Result of foundationdb-pr on Linux CentOS 7

Uh oh!

foundationdb-ci commented Jul 26, 2022

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Uh oh!

Uh oh!

fdb-windows-ci commented Aug 2, 2022

Doxense CI Report for Windows 10

Uh oh!

foundationdb-ci commented Aug 2, 2022

Result of foundationdb-pr on Linux CentOS 7

Uh oh!

foundationdb-ci commented Aug 2, 2022

Result of foundationdb-pr-cluster-tests on Linux CentOS 7

Uh oh!

foundationdb-ci commented Aug 9, 2022

Result of foundationdb-pr-macos on macOS BigSur 11.5.2

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sfc-gh-clin commented Jul 22, 2022 •

edited

Loading