add static method to load store header #2097
Conversation
Result of fdb-record-layer-pr-proto3 on Linux CentOS 7
|
Result of fdb-record-layer-pr-proto2 on Linux CentOS 7
|
|
SonarCloud Quality Gate failed. |
Result of fdb-record-layer-pr-proto3 on Linux CentOS 7
|
Result of fdb-record-layer-pr-proto2 on Linux CentOS 7
|
| @@ -2107,6 +2107,30 @@ private CompletableFuture<Void> checkUserVersion(@Nullable UserVersionChecker us | |||
| }); | |||
| } | |||
|
|
|||
| private static RecordMetaDataProto.DataStoreInfo checkAndParseStoreHeader(@Nullable KeyValue firstKeyValue, | |||
There was a problem hiding this comment.
Instead of duplicating this method, the non-static version should call this version
| SubspaceProvider subspaceProvider1 = new SubspaceProviderByKeySpacePath(keySpacePath); | ||
| Subspace subspace1 = subspaceProvider1.getSubspace(context); | ||
| return readStoreFirstKey(context, subspace1, IsolationLevel.SERIALIZABLE).thenApply(keyValue -> checkAndParseStoreHeader(keyValue, StoreExistenceCheck.NONE, context, subspaceProvider1, subspace1)); |
There was a problem hiding this comment.
| SubspaceProvider subspaceProvider1 = new SubspaceProviderByKeySpacePath(keySpacePath); | |
| Subspace subspace1 = subspaceProvider1.getSubspace(context); | |
| return readStoreFirstKey(context, subspace1, IsolationLevel.SERIALIZABLE).thenApply(keyValue -> checkAndParseStoreHeader(keyValue, StoreExistenceCheck.NONE, context, subspaceProvider1, subspace1)); | |
| SubspaceProvider subspaceProvider = new SubspaceProviderByKeySpacePath(keySpacePath); | |
| Subspace subspace = subspaceProvider.getSubspace(context); | |
| return readStoreFirstKey(context, subspace, IsolationLevel.SERIALIZABLE).thenApply(keyValue -> checkAndParseStoreHeader(keyValue, StoreExistenceCheck.NONE, context, subspaceProvider, subspace)); |
| SubspaceProvider subspaceProvider1 = new SubspaceProviderByKeySpacePath(keySpacePath); | ||
| Subspace subspace1 = subspaceProvider1.getSubspace(context); | ||
| return readStoreFirstKey(context, subspace1, IsolationLevel.SERIALIZABLE).thenApply(keyValue -> checkAndParseStoreHeader(keyValue, StoreExistenceCheck.NONE, context, subspaceProvider1, subspace1)); |
There was a problem hiding this comment.
If this takes the StoreExistenceCheck as a parameter, acn more of this be shared between the static and non-static version of loadStoreHeaderAsync
| @@ -2380,6 +2404,12 @@ private void endRecordStoreStateWrite() { | |||
| }); | |||
| } | |||
|
|
|||
| public static CompletableFuture<RecordMetaDataProto.DataStoreInfo> loadStoreHeaderAsync(@Nonnull FDBRecordContext context, @Nonnull KeySpacePath keySpacePath) { | |||
There was a problem hiding this comment.
This should have a javadoc
It should probably also be marked as INTERNAL.
| @@ -2380,6 +2404,12 @@ private void endRecordStoreStateWrite() { | |||
| }); | |||
| } | |||
|
|
|||
| public static CompletableFuture<RecordMetaDataProto.DataStoreInfo> loadStoreHeaderAsync(@Nonnull FDBRecordContext context, @Nonnull KeySpacePath keySpacePath) { | |||
There was a problem hiding this comment.
This change should come with a github issue, and an update to the release notes
| @@ -3237,6 +3267,49 @@ private CompletableFuture<RecordStoreState> loadRecordStoreStateInternalAsync(@N | |||
| return context.instrument(FDBStoreTimer.Events.LOAD_RECORD_STORE_STATE, storeHeaderFuture.thenCombine(loadIndexStates, RecordStoreState::new)); | |||
| } | |||
|
|
|||
| @Nonnull | |||
| @SuppressWarnings("PMD.CloseResource") | |||
| public static CompletableFuture<Map<String, IndexState>> loadIndexStatesAsync(@Nonnull Subspace subspace, @Nonnull FDBRecordContext context) { | |||
There was a problem hiding this comment.
It would be better to have the non-static version call this version, and remove the duplication of logic here.
Also, this method should have a javadoc and probably be marked as INTERNAL
There was a problem hiding this comment.
Particularly of note for the javadoc is that readable indexes are not included in the returned map
| @@ -209,6 +210,24 @@ public void prefixPrimaryKeysWithNullByteAfterPrefix(int formatVersion, boolean | |||
| } | |||
| } | |||
|
|
|||
| @Test | |||
| public void testLoadStoreHeader() throws Exception { | |||
There was a problem hiding this comment.
There should also be a test for loading the index statuses
| RecordMetaDataProto.DataStoreInfo dataStoreInfo1 = FDBRecordStore.loadStoreHeaderAsync(context, path).get(); | ||
| // load store header with old method | ||
| RecordMetaDataProto.DataStoreInfo dataStoreInfo2 = recordStore.getRecordStoreState().getStoreHeader(); | ||
| // assert |
There was a problem hiding this comment.
RecordMetaDataProto.DataStoreInfo is a protobuf so it would be better to just assert that the two datastoreInfos are equals, so that this test maintains coverage if those protobuf definitions add fields.
| Assertions.assertEquals(FDBRecordStore.MAX_SUPPORTED_FORMAT_VERSION, dataStoreInfo2.getFormatVersion()); | ||
| Assertions.assertEquals(recordStore.getRecordMetaData().getVersion(), dataStoreInfo1.getMetaDataversion()); | ||
| Assertions.assertEquals(recordStore.getRecordMetaData().getVersion(), dataStoreInfo2.getMetaDataversion()); | ||
| Assertions.assertEquals(dataStoreInfo1.getLastUpdateTime(), dataStoreInfo2.getLastUpdateTime()); |
There was a problem hiding this comment.
It's probably overkill, but it might be good to change the store header and assert that getting it still gets the same value.
Perhaps more relevant for getting the index states than the store header
Add this static method to load store header without opening the store.