From 02f9c0446329a934706d1e4183933c39d8dbba8d Mon Sep 17 00:00:00 2001 From: Matt Hess Date: Tue, 7 Nov 2023 16:07:02 -0600 Subject: [PATCH] Preserve logic via dependency migration test (#9723) Signed-off-by: Matt Hess Signed-off-by: Ivo Yankov --- .../main/java/com/hedera/node/app/Hedera.java | 52 +-- .../node/app/OrderedServiceMigrator.java | 136 ++++++++ .../state/merkle/DependencyMigrationTest.java | 305 ++++++++++++++++++ 3 files changed, 443 insertions(+), 50 deletions(-) create mode 100644 hedera-node/hedera-app/src/main/java/com/hedera/node/app/OrderedServiceMigrator.java create mode 100644 hedera-node/hedera-app/src/test/java/com/hedera/node/app/state/merkle/DependencyMigrationTest.java diff --git a/hedera-node/hedera-app/src/main/java/com/hedera/node/app/Hedera.java b/hedera-node/hedera-app/src/main/java/com/hedera/node/app/Hedera.java index afb97728247e..426b17c4dc46 100644 --- a/hedera-node/hedera-app/src/main/java/com/hedera/node/app/Hedera.java +++ b/hedera-node/hedera-app/src/main/java/com/hedera/node/app/Hedera.java @@ -37,7 +37,6 @@ import com.hedera.node.app.fees.congestion.EntityUtilizationMultiplier; import com.hedera.node.app.fees.congestion.ThrottleMultiplier; import com.hedera.node.app.ids.EntityIdService; -import com.hedera.node.app.ids.WritableEntityIdStore; import com.hedera.node.app.info.CurrentPlatformStatusImpl; import com.hedera.node.app.info.NetworkInfoImpl; import com.hedera.node.app.info.SelfNodeInfoImpl; @@ -57,7 +56,6 @@ import com.hedera.node.app.spi.workflows.record.GenesisRecordsBuilder; import com.hedera.node.app.state.HederaState; import com.hedera.node.app.state.merkle.MerkleHederaState; -import com.hedera.node.app.state.merkle.MerkleSchemaRegistry; import com.hedera.node.app.state.recordcache.RecordCacheService; import com.hedera.node.app.throttle.CongestionThrottleService; import com.hedera.node.app.throttle.SynchronizedThrottleAccumulator; @@ -98,7 +96,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Locale; -import java.util.Objects; import java.util.Set; import java.util.function.IntSupplier; import org.apache.logging.log4j.LogManager; @@ -422,53 +419,8 @@ private void onMigrate( final var selfNodeInfo = SelfNodeInfoImpl.of(nodeAddress, version); final var networkInfo = new NetworkInfoImpl(selfNodeInfo, platform, bootstrapConfigProvider); - logger.info("Migrating Entity ID Service as pre-requisite for other services"); - final var entityIdRegistration = servicesRegistry.registrations().stream() - .filter(service -> EntityIdService.NAME.equals(service.service().getServiceName())) - .findFirst() - .orElseThrow(); - final var entityIdRegistry = (MerkleSchemaRegistry) entityIdRegistration.registry(); - entityIdRegistry.migrate( - state, - previousVersion, - currentVersion, - configProvider.getConfiguration(), - networkInfo, - backendThrottle, - // We call with null here because we're migrating the entity ID service itself - null); - // Now that the Entity ID Service is migrated, migrate the remaining services - servicesRegistry.registrations().stream() - .filter(r -> !Objects.equals(entityIdRegistration, r)) - .forEach(registration -> { - // FUTURE We should have metrics here to keep track of how long it takes to migrate each service - final var service = registration.service(); - final var serviceName = service.getServiceName(); - logger.info("Migrating Service {}", serviceName); - final var registry = (MerkleSchemaRegistry) registration.registry(); - - // The token service has a dependency on the entity ID service during genesis migrations, so we - // CAREFULLY create a different WritableStates specific to the entity ID service. The different - // WritableStates instances won't be able to see the changes made by each other, but there shouldn't - // be any conflicting changes. We'll inject this into the MigrationContext below to enable - // generation of entity IDs. - final var entityIdWritableStates = state.createWritableStates(EntityIdService.NAME); - final var entityIdStore = new WritableEntityIdStore(entityIdWritableStates); - - registry.migrate( - state, - previousVersion, - currentVersion, - configProvider.getConfiguration(), - networkInfo, - backendThrottle, - requireNonNull(entityIdStore)); - // Now commit any changes that were made to the entity ID state (since other service entities could - // depend on newly-generated entity IDs) - if (entityIdWritableStates instanceof MerkleHederaState.MerkleWritableStates mws) { - mws.commit(); - } - }); + final var migrator = new OrderedServiceMigrator(servicesRegistry, backendThrottle); + migrator.doMigrations(state, currentVersion, previousVersion, configProvider.getConfiguration(), networkInfo); logger.info("Migration complete"); } diff --git a/hedera-node/hedera-app/src/main/java/com/hedera/node/app/OrderedServiceMigrator.java b/hedera-node/hedera-app/src/main/java/com/hedera/node/app/OrderedServiceMigrator.java new file mode 100644 index 000000000000..f19bacf43fb7 --- /dev/null +++ b/hedera-node/hedera-app/src/main/java/com/hedera/node/app/OrderedServiceMigrator.java @@ -0,0 +1,136 @@ +/* + * Copyright (C) 2023 Hedera Hashgraph, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.hedera.node.app; + +import static java.util.Objects.requireNonNull; + +import com.hedera.hapi.node.base.SemanticVersion; +import com.hedera.node.app.ids.EntityIdService; +import com.hedera.node.app.ids.WritableEntityIdStore; +import com.hedera.node.app.service.token.impl.TokenServiceImpl; +import com.hedera.node.app.services.ServicesRegistry; +import com.hedera.node.app.spi.info.NetworkInfo; +import com.hedera.node.app.spi.state.SchemaRegistry; +import com.hedera.node.app.state.merkle.MerkleHederaState; +import com.hedera.node.app.state.merkle.MerkleSchemaRegistry; +import com.hedera.node.app.throttle.ThrottleAccumulator; +import com.hedera.node.config.VersionedConfiguration; +import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.Nullable; +import java.util.Comparator; +import java.util.Objects; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +/** + * The entire purpose of this class is to ensure that inter-service dependencies are respected between + * migrations. The only required dependency right now is the {@link EntityIdService}, which is needed + * for genesis blocklist accounts in the token service genesis migration. (See {@link + * TokenServiceImpl#registerSchemas(SchemaRegistry)}). + * + *

Note: there are only two ordering requirements to maintain: first, that the entity ID service + * is migrated before the token service; and second, that the remaining services are migrated _in any + * deterministic order_. In order to ensure the entity ID service is migrated before the token service, + * we'll just migrate the entity ID service first. + */ +public class OrderedServiceMigrator { + private static final Logger logger = LogManager.getLogger(OrderedServiceMigrator.class); + private final ServicesRegistry servicesRegistry; + private final ThrottleAccumulator backendThrottle; + + public OrderedServiceMigrator( + @NonNull final ServicesRegistry servicesRegistry, @NonNull final ThrottleAccumulator backendThrottle) { + this.servicesRegistry = requireNonNull(servicesRegistry); + this.backendThrottle = requireNonNull(backendThrottle); + } + + /** + * Migrates the services registered with the {@link ServicesRegistry} + */ + public void doMigrations( + @NonNull final MerkleHederaState state, + @NonNull final SemanticVersion currentVersion, + @Nullable final SemanticVersion previousVersion, + @NonNull final VersionedConfiguration versionedConfiguration, + @NonNull final NetworkInfo networkInfo) { + requireNonNull(state); + requireNonNull(currentVersion); + requireNonNull(versionedConfiguration); + requireNonNull(networkInfo); + + logger.info("Migrating Entity ID Service as pre-requisite for other services"); + final var entityIdRegistration = servicesRegistry.registrations().stream() + .filter(service -> EntityIdService.NAME.equals(service.service().getServiceName())) + .findFirst() + .orElseThrow(); + final var entityIdRegistry = (MerkleSchemaRegistry) entityIdRegistration.registry(); + entityIdRegistry.migrate( + state, + previousVersion, + currentVersion, + versionedConfiguration, + networkInfo, + backendThrottle, + // We call with null here because we're migrating the entity ID service itself + null); + + // Now that the Entity ID Service is migrated, migrate the remaining services in name order. Note: the name + // ordering itself isn't important, just that the ordering is deterministic + servicesRegistry.registrations().stream() + .filter(r -> !Objects.equals(entityIdRegistration, r)) + .sorted(Comparator.comparing( + (ServicesRegistry.Registration r) -> r.service().getServiceName())) + .forEach(registration -> { + // FUTURE We should have metrics here to keep track of how long it takes to + // migrate each service + final var service = registration.service(); + final var serviceName = service.getServiceName(); + logger.info("Migrating Service {}", serviceName); + final var registry = (MerkleSchemaRegistry) registration.registry(); + + // The token service has a dependency on the entity ID service during genesis migrations, so we + // CAREFULLY create a different WritableStates specific to the entity ID service. The different + // WritableStates instances won't be able to "see" the changes made by each other, meaning that a + // change made with WritableStates instance X would _not_ be read by a separate WritableStates + // instance Y. However, since the inter-service dependencies are limited to the EntityIdService, + // there shouldn't be any changes made in any single WritableStates instance that would need to be + // read by any other separate WritableStates instances. This should hold true as long as the + // EntityIdService is not directly injected into any genesis generation code. Instead, we'll inject + // this entity ID writable states instance into the MigrationContext below, to enable generation of + // entity IDs through an appropriate API. + final var entityIdWritableStates = state.createWritableStates(EntityIdService.NAME); + final var entityIdStore = new WritableEntityIdStore(entityIdWritableStates); + + registry.migrate( + state, + previousVersion, + currentVersion, + versionedConfiguration, + networkInfo, + backendThrottle, + // If we have reached this point in the code, entityIdStore should not be null because the + // EntityIdService should have been migrated already. We enforce with requireNonNull in case + // there are scenarios we haven't considered. + requireNonNull(entityIdStore)); + // Now commit any changes that were made to the entity ID state (since other service entities could + // depend on newly-generated entity IDs) + if (entityIdWritableStates instanceof MerkleHederaState.MerkleWritableStates mws) { + mws.commit(); + } + }); + } +} diff --git a/hedera-node/hedera-app/src/test/java/com/hedera/node/app/state/merkle/DependencyMigrationTest.java b/hedera-node/hedera-app/src/test/java/com/hedera/node/app/state/merkle/DependencyMigrationTest.java new file mode 100644 index 000000000000..dd116398a493 --- /dev/null +++ b/hedera-node/hedera-app/src/test/java/com/hedera/node/app/state/merkle/DependencyMigrationTest.java @@ -0,0 +1,305 @@ +/* + * Copyright (C) 2023 Hedera Hashgraph, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.hedera.node.app.state.merkle; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; + +import com.hedera.hapi.node.base.SemanticVersion; +import com.hedera.hapi.node.state.common.EntityNumber; +import com.hedera.node.app.OrderedServiceMigrator; +import com.hedera.node.app.ids.EntityIdService; +import com.hedera.node.app.services.ServicesRegistryImpl; +import com.hedera.node.app.spi.Service; +import com.hedera.node.app.spi.fixtures.state.NoOpGenesisRecordsBuilder; +import com.hedera.node.app.spi.info.NetworkInfo; +import com.hedera.node.app.spi.state.MigrationContext; +import com.hedera.node.app.spi.state.Schema; +import com.hedera.node.app.spi.state.SchemaRegistry; +import com.hedera.node.app.spi.state.StateDefinition; +import com.hedera.node.app.spi.state.WritableStates; +import com.hedera.node.app.throttle.ThrottleAccumulator; +import com.hedera.node.config.VersionedConfigImpl; +import com.swirlds.common.constructable.ConstructableRegistry; +import edu.umd.cs.findbugs.annotations.NonNull; +import java.util.LinkedList; +import java.util.List; +import java.util.Set; +import org.assertj.core.api.Assertions; +import org.jetbrains.annotations.NotNull; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class DependencyMigrationTest extends MerkleTestBase { + private static final long INITIAL_ENTITY_ID = 5; + + @Mock + private ThrottleAccumulator accumulator; + + @Mock + private VersionedConfigImpl versionedConfig; + + @Mock + private NetworkInfo networkInfo; + + private MerkleHederaState merkleTree; + + @BeforeEach + void setUp() { + registry = mock(ConstructableRegistry.class); + merkleTree = new MerkleHederaState((tree, state) -> {}, (e, m, s) -> {}, (s, p, ds, t, dv) -> {}); + } + + @Nested + @SuppressWarnings("DataFlowIssue") + final class ConstructorTests { + @Test + void servicesRegistryRequired() { + Assertions.assertThatThrownBy(() -> new OrderedServiceMigrator(null, accumulator)) + .isInstanceOf(NullPointerException.class); + } + + @Test + void throttleAccumulatorRequired() { + Assertions.assertThatThrownBy(() -> new OrderedServiceMigrator(mock(ServicesRegistryImpl.class), null)) + .isInstanceOf(NullPointerException.class); + } + } + + @Nested + @SuppressWarnings("DataFlowIssue") + @ExtendWith(MockitoExtension.class) + final class DoMigrationsNullParams { + @Mock + private ServicesRegistryImpl servicesRegistry; + + @Test + void stateRequired() { + final var subject = new OrderedServiceMigrator(servicesRegistry, accumulator); + Assertions.assertThatThrownBy(() -> + subject.doMigrations(null, SemanticVersion.DEFAULT, null, versionedConfig, networkInfo)) + .isInstanceOf(NullPointerException.class); + } + + @Test + void currentVersionRequired() { + final var subject = new OrderedServiceMigrator(servicesRegistry, accumulator); + Assertions.assertThatThrownBy( + () -> subject.doMigrations(merkleTree, null, null, versionedConfig, networkInfo)) + .isInstanceOf(NullPointerException.class); + } + + @Test + void versionedConfigRequired() { + final var subject = new OrderedServiceMigrator(servicesRegistry, accumulator); + Assertions.assertThatThrownBy( + () -> subject.doMigrations(merkleTree, SemanticVersion.DEFAULT, null, null, networkInfo)) + .isInstanceOf(NullPointerException.class); + } + + @Test + void networkInfoRequired() { + final var subject = new OrderedServiceMigrator(servicesRegistry, accumulator); + Assertions.assertThatThrownBy(() -> + subject.doMigrations(merkleTree, SemanticVersion.DEFAULT, null, versionedConfig, null)) + .isInstanceOf(NullPointerException.class); + } + } + + @Test + @DisplayName("Genesis inter-service dependency migration works") + void genesisWithNullVersion() { + // Given: register the EntityIdService and the DependentService (order of registration shouldn't matter) + final var servicesRegistry = new ServicesRegistryImpl(registry, new NoOpGenesisRecordsBuilder()); + final var entityService = new EntityIdService() { + @Override + public void registerSchemas(@NonNull SchemaRegistry registry) { + registry.register(new Schema(SemanticVersion.DEFAULT) { + @NonNull + @Override + public Set statesToCreate() { + return Set.of(StateDefinition.singleton(ENTITY_ID_STATE_KEY, EntityNumber.PROTOBUF)); + } + + public void migrate(@NonNull MigrationContext ctx) { + final var entityIdState = ctx.newStates().getSingleton(ENTITY_ID_STATE_KEY); + entityIdState.put(new EntityNumber(INITIAL_ENTITY_ID)); + } + }); + } + }; + final DependentService dsService = new DependentService(); + Set.of(entityService, dsService).forEach(servicesRegistry::register); + + // When: the migrations are run + final var subject = new OrderedServiceMigrator(servicesRegistry, mock(ThrottleAccumulator.class)); + subject.doMigrations( + merkleTree, SemanticVersion.newBuilder().major(2).build(), null, versionedConfig, networkInfo); + + // Then: we verify the migrations had the desired effects on both entity ID state and DependentService state + // First check that the entity ID service has an updated entity ID, despite its schema migration not doing + // anything except setting the initial entity ID. DependentService's schema #2 should have caused the increments + // with its new additions to its own state. + final var postMigrationEntityIdState = + merkleTree.createReadableStates(EntityIdService.NAME).getSingleton(EntityIdService.ENTITY_ID_STATE_KEY); + assertThat(postMigrationEntityIdState.get()).isEqualTo(new EntityNumber(INITIAL_ENTITY_ID + 2)); + + // Also verify that both of the DependentService's schema migrations took place. First the initial mappings are + // created, then the new mappings dependent on the incrementing entity ID are added + final var postMigrationDsState = + merkleTree.createReadableStates(DependentService.NAME).get(DependentService.STATE_KEY); + assertThat(postMigrationDsState.get(INITIAL_ENTITY_ID - 1)).isEqualTo("previously added"); + assertThat(postMigrationDsState.get(INITIAL_ENTITY_ID)).isEqualTo("last added"); + assertThat(postMigrationDsState.get(INITIAL_ENTITY_ID + 1)).isEqualTo("newly-added 1"); + assertThat(postMigrationDsState.get(INITIAL_ENTITY_ID + 2)).isEqualTo("newly-added 2"); + } + + @Test + @DisplayName("Service migrations are ordered as expected") + void expectedMigrationOrdering() { + final var orderedInvocations = new LinkedList<>(); + + // Given: register four services, each with their own schema migration, that will add an object to + // orderedInvocations during migration. We'll do this to track the order of the service migrations + final var servicesRegistry = new ServicesRegistryImpl(registry, new NoOpGenesisRecordsBuilder()); + // Define the Entity ID Service: + final EntityIdService entityIdService = new EntityIdService() { + @Override + public void registerSchemas(@NonNull SchemaRegistry registry) { + registry.register(new Schema(SemanticVersion.DEFAULT) { + @NonNull + public Set statesToCreate() { + return Set.of(StateDefinition.singleton(ENTITY_ID_STATE_KEY, EntityNumber.PROTOBUF)); + } + + public void migrate(@NonNull MigrationContext ctx) { + orderedInvocations.add("EntityIdService#migrate"); + } + }); + } + }; + // Define Service A: + final var serviceA = new Service() { + @NonNull + @Override + public String getServiceName() { + return "A-Service"; + } + + @Override + public void registerSchemas(@NonNull SchemaRegistry registry) { + registry.register(new Schema(SemanticVersion.DEFAULT) { + public void migrate(@NonNull MigrationContext ctx) { + orderedInvocations.add("A-Service#migrate"); + } + }); + } + }; + // Define Service B: + final var serviceB = new Service() { + @NonNull + @Override + public String getServiceName() { + return "B-Service"; + } + + @Override + public void registerSchemas(@NonNull SchemaRegistry registry) { + registry.register(new Schema(SemanticVersion.DEFAULT) { + public void migrate(@NonNull MigrationContext ctx) { + orderedInvocations.add("B-Service#migrate"); + } + }); + } + }; + // Define DependentService: + final DependentService dsService = new DependentService() { + @Override + public void registerSchemas(@NonNull SchemaRegistry registry) { + registry.register(new Schema(SemanticVersion.DEFAULT) { + public void migrate(@NonNull MigrationContext ctx) { + orderedInvocations.add("DependentService#migrate"); + } + }); + } + }; + // Intentionally register the services in a different order than the expected migration order + List.of(dsService, serviceA, entityIdService, serviceB).forEach(servicesRegistry::register); + + // When: the migrations are run + final var subject = new OrderedServiceMigrator(servicesRegistry, mock(ThrottleAccumulator.class)); + subject.doMigrations( + merkleTree, SemanticVersion.newBuilder().major(1).build(), null, versionedConfig, networkInfo); + + // Then: we verify the migrations were run in the expected order + Assertions.assertThat(orderedInvocations) + .containsExactly( + // EntityIdService should be migrated first + "EntityIdService#migrate", + // And the rest are migrated by service name + "A-Service#migrate", + "B-Service#migrate", + "DependentService#migrate"); + } + + // This class represents a service that depends on EntityIdService. This class will create a simple mapping from an + // entity ID to a string value. + private static class DependentService implements Service { + static final String NAME = "DependentService"; + static final String STATE_KEY = "DS_MAPPINGS"; + + @NotNull + @Override + public String getServiceName() { + return NAME; + } + + public void registerSchemas(@NonNull SchemaRegistry registry) { + // Schema #1 - initial schema + registry.register(new Schema(SemanticVersion.DEFAULT) { + @NonNull + @Override + public Set statesToCreate() { + return Set.of(StateDefinition.inMemory(STATE_KEY, LONG_CODEC, STRING_CODEC)); + } + + public void migrate(@NonNull final MigrationContext ctx) { + WritableStates dsWritableStates = ctx.newStates(); + dsWritableStates.get(STATE_KEY).put(INITIAL_ENTITY_ID - 1, "previously added"); + dsWritableStates.get(STATE_KEY).put(INITIAL_ENTITY_ID, "last added"); + } + }); + + // Schema #2 - schema that adds new mappings, dependent on EntityIdService + registry.register(new Schema(SemanticVersion.newBuilder().major(2).build()) { + public void migrate(@NonNull final MigrationContext ctx) { + final WritableStates dsWritableStates = ctx.newStates(); + final var newEntityNum1 = ctx.newEntityNum(); + dsWritableStates.get(STATE_KEY).put(newEntityNum1, "newly-added 1"); + final var newEntityNum2 = ctx.newEntityNum(); + dsWritableStates.get(STATE_KEY).put(newEntityNum2, "newly-added 2"); + } + }); + } + } +}