> Will there be any compatibility issues after moving enum `StorageTypeProto` from `ScmServerDatanodeHeartbeatProtocol.proto` to `hdds.proto` and standardize, and change all the places where `StorageTypeProto` is used in the existing protobuf to `hdds.proto#StorageTypeProto`?
There are different levels of compatibility: For binary compatibility, it is okay since "enum is compatible with int32, uint32, int64, and uint64 in terms of wire format" according to https://protobuf.dev/programming-guides/proto2/#updating It is easy to test: write both the new and the old protos to files and then compare the binaries. For API compatibility, the Java classes will be different. The question: are we using it in a public API? It also seems not since we are using - org.apache.hadoop.hdds.protocol.StorageType in the public APIs. (e.g. OzoneBucket.getStorageType(). ) According to https://protobuf.dev/programming-guides/proto2/#enum , we should add [deprecated=true] to each field instead of removing the old protos. Please try. BTW, we also have another StorageTypeProto defined in OmClientProtocol.proto. We should deprecate it as well. Please file a JIRA for the further discussion. Tsz-Wo On Mon, Aug 19, 2024 at 2:17 AM mrchenx <mrch...@126.com> wrote: > Dear Ozone Devs, > > > > > I am currently developing some features related to StoragePolicy and have > encountered an issue for which I am seeking a solution. > > > > > ## What I want to do > > - Move the enum `StorageTypeProto` from > `ScmServerDatanodeHeartbeatProtocol.proto` to `hdds.proto`. > > - Most of the protobuf definitions that need to be imported in our > project are located in `hdds.proto`, so I want to move enum > `StorageTypeProto` to `hdds.proto`. > > > > > - Standardize the usage of `hdds.proto#StorageTypeProto` across all > protobuf files in the project. > > - This would require removing > `ScmServerDatanodeHeartbeatProtocol.proto#StorageTypeProto` and > `OmClientProtocol.proto#StorageTypeProto`. > > > > > ## The error I encountered > > However, I encountered an issue where `proto-backwards-compatibility` > throws errors after moving the `enum StorageTypeProto`. > > > > > ```bash > > [INFO] --- > proto-backwards-compatibility:1.0.7:backwards-compatibility-check (default) > @ hdds-interface-server --- > > //... > > [ERROR] CONFLICT: "StorageTypeProto" field: "ARCHIVE" has been removed, > but is not reserved [ScmServerDatanodeHeartbeatProtocol.proto] > > [ERROR] CONFLICT: "StorageTypeProto" field: "DISK" has been removed, but > is not reserved [ScmServerDatanodeHeartbeatProtocol.proto] > > [ERROR] CONFLICT: "StorageTypeProto" field: "PROVIDED" has been removed, > but is not reserved [ScmServerDatanodeHeartbeatProtocol.proto] > > [ERROR] CONFLICT: "StorageTypeProto" field: "RAM_DISK" has been removed, > but is not reserved [ScmServerDatanodeHeartbeatProtocol.proto] > > [ERROR] CONFLICT: "StorageTypeProto" field: "SSD" has been removed, but is > not reserved [ScmServerDatanodeHeartbeatProtocol.proto] > > [ERROR] CONFLICT: "StorageTypeProto" integer: "1" has been removed, but is > not reserved [ScmServerDatanodeHeartbeatProtocol.proto] > > [ERROR] CONFLICT: "StorageTypeProto" integer: "2" has been removed, but is > not reserved [ScmServerDatanodeHeartbeatProtocol.proto] > > [ERROR] CONFLICT: "StorageTypeProto" integer: "3" has been removed, but is > not reserved [ScmServerDatanodeHeartbeatProtocol.proto] > > [ERROR] CONFLICT: "StorageTypeProto" integer: "4" has been removed, but is > not reserved [ScmServerDatanodeHeartbeatProtocol.proto] > > [ERROR] CONFLICT: "StorageTypeProto" integer: "5" has been removed, but is > not reserved [ScmServerDatanodeHeartbeatProtocol.proto] > > ``` > > > > > ## My question > > - Will there be any compatibility issues after moving enum > `StorageTypeProto` from `ScmServerDatanodeHeartbeatProtocol.proto` to > `hdds.proto` and standardize, and change all the places where > `StorageTypeProto` is used in the existing protobuf to > `hdds.proto#StorageTypeProto`? > > - According to my tests, when I add the compile flag > `-DallowBreakingChanges=true` to bypass the errors, old clients can still > successfully execute create/list bucket operations. > > - As I understand it, the `field id` of a protobuf enum is what matters, > as the `id` is the valid payload, while changes to the enum's field names > or the package in which the enum is defined should not cause compatibility > issues (though we will need to update the relevant Java code to reflect > these changes). > > > > > - If this does not cause compatibility issues, how should I handle the > errors reported by `proto-backwards-compatibility`? > > >
