HDDS-10336. Allow setting client side ozone.replication configs to EC.

[SaketaChalamchala]

What changes were proposed in this pull request?

1. Setting the ozone.replication.type and ozone.replication configs to EC causes SCM to crash. This is because the BackgroundPipelineCreator does not skip pipeline creation when replication type is EC and tries to create an EC pipeline with HddsProtos.ReplicationFactor (ZERO, ONE, THREE) which is not supported.
   BackgroundPipelineCreator should not be creating EC pipelines. The proposed solution skips creating pipelines if ozone.replication.type is EC unless ozone.scm.pipeline.creation.auto.factor.one=true in which case, only RATIS ONE pipelines are created by BackgroundPipelineCreator.

2. The order of replication config resolution of key is not as expected causing existing acceptance tests to fail.
   Current order of key's replication config resolution is:
       1. Replication config parameters passed from client during key create.
       2. Replication config from client side configs if parameters are not passed.
       3. Bucket replication config if client side parameters are not passed.
       4. Server side default if bucket replication config is null.
   So, if ozone.replication.type is set to EC and a key is created in a bucket, the key is always created with EC replication regardless of whether the bucket's replication config is EC or RATIS. The same goes for if ozone.replication.type is set to RATIS.
   The expected order of replication config resolution for key should be:
       1. Replication config parameters passed from client during key create.
       2. Bucket replication config if client side parameters are not passed.
       3. Replication config from client side configs if bucket replication config is null.
       3. Server side default config.
   So, this way key creation always used bucket replication configs unless replication config params are specifically passed through the client. It will default to server default replication config only if both key params and bucket replication config are null.

3. The order of replication config resolution of bucket is not as expected causing existing acceptance tests to fail.
   Current order of bucket's replication config resolution is
       1. Replication config parameters passed from client during bucket create.
       2. Server side default config if parameters are not passed.
   So, this means ozone.replication configs has no effect on bucket creation. You can set it to EC or RATIS but default bucket creation will always use the server default replication config.
   The proposed and I think expected order of bucket's replication config resolution should be
       1. Replication config parameters passed from client during bucket create.
       2. Replication config from client side configs if parameters are not passed.
       3. Server side default if client side replication config is null.

What is the link to the Apache JIRA

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

How was this patch tested?

Acceptance test.

[adoroszlai]

Thanks @SaketaChalamchala for the patch. I think items (2) and (3) are by design, to be backward compatible with old behavior, before bucket-level replication setting existed.

[SaketaChalamchala]

Thanks @SaketaChalamchala for the patch. I think items (2) and (3) are by design, to be backward compatible with old behavior, before bucket-level replication setting existed.

Ok, so would it be better to limit the scope of this PR to (1) and maybe create a DOC jira to specify what order of resolution to to expect when ozone.replication is configured in ozone-site.xml with bucket-level replication setting also being an option?
Is the intent to remove ozone.replication usage at some point?

[adoroszlai]

limit the scope of this PR to (1)

Yes, (1) can be fixed without controversy.

and maybe create a DOC jira to specify what order of resolution to to expect when ozone.replication is configured in ozone-site.xml with bucket-level replication setting also being an option?

Yes, I think doc needs some clarification.

Is the intent to remove ozone.replication usage at some point?

I don't know. It was "undocumented" (removed from ozone-default.xml) when ozone.server.default.replication settings were introduced. We may need to evaluate various interfaces (CLI, Java API, Hadoop FS API, S3) and consider backward compatibility.

[adoroszlai]

Thanks @SaketaChalamchala for updating the patch. The change in BackgroundPipelineCreator looks good.

I have some concern about the the changes in acceptance tests.

1. Same additional config is applied to both client and server-side, so we are not testing all possible combinations.
2. We don't need to start a new cluster to test different client-side config.
3. No need to run all (basic) EC tests, since we just want to verify the replication type of the keys getting created. Various data sizes are tested to exercise EC write/read paths, but this is independent of where the replication config is coming from. So for testing precedence of settings creating one small key per test case would be enough.

Usually I'm in favor of acceptance tests, but in this case I think it's better to add integration test cases instead. We can more easily generate a test matrix for combinations over: client config, client CLI options, bucket replication type, server config (2 different keys). Let me know if you would like more specific examples, or if you'd prefer someone else to implement the test.

[aswinshakil]

The old ordering in (2) and (3) is fine as long as the same order is followed in all the code paths. As @adoroszlai mentioned we can simulate this in integration tests with all possible combinations.

[SaketaChalamchala]

Thanks for the review @adoroszlai and @aswinshakil. Updated the patch.

[adoroszlai]

Thanks @SaketaChalamchala for updating the patch.

There are some checkstyle failures.

[adoroszlai]

Can you please check test failures?

testReplicationOrderDefaultECCluster() timed out after 100 seconds
testReplicationOrderDefaultRatisCluster() timed out after 100 seconds