HDDS-14957. Separate OpenKeyInfo to differentiate KeyInfo

[YutaLin]

What changes were proposed in this pull request?

Use a separate OpenKeyInfo for openKeyTable to differentiate with the final KeyInfo in the keyTable

Please describe your PR in detail:
Base on discussion: #9815 (comment)
We've add expectedDataGeneration and expectedETag in KeyInfo but these value should not be stored after commit, otherwise it has space overhead, so we introduced OpenKeyInfo to prevent space waste.

What is the link to the Apache JIRA

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

How was this patch tested?

Update tests with OmOpenKeyInfo and run CI Action(https://github.com/YutaLin/ozone/actions/runs/25239339785)

[YutaLin]

Hi @peterxcli, @ivandika3
Please help me review this, thanks!

[ivandika3]

Thanks @YutaLin for the patch, but I don't think this is a compatible change since we are changing the DB definition in-place. We need to think further on how to approach this.

[ivandika3]

IMO it is fine to leave things be unless there is a good reason other than separation of concern (e.g. https://issues.apache.org/jira/browse/HDDS-10611).

From the overhead point of view, adding fields to protobuf would not cause increased protobuf size as long as we remember to unassign the fields.

[YutaLin]

Hey @ivandika3,

adding fields to protobuf would not cause increased protobuf size as long as we remember to unassign the fields.

Thanks for review, I will close this patch since it's no longer needed.

[peterxcli]

I was thinking if we could just introduce a open key info serde class that only store a subset of key info, without, no explicitly unassign need, only need to build the data class from the incoming key info in request.

[peterxcli]

if you agree with my point, let's reopen this and refactor it! thanks!

[YutaLin]

Hi @peterxcli, that's a great feedback, I will raise another patch. thanks!

[YutaLin]

I don't think this is a compatible change since we are changing the DB definition in-place.

Hi @ivandika3, can you elaborate?
Would it be acceptable to implement a backward compatible codec that first attempts to serialize OpenKeyInfo and falls back to KeyInfo if that attempt fails?

[ivandika3]

@YutaLin The new OpenKeyInfo proto and KeyInfo proto are not compatible. For example, the field 1 for OpenKeyInfo is a KeyInfo, but field 1 for KeyInfo is volumeName although both are used to write/read to/from openKeyTable. This means it's not backward and forward compatible. You can try to run protolock tool we use to check the protobuf compatibility https://ozone.apache.org/docs/developer-guide/project/release-guide/#build-and-commit-protolock-files-to-the-master-branch

Would it be acceptable to implement a backward compatible codec that first attempts to serialize OpenKeyInfo and falls back to KeyInfo if that attempt fails?

No I don't think so, duplicate codec serde might cause performance issues since openKeyTable & openFileTable is in the critical path. I think the approach in #10181 might be enough.

[peterxcli]

The new OpenKeyInfo proto and KeyInfo proto are not compatible.

How about we add a Codec for each OM definition, where each Codec is actually a projection of a subset of the KeyInfo proto’s properties.