Skip to content

Redwood - Skip page copy for update if page wasn't already cached#4678

Merged
sfc-gh-ngoyal merged 7 commits into
apple:masterfrom
sfc-gh-satherton:redwood-commit-read-uncacheable
May 4, 2021
Merged

Redwood - Skip page copy for update if page wasn't already cached#4678
sfc-gh-ngoyal merged 7 commits into
apple:masterfrom
sfc-gh-satherton:redwood-commit-read-uncacheable

Conversation

@sfc-gh-satherton

@sfc-gh-satherton sfc-gh-satherton commented Apr 17, 2021

Copy link
Copy Markdown
Collaborator

Reading a page from the pager with the uncacheable flag now returns whether not the page result is from the cache. CommitSubtree uses this to avoid caching a page version that is about to be updated and avoids having to copy the old version for update.

Also added some output improvements to the performance unit test and added a function to the Pager to request a page eviction explicitly.

This PR looks large because it is a descendant of #4616. The final commit has the copy skipping changes.

Passes 100k Redwood unit tests 20210417-025121-satherton_btree-c1870e53bd9b9a18

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 master 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)

@foundationdb-ci

Copy link
Copy Markdown
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: foundationdb-pull-request-build
  • Commit ID: 5c3cb0d
  • Result: FAILED
  • Build Logs (available for 7 days)

@foundationdb-ci

Copy link
Copy Markdown
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: foundationdb-pull-request-build
  • Commit ID: fc9df5a
  • Result: SUCCEEDED
  • Build Logs (available for 7 days)

@sfc-gh-satherton sfc-gh-satherton force-pushed the redwood-commit-read-uncacheable branch from fc9df5a to c891e55 Compare May 3, 2021 20:42
@foundationdb-ci

Copy link
Copy Markdown
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: foundationdb-pull-request-build
  • Commit ID: 5ffd29d
  • Result: FAILED
  • Build Logs (available for 7 days)

@foundationdb-ci

Copy link
Copy Markdown
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: foundationdb-pull-request-build
  • Commit ID: c891e55
  • Result: SUCCEEDED
  • Build Logs (available for 7 days)

}

// Try to evict the item at index from cache
// Returns true if item is evicted or was not present in cache

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it's returning false for the later case?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! I'll fix this.

@sfc-gh-ngoyal sfc-gh-ngoyal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@sfc-gh-ngoyal sfc-gh-ngoyal merged commit 434e38d into apple:master May 4, 2021
sfc-gh-satherton added a commit to sfc-gh-satherton/foundationdb that referenced this pull request May 6, 2021
@sfc-gh-satherton sfc-gh-satherton changed the title Skip page copy for update if page wasn't already cached Redwood - Skip page copy for update if page wasn't already cached May 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants