From 9254dcf1e94e76f33ee46cdee2c37758ba718042 Mon Sep 17 00:00:00 2001 From: svm Date: Thu, 2 Jun 2022 17:07:49 -0700 Subject: [PATCH] Resolve #1710 UserVersionChecker should be getting RecordMetaDataProto.DataStoreInfo --- docs/ReleaseNotes.md | 2 +- .../provider/foundationdb/FDBRecordStore.java | 12 +++---- .../foundationdb/FDBRecordStoreBase.java | 31 +++++++++++++++++++ 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/docs/ReleaseNotes.md b/docs/ReleaseNotes.md index f3f3a6244a..0399af29fe 100644 --- a/docs/ReleaseNotes.md +++ b/docs/ReleaseNotes.md @@ -34,7 +34,7 @@ This release also updates downstream dependency versions. Most notably, the prot * **Feature** Feature 2 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN) * **Feature** Feature 3 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN) * **Feature** Publish test jar as part of the regular distribution [(Issue #1703)](https://github.com/FoundationDB/fdb-record-layer/issues/1703) -* **Feature** Feature 5 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN) +* **Feature** UserVersionChecker should be getting RecordMetaDataProto.DataStoreInfo [(Issue #1710)](https://github.com/FoundationDB/fdb-record-layer/issues/1710) * **Breaking change** Change 1 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN) * **Breaking change** Change 2 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN) * **Breaking change** Change 3 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN) diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/FDBRecordStore.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/FDBRecordStore.java index 8657b99ca5..18b5cab64c 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/FDBRecordStore.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/FDBRecordStore.java @@ -1991,9 +1991,6 @@ private CompletableFuture checkVersion(@Nullable UserVersionChecker use @Nonnull private CompletableFuture checkVersion(@Nonnull RecordMetaDataProto.DataStoreInfo storeHeader, @Nullable UserVersionChecker userVersionChecker) { RecordMetaDataProto.DataStoreInfo.Builder info = storeHeader.toBuilder(); - final boolean newStore = info.getFormatVersion() == 0; - final int oldMetaDataVersion = newStore ? -1 : info.getMetaDataversion(); - final int oldUserVersion = newStore ? -1 : info.getUserVersion(); if (info.hasFormatVersion() && info.getFormatVersion() >= SAVE_UNSPLIT_WITH_SUFFIX_FORMAT_VERSION) { // If the store is already using a format version greater than or equal to the version // where the unsplit records are now written with an extra suffix, use the value for @@ -2002,7 +1999,7 @@ private CompletableFuture checkVersion(@Nonnull RecordMetaDataProto.Dat omitUnsplitRecordSuffix = info.getOmitUnsplitRecordSuffix(); } final boolean[] dirty = new boolean[1]; - final CompletableFuture checkedUserVersion = checkUserVersion(userVersionChecker, oldUserVersion, oldMetaDataVersion, info, dirty); + final CompletableFuture checkedUserVersion = checkUserVersion(userVersionChecker, storeHeader, info, dirty); final CompletableFuture checkedRebuild = checkedUserVersion.thenCompose(vignore -> checkPossiblyRebuild(userVersionChecker, info, dirty)); return checkedRebuild.thenCompose(vignore -> { if (dirty[0]) { @@ -2013,12 +2010,15 @@ private CompletableFuture checkVersion(@Nonnull RecordMetaDataProto.Dat }).thenCompose(this::removeReplacedIndexesIfChanged); } - private CompletableFuture checkUserVersion(@Nullable UserVersionChecker userVersionChecker, int oldUserVersion, int oldMetaDataVersion, + private CompletableFuture checkUserVersion(@Nullable UserVersionChecker userVersionChecker, + @Nonnull final RecordMetaDataProto.DataStoreInfo storeHeader, @Nonnull RecordMetaDataProto.DataStoreInfo.Builder info, @Nonnull boolean[] dirty) { + final boolean newStore = info.getFormatVersion() == 0; + final int oldUserVersion = newStore ? -1 : info.getUserVersion(); if (userVersionChecker == null) { return AsyncUtil.DONE; } - return userVersionChecker.checkUserVersion(oldUserVersion, oldMetaDataVersion, metaDataProvider) + return userVersionChecker.checkUserVersion(storeHeader, metaDataProvider) .thenApply(newUserVersion -> { userVersion = newUserVersion; if (newUserVersion != oldUserVersion) { diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/FDBRecordStoreBase.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/FDBRecordStoreBase.java index 91b383f703..b10d197350 100644 --- a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/FDBRecordStoreBase.java +++ b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/FDBRecordStoreBase.java @@ -36,6 +36,7 @@ import com.apple.foundationdb.record.RecordFunction; import com.apple.foundationdb.record.RecordIndexUniquenessViolation; import com.apple.foundationdb.record.RecordMetaDataBuilder; +import com.apple.foundationdb.record.RecordMetaDataProto; import com.apple.foundationdb.record.RecordMetaDataProvider; import com.apple.foundationdb.record.ScanProperties; import com.apple.foundationdb.record.TupleRange; @@ -169,11 +170,41 @@ default FDBStoreTimer getTimer() { interface UserVersionChecker { /** * Check the user version. + *

+ * If this store is being created for the first time, the store header will be set to the default instance. In + * particular, the format version will be zero, whereas the format version on all stores that have already been + * created will be greater than zero. + * + * @param storeHeader store header + * @param metaData the meta-data provider that will be used to get meta-data + * + * @return the user version to store in the record info header + */ + default CompletableFuture checkUserVersion(@Nonnull RecordMetaDataProto.DataStoreInfo storeHeader, + RecordMetaDataProvider metaData) { + final boolean newStore = storeHeader.getFormatVersion() == 0; + final int oldMetaDataVersion = newStore ? -1 : storeHeader.getMetaDataversion(); + final int oldUserVersion = newStore ? -1 : storeHeader.getUserVersion(); + return checkUserVersion(oldUserVersion, oldMetaDataVersion, metaData); + } + + /** + * Check the user version. + *

+ * This method is not called on classes that override + * {@link #checkUserVersion(RecordMetaDataProto.DataStoreInfo, RecordMetaDataProvider)}. Such implementations + * can just throw an exception from here. + * * @param oldUserVersion the old user version or -1 if this is a new record store * @param oldMetaDataVersion the old meta-data version * @param metaData the meta-data provider that will be used to get meta-data + * * @return the user version to store in the record info header + * + * @deprecated use {@link #checkUserVersion(RecordMetaDataProto.DataStoreInfo, RecordMetaDataProvider)} instead */ + @API(API.Status.DEPRECATED) + @Deprecated CompletableFuture checkUserVersion(int oldUserVersion, int oldMetaDataVersion, RecordMetaDataProvider metaData);