Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce unnecessary clears of legacy version space #1936

Closed
alecgrieser opened this issue Dec 5, 2022 · 0 comments · Fixed by #1937
Closed

Reduce unnecessary clears of legacy version space #1936

alecgrieser opened this issue Dec 5, 2022 · 0 comments · Fixed by #1937
Assignees
Labels
performance Performance issues

Comments

@alecgrieser
Copy link
Contributor

Whenever we perform check version, if the meta-data or format version have changed, we try and clear the old version subspace if the new meta-data doesn't store record versions:

if (metaDataVersionChanged) {
// Clear the version table if we are no longer storing record versions.
if (!metaData.isStoreRecordVersions()) {
final Transaction tr = ensureContextActive();
tr.clear(getSubspace().subspace(Tuple.from(RECORD_VERSION_KEY)).range());
}
info.setMetaDataversion(newMetaDataVersion);
}

This is done because if we change the meta-data from storing to not storing record versions, than we will never consult this subspace again, and so if we don't clear it out, we'll leave stale data around. Worse, this can actually could cause records to be associated with the wrong versions if:

  1. A record is stored with a version
  2. Version storage is turned off, but the version data not cleared
  3. That record is updated (or the record is deleted and a new record is stored at the same primary key)
  4. Version storage is turned back on
  5. The record written in step 3 now will be associated with the version saved in step 1

Note that this is not a concern with the newer version format, where versions are stored next to the record, as in that case, the version is always read, even if the meta-data option to store records with versions is not enabled. (The option effectively controls whether or not versions are added by default, not whether they are stored and read at all, like it does in the old format version.)

We can reduce the number of range clears here by being more exact about when we issue this clear range. For example, we never need to clear this out on new stores (as the range is already empty), nor do we need to if the store was not using the old version subspace before the meta-data or format versions are updated. We also don't need to clear this out if the old meta-data did not store record versions, but that information is unfortunately not available during checkVersion, because it doesn't have a copy of the old meta-data (just the old meta-data version).

@alecgrieser alecgrieser added the performance Performance issues label Dec 5, 2022
@alecgrieser alecgrieser self-assigned this Dec 5, 2022
alecgrieser added a commit to alecgrieser/fdb-record-layer that referenced this issue Dec 5, 2022
…on space

This reduces the number of unnecessary clears issued during check version by being more precise about when the legacy version subspace is cleared. It will only clear the range now if (1) the store is not new and (2) the store header suggests that any versions in the old store would have been stored in the old space. It introduces a test that validates that the version is cleared if versions were stored in that old subspace and the meta-data is changed to so that versions are no longer stores. That test valdiates that there are fewer range clears now if the range doesn't have to be cleared, but other than that assert, the test asserts pass both before and after the main code changes.

This resolves FoundationDB#1936.
@MMcM MMcM closed this as completed in #1937 Dec 5, 2022
MMcM added a commit that referenced this issue Dec 5, 2022
Resolves #1936: Reduce unnecessary clears of legacy version space
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant