anuragp010 opened a new pull request, #11845: URL: https://github.com/apache/ozone/pull/11845
## What changes were proposed in this pull request? This PR is to enable a review for merging latest master changes from master into [HDDS-11233](https://issues.apache.org/jira/browse/HDDS-11233) - storage policy feature branch. ### **Conflict**s Resolved: 1. ClientProtocol.java: **Conflict**: `import org.apache.hadoop.ozone.om.helpers.OmBucketArgs;` was added by the storage policy feature branch on the same line where master added `import org.apache.hadoop.ozone.om.helpers.ListStatusResult; `. **Resolution**: Add `ListStatusResult` import before `OmBucketArgs` import. 2. ContainerProtocolCalls.java: **Conflict**: In `writeSmallFile` method, the feature branch uses `PutSmallFileRequestProto.Builder putSmallFileBuilder `and then passes the same builder object to `setPutSmallFile` of `ContainerCommandRequestProto.newBuilder()`, whereas in master, `putSmallFileRequest` is created and passed to `setPutSmallFile`. The master also added `setWritePipelineVersion` after `setPutSmallFile`, which caused the **Conflict**. **Resolution**: Keep the feature branch's change which uses `putSmallFileBuilder` and then add the line to `setWritePipelineVersion` below it. 3. ContainerTestUtils.java: **Conflict**: `import org.apache.hadoop.fs.StorageType;` is imported by the feature branch on the same line that master adds `import org.apache.hadoop.hdds.HDDSVersion;` **Resolution**: import `StorageType` first and then `HDDSVersion` . **Occurrences**: ScmClientProtocolServer.java, TestKeyValueHandler.java, TestMiniOzoneCluster.java 4. DatanodeInfo.java: **Conflict**: `import org.apache.hadoop.hdds.scm.node.PendingContainerTracker.PendingContainerAllocations;` replaces the `TwoWindowBucket` import on the feature branch while master adds the import, `import org.apache.hadoop.hdds.upgrade.HDDSVersionUtils;` after the import of `TwoWindowBucket`. **Resolution**: Replace the `TwoWindowBucket` import with` PendingContainerAllocations` and then add the `HddsVersionUtils` import. 5. DataTestUtil.java: **Conflict**: The feature branch gets rid of the `StorageType` import whereas master is getting rid of the `ReplicationFactor`, `ReplicationType` and `Table` imports. Since they are on similar lines, there is a **Conflict**. **Resolution**: Remove all 4 imports from the file. 6. ECUnderReplicationHandler.java: **Conflict**: In `processDecommissioningIndexes` method the feature branch adds a TODO comment and adds `StorageType.Default` as an arg to the `getTargetDatanodes` call. The master earlier supported only copying the indexes which are on DNs which are to be decomissioned. With the newly added code, it supports reconstructing the indexes which are on DNs as an alternative before trying to copy provided some conditions are met. **Resolution**: Add the newly added functionality from master, while the `StorageType` related changes are added to the traditional path of copying. 7. MisReplicationHandler.java: **Conflict**: In the method `processAndSendCommands`, the feature branch adds `StorageType.DEFAULT` as an arg to the `ReplicationManagerUtil.getTargetDatanodes` call, whereas master adds a bug fix to the lines after the `ReplicationManagerUtil.getTargetDatanodes` call. **Resolution**: Keep the bug fix from master while adding `StorageType.DEFAULT` to the `ReplicationManagerUtil.getTargetDatanodes` call . 8. NodeManager.java: **Conflict**: `import org.apache.hadoop.fs.StorageType;` is added by feature branch on the same line on which master adds `ComponentVersion` and `HDDSVersion` imports. **Resolution**: Keep the master branch imports and add `StorageType` import before them. **Occurrences**: ReplicationManager.java, TestReplicationManager.java, 9. OmClientProtocol.proto: **Conflict**: Master adds the field `excludedETag = 25;` to the `message KeyArgs`, whereas the feature branch adds the field `storagePolicy = 25` to the same message. **Resolution**: Keep the master added `excludedETag = 25;` and add the field `storagePolicy` field to `KeyArgs` incrementing the field number to 26, i.e, `storagePolicy = 26;` . 10. OMKeyArgs.java: **Conflict**: In the method `toProtobuf()`, changes for `builder.setStoragePolicy` from the feature branch is on the same lines as `builder.setExcludedETag` from master. **Resolution**: Add `builder.setExcludedETag` and then add `builder.setStoragePolicy` after it. 11. OzoneBucket.java: **Conflict**: The feature branch adds `import org.apache.hadoop.ozone.om.helpers.OmBucketArgs;` on the same line on which master adds `import org.apache.hadoop.ozone.om.helpers.ListStatusResult;` **Resolution**: First add import for `ListStatusResult` followed by the `OmBucketArgs` import. 12. OzoneManager.java: **Conflict**: The feature branch removes a spacing indent on the line above final `UserGroupInformation ugi` in `checkAdminUserPrivilege` whereas master modifies the assignment value of `UserGroupInformation ugi` to use `ProtobufRpcEngine.Server.getRemoteUser();` **Resolution**: Keep master changes while removing the spacing indent. 13. OzoneManagerProtocolClientSideTranslatorPB.java: **Conflict**: In the method `openKey`, `keyArgs.setStoragePolicy` changes are on the same lines as `keyArgs.setExcludedETag`. **Resolution**: First the `keyArgs.setExcludedETag` related changes are added followed by the `keyArgs.setStoragePolicy` changes. 14. Pipeline.java: **Conflict**: `import java.util.concurrent.locks.ReadWriteLock;` and `import java.util.concurrent.locks.ReentrantReadWriteLock;` are added by the feature branch on the same lines on which master adds `import java.util.function.Function;` **Resolution**: Add the import for `ReadWriteLock` and `ReentrantReadWriteLock` followed by the import for `Function`. 15. ScmAdminProtocol.proto: **Conflict**: `listStorageTypeUsageInfoRequestProto = 53;` is added by the feature branch on the same line master adds `getPeerUpgradeStatusRequest = 53;` in the `message ScmContainerLocationRequest`. **Resolution**: Add `getPeerUpgradeStatusRequest = 53;` and then add `listStorageTypeUsageInfoRequestProto = 54;` **Conflict**: The feature branch adds `listStorageTypeUsageInfoResponseProto = 53;` on the same line where master adds `getPeerUpgradeStatusResponse = 53;` in the `message ScmContainerLocationRespons`e. **Resolution**: Add `getPeerUpgradeStatusResponse = 53` and `listStorageTypeUsageInfoResponseProto = 54;` **Conflict**: The feature branch adds `ListStorageTypeUsageInfo = 49;` on the same line where master adds `GetPeerUpgradeStatus = 49;` in the enum Type. **Resolution**: Add `GetPeerUpgradeStatus = 49;` and `ListStorageTypeUsageInfo = 50;` 16. ScmBlockLocationProtocol.java: **Conflict**: The feature branch removes `import org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor;` and `import org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationType;` while the master adds import `org.apache.hadoop.hdds.protocol.proto.HddsProtos;` above the 2 imports. **Resolution**: Remove imports for `ReplicationFactor` and `ReplicationType` and adds import for `HddsProtos`. 17. ScmBlockLocationProtocolServerSideTranslatorPB.java: **Conflict**: In `allocateScmBlock`, the feature branch added changes for setting the `StorageTier` to the `AllocateBlockResponse`. Master adds functionality on the same lines to compute and return the pipeline version per block. **Resolution**: Keep the pipeline versioning logic added on master and add storagePolicy changes for including `storageTier` as part of the `AllocateBlockResponse`. 18. SCMContainerPlacementRackAware.java: **Conflict**: Feature branch adds `import org.apache.hadoop.fs.StorageType;` after `import java.util.stream.Collectors;` but master removes `import java.util.stream.Collectors;` . **Resolution**: Remove `import java.util.stream.Collectors;` and keep the `StorageType` import. **Conflict**: In `chooseNode`, the master's changes ensures that each type of search (affinity based, any rack not in usedNodes, anyRack) has its own maxRetries limit, not one umbrella limit. In the feature branch, `storageType` was added to this search by including `storageType` in the params of `chooseNode` and including it in the `isValidNode` check as an arg while also including it when throwing an `SCMException`. **Resolution**: Keep the master's changes to `chooseNode`. Add `StorageType` as a param to `chooseNode`. Add `StorageType` to the isValid method call as an arg. Add `StorageType` in the SCMException messages. 19. SCMNodeManager.java: **Conflict**: Feature branch adds `import org.apache.hadoop.fs.StorageType;` and `import org.apache.hadoop.hdds.client.StorageTypeUtils;` on the same lines that master adds `import org.apache.hadoop.hdds.ComponentVersion;` and `import org.apache.hadoop.hdds.HddsUtils;` **Resolution**: Add all 4 imports `StorageType`, `ComponentVersion`, `HddsUtils` and `StorageTypeUtils`. 20. StorageContainerLocationProtocol.java: **Conflict**: Master makes this interface an empty abstract class whereas feature branch adds `listStorageTypeUsageInfo` and modifies all `allocateContainer` methods to use `StorageTier`. Master moves the core methods and functionality into `StorageContainerLocationInternalInterface`. **Resolution**: Add all the StoragePolicy related changes made in `StorageContainerLocationProtocol` to `StorageContainerLocationInternalInterface`. 21. StorageContainerLocationProtocolServerSideTranslator.java: **Conflict**: Minor **Conflict** related to removal of a single space in feature branch. **Resolution**: Keep the space removal and apply the changes from master. 22. TestECUnderReplicationHandler.java: **Conflict**: In `testMaintenanceDoesNotRequestZeroNodes`, master changes `verify(ecPlacementPolicy, times(0)).chooseDatanodes(anyList(), isNull(), eq(0), anyLong(),anyLong());` to `verify(ecPlacementPolicy, times(1)).chooseDatanodes(anyList(), anyList(), isNull(), anyInt(), anyLong(), anyLong());` while the feature branch adds `any(StorageType.class)` to `chooseDatanodes` verification. **Resolution**: `verify(ecPlacementPolicy, times(1)).chooseDatanodes(anyList(), anyList(), isNull(), anyInt(), anyLong(), anyLong(), any(StorageType.class));` 23. TestEndpoints.java: **Conflict**: `import org.apache.hadoop.hdds.client.OzoneStoragePolicy;` added by feature is on the same line as `import org.apache.hadoop.hdds.HDDSVersion;` added by master. **Resolution**: Add import for `HDDSVersion` and then `OzoneStoragePolicy`. 24. TestHDDSUpgrade.java: **Conflict**: Master deleted this file whereas the feature branch added `StoragePolicy` as args to the allocateContainer calls. **Resolution**: Delete the file as master now supports a new upgrade path - ZDU. 25. TestIncrementalContainerReportHandler.java: **Conflict**: Master removes `import static org.apache.hadoop.hdds.upgrade.HDDSLayoutVersionManager.maxLayoutVersion;` and adds `import static org.apache.hadoop.hdds.scm.upgrade.ScmUpgradeTestUtils.mockVersionManager;` whereas the feature branch adds `import static org.assertj.core.api.Assertions.assertThat;` below the maxLayoutVersion import. **Resolution**: Remove the `maxLayoutVersion` import. Add the import for `mockVersionManager` followed by the import for `assertThat`. 26. TestKeyValueContainer.java: **Conflict**: Master adds `import static org.mockito.Mockito.doAnswer;` and `import static org.mockito.Mockito.doReturn;` whereas the feature branch adds `import static org.mockito.Mockito.eq;` on the same lines. **Resolution**: Add imports for `doAnswer` and `doReturn` followed by the import for `Mockit.eq`. **Conflict**: Feature branch adds the imports `import org.apache.hadoop.hdds.protocol.proto.HddsProtos.StorageTypeProto;` , `import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerReplicaProto;` on the same lines where master adds the imports for `import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerBlocksDeletionACKProto.DeleteBlockTransactionResult; `, `import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.DeletedBlocksTransaction;` **Resolution**: Add all 4 imports. 27. TestNodeDecommissionManager.java: **Conflict**: Master makes changes to the `decom` variable in the `setup` method by adding `mockReplicationManager(conf)`, whereas the feature branch adds `any(StorageTier.class)` to `the when -> thenAnswer` of `allocateContainer` in the following line. **Resolution**: Keep master change and the feature branch change of having `any(StorageTier.class)` as an arg. 28. TestOMBucketSetPropertyRequest.java: **Conflict**: `import org.apache.hadoop.hdds.client.OzoneStoragePolicy;` and `import org.apache.hadoop.hdds.client.StoragePolicy;` added by feature branch on the same line on which ,master adds import `org.apache.hadoop.hdds.protocol.proto.HddsProtos;.` **Resolution**: Keep all 3 imports. 29. TestOzoneFSInputStream.java: **Conflict**: Feature branch removes a `StorageType` import whereas master adds 2 new imports on the same lines - `import org.apache.hadoop.hdds.scm.OzoneClientConfig;` and `import org.apache.hadoop.hdds.scm.storage.PositionedReadTestHelper;` **Resolution**: Remove the `StorageType` import while keeping the imports added by master. 30. TestRatisPipelineProvider.java: **Conflict**: The feature branch adds `StorageTier.getDefaultTier` as an arg to the `RatisReplicationConfig.getInstance` call in `testCreatePipelineWithDefaultLimit` and `testCreatePipelineThrowErrorWithDataNodeLimit` whereas master changes the following line's `ClientVersion.CURRENT_VERSION` to `ClientVersion.CURRENT`. **Resolution**: Keep both master and feature branch changes in both the tests. 31. TestReconAndAdminContainerCLI.java: **Conflict**: `waitForReplicaCount` was added to fix flakiness of `testNodesInDecommissionOrMaintenance` in the feature branch while master fixes this flakiness with `OzoneTestHelper.waitForStableReplicaCount` . **Conflict**: In the feature branch tests failing due to the `RunTimeException` thrown by setupRatisKey's `GenericTestUtils.waitFor() -> { return reconContainerMetadataManager.getKeyCountForContainer(containerID) > 0;}` was fixed by logging the exception and retrying whereas master added a helper, `ReconOmMetaManagerTestUtils.waitUntilReconKeyCounts` for fixing the same issue. **Resolution**: For both the **Conflict**s, the master changes are kept as the fixes are not related to the Storage Policy feature. 32. TestSCMClientProtocolServer.java: **Conflict**: Master adds an import for `ALREADY_FINALIZED` on the same line on which feature branch adds import for `assertThat`. `import org.apache.hadoop.fs.StorageType;` from feature branch and `import org.apache.hadoop.hdds.HDDSVersion;` from master are on the same line. **Resolution**: Keep both the `ALREADY_FINALIZED` and `assertThat` imports. Add the `StorageType` import followed by the `HDDSVersion` import. **Conflict**: Master and feature branch both add `import static org.mockito.Mockito.verify;` . **Resolution**: Keep a single import. 33. TestSCMContainerPlacementRackAware.java: **Conflict**: In `testSourceDatanodeIsNotChosenAsTarget`, the feature branch adds an arg, `StorageType.DEFAULT` to the setup method call. Whereas the master changes the first arg of the setup call from `2` to `1`. **Resolution**: Keep the first arg changed by master and add the arg `StorageType.DEFAULT` to the setup method call. 34. TestSCMNodeManager.java: **Conflict**: Feature branch added `StorageTier` to helpers `assertPipelineCreationFailsWithNotEnoughNodes`, `assertPipelineCreationFailsWithExceedingLimit` when they were creating pipelines which were used by `testScmLayoutOnRegister` whereas the master branch deleted the test and its associated helpers. **Resolution**: The tests were not related directly to the StoragePolicy feature as a result the code is removed staying in line with master. 35. TestStorageDistributionEndpoint.java: **Conflict**: Feature branch adds `import static java.util.Collections.singletonMap;` on the same line on which master adds `import static org.apache.hadoop.ozone.container.upgrade.UpgradeUtils.defaultVersionProto;`. **Resolution**: Keep both imports. 36. UniformDatanodesFactory.java: **Conflict**: Master gets rid of `DatanodeVersion` import and imports `HDDSVersion` on the same line as the feature branch imports `StorageType`. **Resolution**: Import `StorageType` followed by `HDDSVersion`. **Conflict**: The feature branch adds a new List of Lists - `datanodeStorageType` and all it's related code is surrounded by the variables `DatanodeVersion initialVersion` and `DatanodeVersion currentVersion`. Master replaces this `DatanodeVersion` with `ComponentVersion apparentVersion` and `HDDSVersion currentVersion`. Since the changes are on consecutive lines, there are multiple **Conflict**s. The same issue is present in the `Builder` class as well. **Resolution**: Keep the master's replacement of `initialVersion` with `apparentVersion` and` DatanodeVersion currentVersion` to `HddsVersion currentVersion` while adding the StoragePolicy related changes - `datanodeStorageType`. ### Notable Test Failures fixed: 1. TestOMStoragePolicyUpgrade: Now utilizes the new `TESTING_INIT_APPARENT_VERSION_KEY` and `serialize` method rather than `getLayoutVersion()`. 2. TestReconAndAdminContainerCLI is flaky as was fixed by https://github.com/apache/ozone/pull/11814 . The same fix was applied during the merge and the **Conflict** can be handled to accept the master changes during the next merge of master into the feature branch. 3. TestOMLayoutFeature#testNoNewOMLayoutFeaturesAdded is marked Unhealthy as the current state of StoragePolicy branch doesn't support ZDU and is not in compliance of the framework. This will be fixed as part of the epic [HDDS-1526](https://issues.apache.org/jira/browse/HDDS-17526) ## What is the link to the Apache JIRA https://issues.apache.org/jira/browse/HDDS-16116 ## How was this patch tested? 1. CI run. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
