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/08/03 01:26:02 UTC
[GitHub] [ozone] neils-dev opened a new pull request #2485: HDDS-5213. Create and a specific OmTransportFactory for GrpcOmTransport
neils-dev opened a new pull request #2485:
URL: https://github.com/apache/ozone/pull/2485
…NF/services api. Modifications made for green build targeted to s3 grpc incrementental PRs. Integration tests are now independent of s3g implementation (through pom file changes). Smoketests s3g specific are temp not run, delayed until further implementation submitted to support functionality.
## What changes were proposed in this pull request?
Implementation to invoke gRPC transport om factory through serviceprovider _META-INF/services SPI_. Create feature branch environment to support incremental feature PRs for s3g gRPC ozone client, OMProtocol, transport and server. Commit provides green build by modifying the integration tests, acceptance tests and kubernetes setup. Specifically, integrations tests are now intentionally independent of s3g implementation through modified pom files and configuration tests. S3g specific acceptance tests are temporarily disabled (through shell script modifications, test.sh) until supported functionality is submitted in subsequent PRs.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5213?filter=-1&jql=resolution%20in%20(Unresolved%2C%20Fixed)%20AND%20assignee%20in%20(currentUser())%20order%20by%20updated%20DESC
## How was this patch tested?
Manually tested. Invoke _s3g_ to trigger _serviceprovider interface_ attempting to invoke `s3g gRPC` specific _OmClientFactory_. Through docker ozone cluster:
```
$ cd hadoop-ozone/dist/target/ozone-1.2.0-SNAPSHOT/compose/ozone
$ docker-compose up -d --scale datanode=3
$ docker-compose logs -f s3g
in other terminal trigger s3 request through gate using shell aws cli;
$ aws s3api --endpoint http://localhost:9878 create-bucket --bucket=bucket1
```
See attached image for error occurring due to _serviceprovider_ interface invoking _GrpcOmTransportFactory_ (not included in this PR).
--
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 #2485: HDDS-5213. Create and a specific OmTransportFactory for GrpcOmTransport
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #2485:
URL: https://github.com/apache/ozone/pull/2485#discussion_r692268450
##########
File path: hadoop-ozone/dev-support/checks/_lib.sh
##########
@@ -73,7 +73,7 @@ install_k3s() {
}
_install_k3s() {
- curl -sfL https://get.k3s.io | sh -
+ curl -sfL https://get.k3s.io | INSTALL_K3S_VERSION="v1.21.2+k3s1" sh -
Review comment:
These changes to CI check library seem to be unrelated.
--
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 pull request #2485: HDDS-5213. Create and a specific OmTransportFactory for GrpcOmTransport
Posted by GitBox <gi...@apache.org>.
neils-dev commented on pull request #2485:
URL: https://github.com/apache/ozone/pull/2485#issuecomment-905172455
Thanks @adoroszlai, @kerneltime, @swagle
--
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 #2485: HDDS-5213. Create and a specific OmTransportFactory for GrpcOmTransport
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #2485:
URL: https://github.com/apache/ozone/pull/2485#issuecomment-895344318
> the feature is incrementally submitted as PRs to merge into the feature branch
That's fine. The question is: why not follow the logical order of the initial four sub-tasks? Why create a META-INF service descriptor pointing to a non-existing class? Creating a PR for HDDS-5212 first would save you from disabling lots of tests, then enabling them again in the next PR.
--
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 #2485: HDDS-5213. Create and a specific OmTransportFactory for GrpcOmTransport
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #2485:
URL: https://github.com/apache/ozone/pull/2485#discussion_r692676242
##########
File path: hadoop-ozone/dev-support/checks/_lib.sh
##########
@@ -73,7 +73,7 @@ install_k3s() {
}
_install_k3s() {
- curl -sfL https://get.k3s.io | sh -
+ curl -sfL https://get.k3s.io | INSTALL_K3S_VERSION="v1.21.2+k3s1" sh -
Review comment:
These kinds of fixes should be brought into the feature branch via merge from `master`.
--
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 #2485: HDDS-5213. Create and a specific OmTransportFactory for GrpcOmTransport
Posted by GitBox <gi...@apache.org>.
neils-dev commented on a change in pull request #2485:
URL: https://github.com/apache/ozone/pull/2485#discussion_r692608879
##########
File path: hadoop-ozone/dev-support/checks/_lib.sh
##########
@@ -73,7 +73,7 @@ install_k3s() {
}
_install_k3s() {
- curl -sfL https://get.k3s.io | sh -
+ curl -sfL https://get.k3s.io | INSTALL_K3S_VERSION="v1.21.2+k3s1" sh -
Review comment:
This a known bug for kubernetes k3s that appeared a few weeks ago due to a version upgrade to k3s (resolved on master) that appears in the feature branch apache:HDDS-4440-s3-performance. The added line to _lib.sh fixes the issue for the 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 #2485: HDDS-5213. Create and a specific OmTransportFactory for GrpcOmTransport
Posted by GitBox <gi...@apache.org>.
neils-dev commented on a change in pull request #2485:
URL: https://github.com/apache/ozone/pull/2485#discussion_r694888523
##########
File path: hadoop-ozone/dev-support/checks/_lib.sh
##########
@@ -73,7 +73,7 @@ install_k3s() {
}
_install_k3s() {
- curl -sfL https://get.k3s.io | sh -
+ curl -sfL https://get.k3s.io | INSTALL_K3S_VERSION="v1.21.2+k3s1" sh -
Review comment:
Thanks @adoroszlai ; updated commit with your update to the 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 pull request #2485: HDDS-5213. Create and a specific OmTransportFactory for GrpcOmTransport
Posted by GitBox <gi...@apache.org>.
neils-dev commented on pull request #2485:
URL: https://github.com/apache/ozone/pull/2485#issuecomment-902336959
Thanks @adoroszlai for setting up a sub-task to track disabled tests. I have put those (will update as needed) in sub-task HDDS-5648.
--
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] swagle merged pull request #2485: HDDS-5213. Create and a specific OmTransportFactory for GrpcOmTransport
Posted by GitBox <gi...@apache.org>.
swagle merged pull request #2485:
URL: https://github.com/apache/ozone/pull/2485
--
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] swagle commented on pull request #2485: HDDS-5213. Create and a specific OmTransportFactory for GrpcOmTransport
Posted by GitBox <gi...@apache.org>.
swagle commented on pull request #2485:
URL: https://github.com/apache/ozone/pull/2485#issuecomment-904931644
Thanks, @adoroszlai, and @kerneltime for review. Merging 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 #2485: HDDS-5213. Create and a specific OmTransportFactory for GrpcOmTransport
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #2485:
URL: https://github.com/apache/ozone/pull/2485#discussion_r693315073
##########
File path: hadoop-ozone/dev-support/checks/_lib.sh
##########
@@ -73,7 +73,7 @@ install_k3s() {
}
_install_k3s() {
- curl -sfL https://get.k3s.io | sh -
+ curl -sfL https://get.k3s.io | INSTALL_K3S_VERSION="v1.21.2+k3s1" sh -
Review comment:
Updated `HDDS-4440-s3-performance` from `master`: 3b4e2f58b35f1e2dbf553ea183063867f1fd4771.
--
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 pull request #2485: HDDS-5213. Create and a specific OmTransportFactory for GrpcOmTransport
Posted by GitBox <gi...@apache.org>.
neils-dev commented on pull request #2485:
URL: https://github.com/apache/ozone/pull/2485#issuecomment-895719050
> The question is: why not follow the logical order of the initial four sub-tasks? Why create a META-INF service descriptor pointing to a non-existing class? Creating a PR for [HDDS-5212](https://issues.apache.org/jira/browse/HDDS-5212) first would save you from disabling lots of tests, then enabling them again in the next PR.
Thanks @adoroszlai. Ha, not looking to complicate, we will be able to go through the subtasks - not to worry. It is simpler with the transport factory service provider and the build setup _following_ the gRPC client/server generation (HDDS-5210 submitted and reviewed - thanks!). On the disabling of tests, here we have setup the environment to decouple the s3g implementation from the integration tests and disabled acceptance tests that we won't be able to re-enable until at least the subtasks HDDS-5210 through HDDS-5213 are submitted.
> The question is: why not follow the logical order of the initial four sub-tasks?
It is simpler - proceed the pom changes (HDDS-5210) with setting up the build environ and the service provider. Will follow with the client (dependent on the factory), etc (btw we will be adding subtasks for the s3g ozoneclientproducer & cache, endpoint, cdi filter).
As an side, as we discussed previously offline, I have a sketch of what we can look to do for the incremental PRs and merges to the feature branch for this feature. I'll share it with you and we can discuss.
> That's fine.
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 pull request #2485: HDDS-5213. Create and a specific OmTransportFactory for GrpcOmTransport
Posted by GitBox <gi...@apache.org>.
neils-dev commented on pull request #2485:
URL: https://github.com/apache/ozone/pull/2485#issuecomment-894586807
Thanks for reviewing this and the comments @adoroszlai. Following up on what we discussed earlier offline, the feature is incrementally submitted as PRs to merge into the feature branch. The initial PR contained modifications to the build - pom files associated with gRPC and we would like to proceed the incremental PR submission with a commit that selects the grpc om transport (factory) for the s3g gRPC transport. In addition included in this PR are changes to the system build that provides a 'green build' to build upon (seen with the unit, integration and acceptance test environments). Initially we did not include the implementation for the gRPC om transport factory as it is directly coupled with the client (HDDS-5212). If we can proceed with this, we can look to
1. Change the title of the PR and associated jira to "Create service provider for GrpcOmTransport and supporting build" from "Create and a specific OmTransportFactory for GrpcOmTransport"
2. Follow up with HDDS-5212 including the initial client side implementation.
> Thanks @neils-dev for working on this.
>
> > GrpcOmTransportFactory (not included in this PR).
>
> I might be missing something, but not including `GrpcOmTransportFactory` in a PR with the title "Create and a specific OmTransportFactory for GrpcOmTransport" sounds contradictory.
>
> I think it would be better to publish the `GrpcOmTransport` implemention first ([HDDS-5212](https://issues.apache.org/jira/browse/HDDS-5212)), and only afterwards add the service definition for the factory ([HDDS-5213](https://issues.apache.org/jira/browse/HDDS-5213)). That seems to be the logical order for these subtasks.
--
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 #2485: HDDS-5213. Create and a specific OmTransportFactory for GrpcOmTransport
Posted by GitBox <gi...@apache.org>.
neils-dev commented on a change in pull request #2485:
URL: https://github.com/apache/ozone/pull/2485#discussion_r693098176
##########
File path: hadoop-ozone/dev-support/checks/_lib.sh
##########
@@ -73,7 +73,7 @@ install_k3s() {
}
_install_k3s() {
- curl -sfL https://get.k3s.io | sh -
+ curl -sfL https://get.k3s.io | INSTALL_K3S_VERSION="v1.21.2+k3s1" sh -
Review comment:
Thanks @adoroszlai for the comment on bugs fixed on master that affect our feature branch. Can you please apply it to the feature branch` apache:HDDS-4440-s3-performance` from the `master` through a merge? I'll update our PR and re-run CI afterwords.
--
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] kerneltime commented on pull request #2485: HDDS-5213. Create and a specific OmTransportFactory for GrpcOmTransport
Posted by GitBox <gi...@apache.org>.
kerneltime commented on pull request #2485:
URL: https://github.com/apache/ozone/pull/2485#issuecomment-904007889
@neils-dev are you all set to merge this into the 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