elek commented on pull request #1973:
URL: https://github.com/apache/ozone/pull/1973#issuecomment-804029249
Thanks the question (and the clear problem description) @arp7
In general, I should fill both the new and old fields all the time to make
it possible to deprecate the old fields eventually.
But let me explain my view in more details (as I also did offline).
There are two questions here (I think).
1. Using a common `ReplicationConfig` java class with replication specific
sub-classes.
2. The `.proto` representations of this change.
The first part can be done easily with introducing new Java classes (and
only one `ECReplicationConfig` in proto files):
This change is separated and uploaded in the PR #2068
This `ReplicationConfig` is very useful as it can be used everywhere in the
SCM instead of `ReplicationType` and `ReplicationFactory` without proto
changes. That's what we need for the EC.
The second question: do we need to follow existing conventions (long-term)
or not. There are two options here:
1. Following existing convention: using a `type` field in the proto
structure + use one of the subclassess representations based on the type (eg.
`ECReplicationConfig`, `RatisReplicationConfig`, `StandaloneReplicationConfig`)
(Important: here we are talking about protobuf structures not about Java
classes). This is the convention what we follow everywhere (for example in
`ContainerCommandRequestProto`) and this is the convention which is followed by
this patch.
2. The second option is to keep everything as is (just adding the new
`ECReplicationConfig` field). This approach is equivalent is using only #2068
without this patch.
Summary: 1. follows the convention and more flexible (even
`RatisReplicationConfig` can have more configuration fields) but can be used
only gradually and until Ozone 2.0 we need to support the old fields, too. 2.
is simple, but introduces some level of technical debt.
Using the first approach (this patch) we need to take care about the
backward compatibility:
Client version | Server version | Wire message | Message stored in OM Raft
log, and DB | Behavior on downgrade
-- | -- | -- | -- | --
New | Old | uses factor/type AND ReplicationConfig (first can be optional
if server is known to be new) | store factor/type | server is old, no
downgrade
Old | Old | uses factor/type | store factor/type | server is old, no
downgrade
New | New | uses factor/type AND ReplicationConfig | store factor/type +
ReplicationConfig | can be done until we store old factor/type everywhere
(finalize)
Old | New | uses factor/type | store factor/type + ReplicationConfig |
factor/type is used to calculate ReplicationConfig, both can be stored
--
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]
