Resolve #1710 UserVersionChecker should be getting RecordMetaDataProt…

…o.DataStoreInfo
FoundationDB · Jun 3, 2022 · d72b4bd · d72b4bd
1 parent d4f3dad
commit d72b4bd
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 7 deletions.
diff --git 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/...ore/src/main/java/com/apple/foundationdb/record/provider/foundationdb/FDBRecordStore.java b/...ore/src/main/java/com/apple/foundationdb/record/provider/foundationdb/FDBRecordStore.java
@@ -1991,9 +1991,6 @@ private CompletableFuture<Boolean> checkVersion(@Nullable UserVersionChecker use
     @Nonnull
     private CompletableFuture<Boolean> 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<Boolean> checkVersion(@Nonnull RecordMetaDataProto.Dat
             omitUnsplitRecordSuffix = info.getOmitUnsplitRecordSuffix();
         }
         final boolean[] dirty = new boolean[1];
-        final CompletableFuture<Void> checkedUserVersion = checkUserVersion(userVersionChecker, oldUserVersion, oldMetaDataVersion, info, dirty);
+        final CompletableFuture<Void> checkedUserVersion = checkUserVersion(userVersionChecker, storeHeader, info, dirty);
         final CompletableFuture<Void> checkedRebuild = checkedUserVersion.thenCompose(vignore -> checkPossiblyRebuild(userVersionChecker, info, dirty));
         return checkedRebuild.thenCompose(vignore -> {
             if (dirty[0]) {
@@ -2013,12 +2010,15 @@ private CompletableFuture<Boolean> checkVersion(@Nonnull RecordMetaDataProto.Dat
         }).thenCompose(this::removeReplacedIndexesIfChanged);
     }
 
-    private CompletableFuture<Void> checkUserVersion(@Nullable UserVersionChecker userVersionChecker, int oldUserVersion, int oldMetaDataVersion,
+    private CompletableFuture<Void> 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/...src/main/java/com/apple/foundationdb/record/provider/foundationdb/FDBRecordStoreBase.java b/...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,42 @@ default FDBStoreTimer getTimer() {
     interface UserVersionChecker {
         /**
          * Check the user version.
+         * <p>
+         * 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
+         */
+        @SuppressWarnings("deprecation")
+        default CompletableFuture<Integer> 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.
+         * <p>
+         * 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 <code>-1</code> 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<Integer> checkUserVersion(int oldUserVersion, int oldMetaDataVersion,
                                                     RecordMetaDataProvider metaData);