You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/06/28 23:25:18 UTC
[GitHub] [ozone] neils-dev opened a new pull request #2375: Initial commit for HDDS-4440. Commit for HDDS-5210 - gRPC service de…
neils-dev opened a new pull request #2375:
URL: https://github.com/apache/ozone/pull/2375
…finition for existing OM protocol. Grpc protocol for servicing s3g file operation requests. Grpc definition for generating gRPC server and client stubs serialization and deserialization for OM protocol and processing with blocking requests. Definitions for gRPC and netty packages and standardizing versions for ozone build affecting several pom build files.
## What changes were proposed in this pull request?
Patch modifies ozone build, pom build files, to generate s3gateway gRPC server interface and client stubs for transport of OM protocol. Through build initiated protoc compile of OM protocol protobuf defined in `hadoop-ozone/interface-client/pom.xml` source for gRPC serialization & deserialization server and client interfaces is generated in `hadoop-ozone/interface-client/target/generated-sources/protobuf/java/org/apache/hadoop/ozone/protocol/proto/OzoneManagerServiceGrpc.java`.
Ozone pom build files modified to use consistant netty and io.grpc versions _`<netty.version>4.1.63.Final</netty.version>`_ and _`<io.grpc.version>1.38.0</io.grpc.version>`_ respectively.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5210
## How was this patch tested?
Patch was tested by completing a clean build with generated gRPC server and client stubs. Generated OM protocol gRPC interface server and client code:` hadoop-ozone/interface-client/target/generated-sources/protobuf/java/org/apache/hadoop/ozone/protocol/proto/OzoneManagerServiceGrpc.java`
--
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
[GitHub] [ozone] adoroszlai commented on a change in pull request #2375: HDDS-5210. Create GRPC service definition and for existing Om protocol
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #2375:
URL: https://github.com/apache/ozone/pull/2375#discussion_r669691106
##########
File path: hadoop-ozone/common/pom.xml
##########
@@ -30,6 +30,21 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
<dependencies>
+ <dependency>
+ <groupId>io.grpc</groupId>
+ <artifactId>grpc-netty</artifactId>
+ <version>${io.grpc.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>io.netty</groupId>
+ <artifactId>netty-codec-http2</artifactId>
+ <version>${netty.version}</version>
Review comment:
With `netty-bom` imported in root `pom.xml`, `netty.version` does not need to be specified anywhere else.
--
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
[GitHub] [ozone] adoroszlai commented on pull request #2375: HDDS-5210. Create GRPC service definition and for existing Om protocol
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #2375:
URL: https://github.com/apache/ozone/pull/2375#issuecomment-883945607
Thanks @neils-dev for updating the patch.
--
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
[GitHub] [ozone] neils-dev commented on a change in pull request #2375: HDDS-5210. Create GRPC service definition and for existing Om protocol
Posted by GitBox <gi...@apache.org>.
neils-dev commented on a change in pull request #2375:
URL: https://github.com/apache/ozone/pull/2375#discussion_r667634294
##########
File path: hadoop-ozone/interface-client/pom.xml
##########
@@ -51,16 +71,46 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
<extensions>true</extensions>
<executions>
<execution>
- <id>compile-protoc</id>
+ <id>compile-protoc-OmGrpc</id>
<goals>
<goal>compile</goal>
<goal>test-compile</goal>
+ <goal>compile-custom</goal>
+ <goal>test-compile-custom</goal>
</goals>
<configuration>
+ <protocArtifact>
+ com.google.protobuf:protoc:${protobuf.version}:exe:${os.detected.classifier}
+ </protocArtifact>
<protoSourceRoot>${basedir}/src/main/proto/</protoSourceRoot>
+ <includes>
+ <include>OmClientProtocol.proto</include>
+ </includes>
+ <outputDirectory>target/generated-sources/protobuf/java</outputDirectory>
+ <clearOutputDirectory>false</clearOutputDirectory>
+ <pluginId>grpc-java</pluginId>
+ <pluginArtifact>
+ io.grpc:protoc-gen-grpc-java:${grpc-compile.version}:exe:${os.detected.classifier}
+ </pluginArtifact>
+ </configuration>
+ </execution>
+ <execution>
Review comment:
Thanks for uncovering the duplication in executions. We can also rewrite the two executions into a single execution by having the _`compile-OmGrpc`_ execution id also generate the protoc generated code that id _`compile-protoc-2`_ currently compiles, namely the `<include>Security.proto</include>`. In this case the single execution is :
```
<execution>
<id>compile-protoc-OmGrpc</id>
<goals>
<goal>compile</goal>
<goal>test-compile</goal>
<goal>compile-custom</goal>
<goal>test-compile-custom</goal>
</goals>
<configuration>
<protocArtifact>
com.google.protobuf:protoc:${protobuf.version}:exe:${os.detected.classifier}
</protocArtifact>
<protoSourceRoot>${basedir}/src/main/proto/</protoSourceRoot>
<includes>
<include>OmClientProtocol.proto</include>
<include>Security.proto</include>
</includes>
<outputDirectory>target/generated-sources/protobuf/java</outputDirectory>
<clearOutputDirectory>false</clearOutputDirectory>
<pluginId>grpc-java</pluginId>
<pluginArtifact>
io.grpc:protoc-gen-grpc-java:${grpc-compile.version}:exe:${os.detected.classifier}
</pluginArtifact>
</configuration>
</execution>
```
Either is fine, split the two or have a single execution - what are your thoughts on this?
--
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
[GitHub] [ozone] adoroszlai commented on a change in pull request #2375: HDDS-5210. Create GRPC service definition and for existing Om protocol
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #2375:
URL: https://github.com/apache/ozone/pull/2375#discussion_r666729604
##########
File path: hadoop-ozone/common/pom.xml
##########
@@ -30,6 +30,21 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
<dependencies>
+ <dependency>
+ <groupId>io.grpc</groupId>
Review comment:
@neils-dev Can you please provide more specific examples (e.g. compile errors or runtime exceptions)? Do you have another branch or commit where the shaded libraries are used? These would help us understand the issues. Thanks.
--
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
[GitHub] [ozone] neils-dev commented on a change in pull request #2375: HDDS-5210. Create GRPC service definition and for existing Om protocol
Posted by GitBox <gi...@apache.org>.
neils-dev commented on a change in pull request #2375:
URL: https://github.com/apache/ozone/pull/2375#discussion_r667634267
##########
File path: hadoop-ozone/interface-client/pom.xml
##########
@@ -51,16 +71,46 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
<extensions>true</extensions>
<executions>
<execution>
- <id>compile-protoc</id>
+ <id>compile-protoc-OmGrpc</id>
<goals>
<goal>compile</goal>
<goal>test-compile</goal>
+ <goal>compile-custom</goal>
+ <goal>test-compile-custom</goal>
</goals>
<configuration>
+ <protocArtifact>
+ com.google.protobuf:protoc:${protobuf.version}:exe:${os.detected.classifier}
+ </protocArtifact>
<protoSourceRoot>${basedir}/src/main/proto/</protoSourceRoot>
+ <includes>
+ <include>OmClientProtocol.proto</include>
+ </includes>
+ <outputDirectory>target/generated-sources/protobuf/java</outputDirectory>
+ <clearOutputDirectory>false</clearOutputDirectory>
+ <pluginId>grpc-java</pluginId>
+ <pluginArtifact>
+ io.grpc:protoc-gen-grpc-java:${grpc-compile.version}:exe:${os.detected.classifier}
Review comment:
Thanks @adoroszlai for your comments. The feature branch was arranged by @elek and I have been working on a local branch submitting PRs to merge into the 'feature branch'. Note that we have a very large PR, #2259 that we have broken into smaller incremental PRs for submission, starting with this PR #2375. Ping @elek on updating the feature branch with master - merge `master` to feature branch `HDDS-4440-s3-performance`.
##########
File path: hadoop-ozone/interface-client/pom.xml
##########
@@ -51,16 +71,46 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
<extensions>true</extensions>
<executions>
<execution>
- <id>compile-protoc</id>
+ <id>compile-protoc-OmGrpc</id>
<goals>
<goal>compile</goal>
<goal>test-compile</goal>
+ <goal>compile-custom</goal>
+ <goal>test-compile-custom</goal>
</goals>
<configuration>
+ <protocArtifact>
+ com.google.protobuf:protoc:${protobuf.version}:exe:${os.detected.classifier}
+ </protocArtifact>
<protoSourceRoot>${basedir}/src/main/proto/</protoSourceRoot>
+ <includes>
+ <include>OmClientProtocol.proto</include>
+ </includes>
+ <outputDirectory>target/generated-sources/protobuf/java</outputDirectory>
+ <clearOutputDirectory>false</clearOutputDirectory>
+ <pluginId>grpc-java</pluginId>
+ <pluginArtifact>
+ io.grpc:protoc-gen-grpc-java:${grpc-compile.version}:exe:${os.detected.classifier}
+ </pluginArtifact>
+ </configuration>
+ </execution>
+ <execution>
Review comment:
Thanks for uncovering the duplication in executions. We can also rewrite the two executions into a single execution by having the _`compile-OmGrpc`_ execution id also generate the protoc generated code that id _`compile-protoc-2`_ currently compiles, namely the `<include>Security.proto</include>`. In this case the single execution is :
```
<execution>
<id>compile-protoc-OmGrpc</id>
<goals>
<goal>compile</goal>
<goal>test-compile</goal>
<goal>compile-custom</goal>
<goal>test-compile-custom</goal>
</goals>
<configuration>
<protocArtifact>
com.google.protobuf:protoc:${protobuf.version}:exe:${os.detected.classifier}
</protocArtifact>
<protoSourceRoot>${basedir}/src/main/proto/</protoSourceRoot>
<includes>
<include>OmClientProtocol.proto</include>
<include>Security.proto</include>
</includes>
<outputDirectory>target/generated-sources/protobuf/java</outputDirectory>
<clearOutputDirectory>false</clearOutputDirectory>
<pluginId>grpc-java</pluginId>
<pluginArtifact>
io.grpc:protoc-gen-grpc-java:${grpc-compile.version}:exe:${os.detected.classifier}
</pluginArtifact>
</configuration>
</execution>
```
Either is fine, spit the two or have a single execution - what are your thoughts on this?
--
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
[GitHub] [ozone] neils-dev commented on a change in pull request #2375: HDDS-5210. Create GRPC service definition and for existing Om protocol
Posted by GitBox <gi...@apache.org>.
neils-dev commented on a change in pull request #2375:
URL: https://github.com/apache/ozone/pull/2375#discussion_r668151585
##########
File path: hadoop-ozone/common/pom.xml
##########
@@ -30,6 +30,21 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
<dependencies>
+ <dependency>
+ <groupId>io.grpc</groupId>
Review comment:
@adoroszlai, as suggested see attached for sample errors when using shaded libs.
[protoc_generated_errors_shaded.txt](https://github.com/apache/ozone/files/6803515/protoc_generated_errors_shaded.txt)
--
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
[GitHub] [ozone] neils-dev commented on a change in pull request #2375: HDDS-5210. Create GRPC service definition and for existing Om protocol
Posted by GitBox <gi...@apache.org>.
neils-dev commented on a change in pull request #2375:
URL: https://github.com/apache/ozone/pull/2375#discussion_r670076840
##########
File path: hadoop-ozone/common/pom.xml
##########
@@ -30,6 +30,21 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
<dependencies>
+ <dependency>
+ <groupId>io.grpc</groupId>
+ <artifactId>grpc-netty</artifactId>
+ <version>${io.grpc.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>io.netty</groupId>
+ <artifactId>netty-codec-http2</artifactId>
+ <version>${netty.version}</version>
Review comment:
Reflected in latest commit, removed unnecessary version declarations in pom files. Thanks @adoroszlai.
--
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
[GitHub] [ozone] neils-dev commented on a change in pull request #2375: HDDS-5210. Create GRPC service definition and for existing Om protocol
Posted by GitBox <gi...@apache.org>.
neils-dev commented on a change in pull request #2375:
URL: https://github.com/apache/ozone/pull/2375#discussion_r669821785
##########
File path: hadoop-ozone/common/pom.xml
##########
@@ -30,6 +30,21 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
<dependencies>
+ <dependency>
+ <groupId>io.grpc</groupId>
Review comment:
I'll provide a link to a branch you can view and compile from...
--
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
[GitHub] [ozone] adoroszlai commented on a change in pull request #2375: HDDS-5210. Create GRPC service definition and for existing Om protocol
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #2375:
URL: https://github.com/apache/ozone/pull/2375#discussion_r666814837
##########
File path: hadoop-ozone/interface-client/pom.xml
##########
@@ -51,16 +71,46 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
<extensions>true</extensions>
<executions>
<execution>
- <id>compile-protoc</id>
+ <id>compile-protoc-OmGrpc</id>
<goals>
<goal>compile</goal>
<goal>test-compile</goal>
+ <goal>compile-custom</goal>
+ <goal>test-compile-custom</goal>
</goals>
<configuration>
+ <protocArtifact>
+ com.google.protobuf:protoc:${protobuf.version}:exe:${os.detected.classifier}
+ </protocArtifact>
<protoSourceRoot>${basedir}/src/main/proto/</protoSourceRoot>
+ <includes>
+ <include>OmClientProtocol.proto</include>
+ </includes>
+ <outputDirectory>target/generated-sources/protobuf/java</outputDirectory>
+ <clearOutputDirectory>false</clearOutputDirectory>
+ <pluginId>grpc-java</pluginId>
+ <pluginArtifact>
+ io.grpc:protoc-gen-grpc-java:${grpc-compile.version}:exe:${os.detected.classifier}
+ </pluginArtifact>
+ </configuration>
+ </execution>
+ <execution>
Review comment:
To reduce duplication between the two executions, I suggest:
* moving common configuration outside `<executions>` into a plugin-level `<configuration>`
* separating `protobuf` and `grpc` compilation:
* let `compile-protoc-2` compile all proto files
* let `compile-OmGrpc` compile OM proto only with `grpc-java` plugin
Something like this:
```xml
<plugin>
<groupId>org.xolstice.maven.plugins</groupId>
<artifactId>protobuf-maven-plugin</artifactId>
<version>${protobuf-maven-plugin.version}</version>
<extensions>true</extensions>
<configuration>
<protoSourceRoot>${basedir}/src/main/proto/</protoSourceRoot>
<outputDirectory>target/generated-sources/java</outputDirectory>
<clearOutputDirectory>false</clearOutputDirectory>
</configuration>
<executions>
<execution>
<id>compile-protoc-2</id>
<goals>
<goal>compile</goal>
<goal>test-compile</goal>
</goals>
<configuration>
<protocArtifact>
com.google.protobuf:protoc:${protobuf.version}:exe:${os.detected.classifier}
</protocArtifact>
</configuration>
</execution>
<execution>
<id>compile-grpc</id>
<goals>
<goal>compile-custom</goal>
<goal>test-compile-custom</goal>
</goals>
<configuration>
<includes>
<include>OmClientProtocol.proto</include>
</includes>
<pluginId>grpc-java</pluginId>
<pluginArtifact>
io.grpc:protoc-gen-grpc-java:${io.grpc.version}:exe:${os.detected.classifier}
</pluginArtifact>
</configuration>
</execution>
</executions>
</plugin>
```
##########
File path: hadoop-ozone/interface-client/pom.xml
##########
@@ -51,16 +71,46 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
<extensions>true</extensions>
<executions>
<execution>
- <id>compile-protoc</id>
+ <id>compile-protoc-OmGrpc</id>
<goals>
<goal>compile</goal>
<goal>test-compile</goal>
+ <goal>compile-custom</goal>
+ <goal>test-compile-custom</goal>
</goals>
<configuration>
+ <protocArtifact>
+ com.google.protobuf:protoc:${protobuf.version}:exe:${os.detected.classifier}
+ </protocArtifact>
<protoSourceRoot>${basedir}/src/main/proto/</protoSourceRoot>
+ <includes>
+ <include>OmClientProtocol.proto</include>
+ </includes>
+ <outputDirectory>target/generated-sources/protobuf/java</outputDirectory>
+ <clearOutputDirectory>false</clearOutputDirectory>
+ <pluginId>grpc-java</pluginId>
+ <pluginArtifact>
+ io.grpc:protoc-gen-grpc-java:${grpc-compile.version}:exe:${os.detected.classifier}
Review comment:
I think this should reference `${io.grpc.version}` for consistency.
--
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
[GitHub] [ozone] adoroszlai commented on a change in pull request #2375: HDDS-5210. Create GRPC service definition and for existing Om protocol
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #2375:
URL: https://github.com/apache/ozone/pull/2375#discussion_r667635317
##########
File path: hadoop-ozone/interface-client/pom.xml
##########
@@ -51,16 +71,46 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
<extensions>true</extensions>
<executions>
<execution>
- <id>compile-protoc</id>
+ <id>compile-protoc-OmGrpc</id>
<goals>
<goal>compile</goal>
<goal>test-compile</goal>
+ <goal>compile-custom</goal>
+ <goal>test-compile-custom</goal>
</goals>
<configuration>
+ <protocArtifact>
+ com.google.protobuf:protoc:${protobuf.version}:exe:${os.detected.classifier}
+ </protocArtifact>
<protoSourceRoot>${basedir}/src/main/proto/</protoSourceRoot>
+ <includes>
+ <include>OmClientProtocol.proto</include>
+ </includes>
+ <outputDirectory>target/generated-sources/protobuf/java</outputDirectory>
+ <clearOutputDirectory>false</clearOutputDirectory>
+ <pluginId>grpc-java</pluginId>
+ <pluginArtifact>
+ io.grpc:protoc-gen-grpc-java:${grpc-compile.version}:exe:${os.detected.classifier}
+ </pluginArtifact>
+ </configuration>
+ </execution>
+ <execution>
Review comment:
Depends on whether you need GRPC generated code for `Security.proto`, I guess.
--
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
[GitHub] [ozone] adoroszlai commented on pull request #2375: HDDS-5210. Create GRPC service definition and for existing Om protocol
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #2375:
URL: https://github.com/apache/ozone/pull/2375#issuecomment-889139898
Thanks @neils-dev for the contribution, @xiaoyuyao for the 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
[GitHub] [ozone] neils-dev commented on a change in pull request #2375: HDDS-5210. Create GRPC service definition and for existing Om protocol
Posted by GitBox <gi...@apache.org>.
neils-dev commented on a change in pull request #2375:
URL: https://github.com/apache/ozone/pull/2375#discussion_r665867631
##########
File path: hadoop-ozone/common/pom.xml
##########
@@ -30,6 +30,21 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
<dependencies>
+ <dependency>
+ <groupId>io.grpc</groupId>
Review comment:
Thanks for reviewing this @xiaoyuyao and for your comments. On the using the ratis grpc/netty libraries, initially I wanted to use them, however they were not compatible with the OzoneManagerProtocol we are realizing and using. Works with the unshaded grpc/netty versions ( tested with 4.1.52.Final and 4.1.63.Final ) together with protoc compiler (version 2.5.0 and NOT 3.12.0) without issue.
An example of the incompatibility includes the OzoneManagerProtocol interfacing with the HddsProtos with the thirdparty libraries.
--
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
[GitHub] [ozone] neils-dev commented on a change in pull request #2375: HDDS-5210. Create GRPC service definition and for existing Om protocol
Posted by GitBox <gi...@apache.org>.
neils-dev commented on a change in pull request #2375:
URL: https://github.com/apache/ozone/pull/2375#discussion_r668461776
##########
File path: hadoop-ozone/interface-client/pom.xml
##########
@@ -51,16 +71,46 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
<extensions>true</extensions>
<executions>
<execution>
- <id>compile-protoc</id>
+ <id>compile-protoc-OmGrpc</id>
<goals>
<goal>compile</goal>
<goal>test-compile</goal>
+ <goal>compile-custom</goal>
+ <goal>test-compile-custom</goal>
</goals>
<configuration>
+ <protocArtifact>
+ com.google.protobuf:protoc:${protobuf.version}:exe:${os.detected.classifier}
+ </protocArtifact>
<protoSourceRoot>${basedir}/src/main/proto/</protoSourceRoot>
+ <includes>
+ <include>OmClientProtocol.proto</include>
+ </includes>
+ <outputDirectory>target/generated-sources/protobuf/java</outputDirectory>
+ <clearOutputDirectory>false</clearOutputDirectory>
+ <pluginId>grpc-java</pluginId>
+ <pluginArtifact>
+ io.grpc:protoc-gen-grpc-java:${grpc-compile.version}:exe:${os.detected.classifier}
Review comment:
Updated with commit with fixes reflecting comments and synched with rebased to Master feature branch.
--
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
[GitHub] [ozone] neils-dev commented on a change in pull request #2375: HDDS-5210. Create GRPC service definition and for existing Om protocol
Posted by GitBox <gi...@apache.org>.
neils-dev commented on a change in pull request #2375:
URL: https://github.com/apache/ozone/pull/2375#discussion_r669963701
##########
File path: hadoop-ozone/common/pom.xml
##########
@@ -30,6 +30,21 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
<dependencies>
+ <dependency>
+ <groupId>io.grpc</groupId>
Review comment:
@adoroszlai : code resulted in error; commit with shaded libraries used found in local branch https://github.com/neils-dev/ozone/tree/HDDS-5210_thirdPartyProtoc. Ozone build should give you a sample of errors. Thanks.
--
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
[GitHub] [ozone] adoroszlai merged pull request #2375: HDDS-5210. Create GRPC service definition and for existing Om protocol
Posted by GitBox <gi...@apache.org>.
adoroszlai merged pull request #2375:
URL: https://github.com/apache/ozone/pull/2375
--
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
[GitHub] [ozone] adoroszlai commented on a change in pull request #2375: HDDS-5210. Create GRPC service definition and for existing Om protocol
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #2375:
URL: https://github.com/apache/ozone/pull/2375#discussion_r667688376
##########
File path: hadoop-ozone/interface-client/pom.xml
##########
@@ -51,16 +71,46 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
<extensions>true</extensions>
<executions>
<execution>
- <id>compile-protoc</id>
+ <id>compile-protoc-OmGrpc</id>
<goals>
<goal>compile</goal>
<goal>test-compile</goal>
+ <goal>compile-custom</goal>
+ <goal>test-compile-custom</goal>
</goals>
<configuration>
+ <protocArtifact>
+ com.google.protobuf:protoc:${protobuf.version}:exe:${os.detected.classifier}
+ </protocArtifact>
<protoSourceRoot>${basedir}/src/main/proto/</protoSourceRoot>
+ <includes>
+ <include>OmClientProtocol.proto</include>
+ </includes>
+ <outputDirectory>target/generated-sources/protobuf/java</outputDirectory>
+ <clearOutputDirectory>false</clearOutputDirectory>
+ <pluginId>grpc-java</pluginId>
+ <pluginArtifact>
+ io.grpc:protoc-gen-grpc-java:${grpc-compile.version}:exe:${os.detected.classifier}
Review comment:
Thanks @neils-dev for the info. I have updated the feature branch after all [CI checks passed](https://github.com/adoroszlai/hadoop-ozone/actions/runs/1021704155) for the same commit in my 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
[GitHub] [ozone] adoroszlai commented on a change in pull request #2375: HDDS-5210. Create GRPC service definition and for existing Om protocol
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #2375:
URL: https://github.com/apache/ozone/pull/2375#discussion_r669687866
##########
File path: hadoop-ozone/common/pom.xml
##########
@@ -30,6 +30,21 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
<dependencies>
+ <dependency>
+ <groupId>io.grpc</groupId>
Review comment:
Thanks, but as mentioned, it would be more useful with the code that resulted in these errors.
--
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
[GitHub] [ozone] xiaoyuyao commented on a change in pull request #2375: HDDS-5210. Create GRPC service definition and for existing Om protocol
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #2375:
URL: https://github.com/apache/ozone/pull/2375#discussion_r664933585
##########
File path: hadoop-ozone/common/pom.xml
##########
@@ -30,6 +30,21 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
<dependencies>
+ <dependency>
+ <groupId>io.grpc</groupId>
Review comment:
Most of Ozone modules use shaded grpc/netty from ratis third party library work . Is there a reason to bring the unshaded grpcy/netty dependency?
--
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
[GitHub] [ozone] neils-dev commented on a change in pull request #2375: HDDS-5210. Create GRPC service definition and for existing Om protocol
Posted by GitBox <gi...@apache.org>.
neils-dev commented on a change in pull request #2375:
URL: https://github.com/apache/ozone/pull/2375#discussion_r668460760
##########
File path: hadoop-ozone/interface-client/pom.xml
##########
@@ -51,16 +71,46 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
<extensions>true</extensions>
<executions>
<execution>
- <id>compile-protoc</id>
+ <id>compile-protoc-OmGrpc</id>
<goals>
<goal>compile</goal>
<goal>test-compile</goal>
+ <goal>compile-custom</goal>
+ <goal>test-compile-custom</goal>
</goals>
<configuration>
+ <protocArtifact>
+ com.google.protobuf:protoc:${protobuf.version}:exe:${os.detected.classifier}
+ </protocArtifact>
<protoSourceRoot>${basedir}/src/main/proto/</protoSourceRoot>
+ <includes>
+ <include>OmClientProtocol.proto</include>
+ </includes>
+ <outputDirectory>target/generated-sources/protobuf/java</outputDirectory>
+ <clearOutputDirectory>false</clearOutputDirectory>
+ <pluginId>grpc-java</pluginId>
+ <pluginArtifact>
+ io.grpc:protoc-gen-grpc-java:${grpc-compile.version}:exe:${os.detected.classifier}
Review comment:
Thanks @adoroszlai for pointing out the grpc compiler versioning. Updated for consistency interface-client (ozone) from using `${grpc-compile.version}` with `io.grpc:protock-gen-grpc-java` to `${io.grpc.version}` as suggested. Following that, removed `grpc-compile.version` from entire build (_pom.xml_) and updated `interface-client `(hdds), `interface-server` (hdds), `ozone-csi ` build files (_pom.xml_) to use `${io.grpc.version}`.
--
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