You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "symious (via GitHub)" <gi...@apache.org> on 2024/01/21 13:12:52 UTC

[PR] HDDS-10180. update proto.lock for Ozone 1.4.0 [ozone]

symious opened a new pull request, #6044:
URL: https://github.com/apache/ozone/pull/6044

   ## What changes were proposed in this pull request?
   
   Add proto.lock files from ozone-1.4 release branch to master/
   
   These files are checked as part of the build to prevent protobuf incompatabilities.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-10180
   
   ## How was this patch tested?
   
   No tests.
   


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-10180. update proto.lock for Ozone 1.4.0 [ozone]

Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #6044:
URL: https://github.com/apache/ozone/pull/6044#issuecomment-1904460019

   Need to wait until https://github.com/apache/ozone/pull/5579 is merged.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-10180. update proto.lock for Ozone 1.4.0 [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #6044:
URL: https://github.com/apache/ozone/pull/6044#issuecomment-1903577742

   Thanks @symious for the patch.
   
   The compatibility check reports some errors:
   
   ```
   [INFO] CONFLICT: "SetSnapshotPropertyRequest" field: "snapshotKey" ID: 1 has an updated name, previously "snapshotProperty" [OmClientProtocol.proto]
   [INFO] CONFLICT: "SetSnapshotPropertyRequest" field: "snapshotProperty" has been removed, but is not reserved [OmClientProtocol.proto]
   [INFO] CONFLICT: "SnapshotProperty" ID: "1" has been removed, but is not reserved [OmClientProtocol.proto]
   [INFO] CONFLICT: "SnapshotProperty" ID: "2" has been removed, but is not reserved [OmClientProtocol.proto]
   [INFO] CONFLICT: "SnapshotProperty" ID: "3" has been removed, but is not reserved [OmClientProtocol.proto]
   [INFO] CONFLICT: "SnapshotProperty" field: "exclusiveReplicatedSize" has been removed, but is not reserved [OmClientProtocol.proto]
   [INFO] CONFLICT: "SnapshotProperty" field: "exclusiveSize" has been removed, but is not reserved [OmClientProtocol.proto]
   [INFO] CONFLICT: "SnapshotProperty" field: "snapshotKey" has been removed, but is not reserved [OmClientProtocol.proto]
   [INFO] CONFLICT: "SnapshotPurgeRequest" ID: "2" has been removed, but is not reserved [OmClientProtocol.proto]
   [INFO] CONFLICT: "SnapshotPurgeRequest" field: "updatedSnapshotDBKey" has been removed, but is not reserved [OmClientProtocol.proto]
   ```
   
   caused by changes introduced in #5579:
   
   ```diff
   @@ -1891,15 +1892,16 @@ message SnapshotMoveKeyInfos {
    
    message SnapshotPurgeRequest {
      repeated string snapshotDBKeys = 1;
   -  repeated string updatedSnapshotDBKey = 2;
    }
    
    message SetSnapshotPropertyRequest {
   -  optional SnapshotProperty snapshotProperty = 1;
   +  optional string snapshotKey = 1;
   +  optional SnapshotSize snapshotSize = 2;
   +  optional bool deepCleanedDeletedDir = 3;
   +  optional bool deepCleanedDeletedKey = 4;
    }
    
   -message SnapshotProperty {
   -  optional string snapshotKey = 1;
   +message SnapshotSize {
      optional uint64 exclusiveSize = 2;
      optional uint64 exclusiveReplicatedSize = 3;
    }
   ```
   
   If I understand correctly, build for that PR would have been failed if the `proto.lock` files were updated sooner after the branch cut.
   
   I'll submit a PR to revert #5579, then we can revisit this one.  BTW, it would be better to recreate this branch (after the revert) in your fork.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-10180. update proto.lock for Ozone 1.4.0 [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai closed pull request #6044: HDDS-10180. update proto.lock for Ozone 1.4.0
URL: https://github.com/apache/ozone/pull/6044


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-10180. update proto.lock for Ozone 1.4.0 [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #6044:
URL: https://github.com/apache/ozone/pull/6044#issuecomment-1905674149

   Let's create new PR from fork after #6074 is merged.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-10180. update proto.lock for Ozone 1.4.0 [ozone]

Posted by "symious (via GitHub)" <gi...@apache.org>.
symious commented on PR #6044:
URL: https://github.com/apache/ozone/pull/6044#issuecomment-1905193218

   > Shouldn't there be changes to hadoop-hdds/interface-client/src/main/resources/proto.lock in this change as well? Git shows changes to protos in that module since the last release.
   
   There is an error for updating the lock for this directory: `protolock update failed for ./hadoop-hdds/interface-client/src/main/resources/../proto`


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-10180. update proto.lock for Ozone 1.4.0 [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #6044:
URL: https://github.com/apache/ozone/pull/6044#issuecomment-1905541843

   Thanks a lot @errose28 for spotting the missing change.  Since this PR just picks the change from Ozone 1.4.0 (9090d52387), the release also has incomplete `proto.lock`.
   
   ```
   $ git checkout ozone-1.4.0
   $ protolock status --lockdir=hadoop-hdds/interface-client/src/main/resources --protoroot=hadoop-hdds/interface-client/src/main/proto
   CONFLICT: "data" was moved into oneof "responseData" [DatanodeClientProtocol.proto]
   CONFLICT: "dataBuffers" was moved into oneof "responseData" [DatanodeClientProtocol.proto]
   ```
   
   It complains about a change made in https://github.com/apache/ozone/commit/4136d47868e.  This is not a recent change, first went into 1.1.0.  Same/similar problem for previous releases:
   
   ```
   $ git checkout ozone-1.1.0
   $ protolock status --lockdir=hadoop-hdds/interface-client/src/main/resources --protoroot=hadoop-hdds/interface-client/src/main/proto
   CONFLICT: "data" was moved into oneof "responseData" [DatanodeClientProtocol.proto]
   
   $ git checkout ozone-1.2.0
   $ protolock status --lockdir=hadoop-hdds/interface-client/src/main/resources --protoroot=hadoop-hdds/interface-client/src/main/proto
   CONFLICT: "data" was moved into oneof "responseData" [DatanodeClientProtocol.proto]
   CONFLICT: "dataBuffers" was moved into oneof "responseData" [DatanodeClientProtocol.proto]
   
   ...
   ```
   
   It turns out to be a new compatibility check added in https://github.com/nilslice/protolock/commit/01572fa3b6b26607564fa07a6910c2616b7f89bd recently.
   
   Works fine with the previous release:
   
   ```
   $ go install github.com/nilslice/protolock/cmd/protolock@v0.16.0
   $ protolock status --lockdir=hadoop-hdds/interface-client/src/main/resources --protoroot=hadoop-hdds/interface-client/src/main/proto
   ```
   
   Similar problem applies to `csi.proto`, which was last changed in 2019, `proto.lock` added in 2020.
   
   I suggest:
   
   1. close this PR
   2. update `proto.lock` on `ozone-1.4` branch (filed HDDS-10187) in two steps (two commits in same PR):
      a. using v0.16.0 to get the missing changes without conflicts
      b. using v0.17.0 and `protolock commit --force` to persist structures for the new rule
   3. update `proto.lock` on `master` in a new PR by taking both 9090d52387 and the changes from step 2


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-10180. update proto.lock for Ozone 1.4.0 [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #6044:
URL: https://github.com/apache/ozone/pull/6044#issuecomment-1904905484

   @ChenSammi @errose28 This is ready for review.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org