HDDS-9240. Test container schema V2 and V3 replica mutual import

[ChenSammi]

https://issues.apache.org/jira/browse/HDDS-9240

[sodonnel]

I haven't looked at this in detail, but I think a test like this would be better suited in TestKeyValueContainer, where there are already some other import test cases, such as empty container etc. That way, you are testing the import flow directly.

At the replication supervisor level, it just pushes a blob of data around and it doesn't really care what scheme it is - whether it is V2 or V3 is handled by the exporter and importer code which is all inside KeyValueContainer.java I think.

[ChenSammi]

@sodonnel , the test at the ReplicationSupervisor and DownloadAndImportReplicator can mimic the real container replication case at most extent, the only omit part is the container tar file transmission through GRPC channel. All the related functions in KeyValueContainer will be called during the test, including container creation, container export and container import. I think the current way can cover more logic in the container replication path compared with test with KeyValueContainer only.

[sodonnel]

These are unit tests, not integration tests. If you look at the rest of the tests in ReplicationSupervisor, they have mocked the import / export parts out, and test only the features of the ReplicationSupervisor - limits, expiring old entries, metric counting etc. This new test is very different, and involves setting up volumes etc to facilitate testing the importer and exporter, which are part of the KeyValueContainer class. There should be unit level tests in KeyValueContainer which validate that units behavior, which in this case is importing V2 to V3. The tests will also likely be easier to write and understand, and probably faster to execute.

Another side issue is that there are two code paths in the replicator tasks now - the push and pull replication modes. Pull is basically dead - we don't use it anymore in the new replication manager, which is the default now. The test added here I think uses the download and import code path, which is the old model.

[ChenSammi]

Another side issue is that there are two code paths in the replicator tasks now - the push and pull replication modes. Pull is basically dead - we don't use it anymore in the new replication manager, which is the default now. The test added here I think uses the download and import code path, which is the old model.

Okay. I will move the test location.

[ChenSammi]

@captainzmc , could you help to review the patch?

[captainzmc]

+1 The change looks good. Thanks @ChenSammi for the patch.

[adoroszlai]

Looks like the new test is flaky. Since this is a test-only change, I've reverted it.

Tests run: 176, Failures: 0, Errors: 16, Skipped: 6, Time elapsed: 61.031 s <<< FAILURE! - in org.apache.hadoop.ozone.container.keyvalue.TestKeyValueContainer
Error:  TestKeyValueContainer.testImportV3ReplicaToV2HddsVolume  Time elapsed: 0.116 s  <<< ERROR!
java.lang.IndexOutOfBoundsException: Index: 0, Size: 0
	at java.util.ArrayList.rangeCheck(ArrayList.java:659)
	at java.util.ArrayList.get(ArrayList.java:435)
	at org.apache.hadoop.ozone.container.keyvalue.TestKeyValueContainer.lambda$setUp$2(TestKeyValueContainer.java:174)
	at org.mockito.internal.stubbing.StubbedInvocationMatcher.answer(StubbedInvocationMatcher.java:40)
	at org.mockito.internal.handler.MockHandlerImpl.handle(MockHandlerImpl.java:99)
	at org.mockito.internal.handler.NullResultGuardian.handle(NullResultGuardian.java:29)
	at org.mockito.internal.handler.InvocationNotifierHandler.handle(InvocationNotifierHandler.java:33)
	at org.mockito.internal.creation.bytebuddy.MockMethodInterceptor.doIntercept(MockMethodInterceptor.java:57)
	at org.mockito.internal.creation.bytebuddy.MockMethodAdvice.handle(MockMethodAdvice.java:147)
	at org.apache.hadoop.ozone.container.common.volume.RoundRobinVolumeChoosingPolicy.chooseVolume(RoundRobinVolumeChoosingPolicy.java:50)
	at org.apache.hadoop.ozone.container.keyvalue.TestKeyValueContainer.testMixedSchemaImport(TestKeyValueContainer.java:986)
	at org.apache.hadoop.ozone.container.keyvalue.TestKeyValueContainer.testImportV3ReplicaToV2HddsVolume(TestKeyValueContainer.java:970)

[ChenSammi]

Looks like the new test is flaky. Since this is a test-only change, I've reverted it.

Tests run: 176, Failures: 0, Errors: 16, Skipped: 6, Time elapsed: 61.031 s <<< FAILURE! - in org.apache.hadoop.ozone.container.keyvalue.TestKeyValueContainer
Error:  TestKeyValueContainer.testImportV3ReplicaToV2HddsVolume  Time elapsed: 0.116 s  <<< ERROR!
java.lang.IndexOutOfBoundsException: Index: 0, Size: 0
	at java.util.ArrayList.rangeCheck(ArrayList.java:659)
	at java.util.ArrayList.get(ArrayList.java:435)
	at org.apache.hadoop.ozone.container.keyvalue.TestKeyValueContainer.lambda$setUp$2(TestKeyValueContainer.java:174)
	at org.mockito.internal.stubbing.StubbedInvocationMatcher.answer(StubbedInvocationMatcher.java:40)
	at org.mockito.internal.handler.MockHandlerImpl.handle(MockHandlerImpl.java:99)
	at org.mockito.internal.handler.NullResultGuardian.handle(NullResultGuardian.java:29)
	at org.mockito.internal.handler.InvocationNotifierHandler.handle(InvocationNotifierHandler.java:33)
	at org.mockito.internal.creation.bytebuddy.MockMethodInterceptor.doIntercept(MockMethodInterceptor.java:57)
	at org.mockito.internal.creation.bytebuddy.MockMethodAdvice.handle(MockMethodAdvice.java:147)
	at org.apache.hadoop.ozone.container.common.volume.RoundRobinVolumeChoosingPolicy.chooseVolume(RoundRobinVolumeChoosingPolicy.java:50)
	at org.apache.hadoop.ozone.container.keyvalue.TestKeyValueContainer.testMixedSchemaImport(TestKeyValueContainer.java:986)
	at org.apache.hadoop.ozone.container.keyvalue.TestKeyValueContainer.testImportV3ReplicaToV2HddsVolume(TestKeyValueContainer.java:970)

The failure is caused by recent change introduced in #5300 in TestKeyValueContainer#setUp.

[ChenSammi]

@captainzmc , thanks.