elek commented on a change in pull request #1973:
URL: https://github.com/apache/ozone/pull/1973#discussion_r589288762
##########
File path: hadoop-hdds/interface-server/src/main/proto/ScmServerProtocol.proto
##########
@@ -125,10 +125,17 @@ enum Status {
message AllocateScmBlockRequestProto {
required uint64 size = 1;
required uint32 numBlocks = 2;
+
+ //deprected, use RatisReplicationConfig
Review comment:
```
If we have two separated config classes (RatisReplicationConfig and
ECReplicationConfig), then it's not easy to pass through the same API right?
```
In Java I assume we will have a common superclass / interface, so it will
be possible. Unfortunately we can do it only with protobuf 3 and I wouldn't
like to be blocked by that.
Until using proto3 I followed the same approach what we used earlier:
We have multiple subclasses (like `CloseContainerRequestProto`,
`ListContainerRequestProto` ...) and based on a type
`ContainerCommandRequestProto.type` we choose the right one.
Here I did exactly the same. The type is encoded in the `replication type`
and based on the type we have two protos. But we can deserialize them to good
old java classes which have common marker interface.
> So, we need another config class for non Ratis ( you named only
RatisReplicationConfig, why do we need to bother Ratis or non Ratis? it's just
replication right )?
If I understood well, you talk about STANDALONE.
1. STANDALONE replication is deprecated, therefore I didn't create
configuration for that.
2. The supported way to use ONE factor is using one node Ratis
3. we do need to bother Ratis or non Ratis because they are different
replications. In this model we may have different replication configuration for
each of the replcation method
> I think we can bring ReplicationConfig changes in master itself and in EC
branch we just need to add new EC config.
I think the current state of the EC development is more like a proto-type
phase. I would prefer to do this first on EC branch, check if it works well
end2end (at least the read / write path). If it works well we can add it to the
master.
But I wouldn't change master until we see a working poc from ec.
> So, why can't we add replication type with more specific like EC_3_2 and
replication factor is 5.
Because it's a less generic approach and instead of using meaningful, typed
fields to hold information we would use string based naming conventions which
is more error-prone, what I would like to avoid.
And what about if we need to introduce 2-3 new parameters for EC? Would it
be EC_3_2_PARAM1_PARAM2_PARAM3?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
