Skip to content

HDDS-6065: EC: Provide CLI option to reset the bucket replication config#2927

Merged
umamaheswararao merged 2 commits intoapache:HDDS-3816-ecfrom
umamaheswararao:HDDS-6065
Dec 22, 2021
Merged

HDDS-6065: EC: Provide CLI option to reset the bucket replication config#2927
umamaheswararao merged 2 commits intoapache:HDDS-3816-ecfrom
umamaheswararao:HDDS-6065

Conversation

@umamaheswararao
Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Added the CLI options to set the replication config parameters on a bucket.
One can re-set the replication config on bucket with the following arguments on bucket:
"set-replication-config", bucketPath, "-t", "EC", "-r", "rs-3-2-1024k"

What is the link to the Apache JIRA

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

How was this patch tested?

Added the test to verify the behavior.

@sodonnel
Copy link
Copy Markdown
Contributor

I've got a general question on this, and we may have covered this before, but I cannot remember the details.

The client gets a bucket object, via Volume.getBucket(). This call loads the bucket details from the server and this will include any replication defaults set against the bucket.

Then when we create a key, this bucket setting is passed from the client to the openKey() call or the client can pass a key level override, in which case the replication config from the bucket is not passed back to the server.

I was looking on the server side code, and I think it will just use whatever ReplicationConfig is passed from the client.

What if one user creates a bucket object and keeps it for a long time, creating key after key.

Another user or admin, changes the bucket level replication config.

Will the first client just keep creating keys with the old replication config until it gets a new bucket object?

If that is the case, I am wondering we need need to pass several replicationConfig fields to the server - a key specific one, the client default setting and the bucket level one? That way, if we have only the bucket level one coming from the client, we can adjust it if the server side bucket setting is different from the client side bucket setting.

If my understanding is correct, at the server side, I am not sure if we can tell if the passed replication config came from a key level config, bucket level, or the default replication from the configuration?

@umamaheswararao
Copy link
Copy Markdown
Contributor Author

This is a very good point @sodonnel.
Similar lines, one idea is to have versioning of bucket default config.
Client can have grandfather version. When someone sends with grandfather version, we always take it. Otherwise highest version number should take preference. Let's say bucket has replication config with version1 and someone modified it so we can increment the version to 2. Now when older client who was holding the bucket with version1 replication config, will still pass that replication config, but server knows it's modified and can use that version2. But the problem is about the transparency.
Client still does not know its using different config at the server silently. This needs more discussion I feel.

@sodonnel
Copy link
Copy Markdown
Contributor

If we go with bucket config versioning, then the server should probably throw an exception if it receives a request with older config, prompting the client to refresh its config. Similar to "optimistic locking" when using databases. I suspect this may be a tricky change in the client at this stage.

However, what value is there in passing the bucket config from the client to the server? If we always have to check the server anyway, we can avoid sending the bucket config from the client.

We know there is a heirarchy of places the config can come from:

  1. Key level setting supplied by the client.
  2. Bucket Default
  3. Cluster defaults - however this can be a client side default or server side default. I cannot remember what we decided on the client vs server default part.

If the client sends two replicationConfig fields, both optional:

  1. Key ReplicationConfig
  2. Client Default Replication Config

Then we don't need it to send the bucket level setting, as that is on the server anyway. On the server, we can apply the precedence rules and pick the correct setting accordingly.

@umamaheswararao
Copy link
Copy Markdown
Contributor Author

We made it to server side defaults. So below two should be treated pretty much same from client perspective.

If the client sends two replicationConfig fields, both optional:
Key ReplicationConfig
Client Default Replication Config

How about like: if client passes some config or client has default replication config, then only we will send to server, otherwise it should be simply null. So, Server will decide. If client passes any config, it will take it, otherwise checks at bucket...if nothing at bucket then it should take server default. However there are some challenges to solve at server as there are assumptions and different flags to take default values when null from client.

Ex: in OmKeyCreateRequest#preExecute:

boolean useRatis = ozoneManager.shouldUseRatis();

      HddsProtos.ReplicationFactor factor = keyArgs.getFactor();
      if (factor == null) {
        factor = useRatis ? HddsProtos.ReplicationFactor.THREE :
            HddsProtos.ReplicationFactor.ONE;
      }

      HddsProtos.ReplicationType type = keyArgs.getType();
      if (type == null) {
        type = useRatis ? HddsProtos.ReplicationType.RATIS :
            HddsProtos.ReplicationType.STAND_ALONE;
      }

I think this is a different problem exist. This JIRA can still go I think. I have just filed a JIRA(HDDS-6131) to investigate address the above mentioned issue.

@sodonnel
Copy link
Copy Markdown
Contributor

Yea, it makes sense to create another Jira for the issue I raised, as its not caused by this change - its a pre-existing problem. This change LGTM.

@umamaheswararao umamaheswararao merged commit f8fd2ff into apache:HDDS-3816-ec Dec 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants