Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrating Protobuf with OpenSearch Transport #7131

Closed
wants to merge 11 commits into from

Conversation

VachaShah
Copy link
Collaborator

@VachaShah VachaShah commented Apr 12, 2023

Description

The change in this PR is a WIP POC for integrating protobuf end-to-end for one OpenSearch API: cat nodes.

Once protobuf is end-to-end integrated for this API, we will test the API and run benchmarks to see the level of improvements. Once the benchmarks are finalized, we will start integrating protobuf in the codebase.

When the change is ready to be added in the codebase, the files in this PR will be divided into multiple parts to make the changes readable and easy to review. For now, this PR shows the scope of the project.

The idea is to provide a separate Transport option for protobuf along with the current Transport which uses Java serialization and deserialization.

Things to note in this PR:

  • A separate Transport layer with protobuf integration using CodedInputStream and CodedOutputStream from protobuf instead of StreamInput and StreamOutput.
  • New classes related to request and response for Cluster, Nodes API etc. have been created with protobuf integration for the sake of this end-to-end POC. When we decide to commit this code in the repo after benchmarks, we should be able to modify the existing classes to integrate with protobuf. It is not done right now since changing these classes results in a lot of changes to dependent classes which are not required for POC.

Issues Resolved

#6844

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@VachaShah VachaShah force-pushed the protobuf-writeable branch from 4f311b4 to f60609c Compare April 22, 2023 00:03
@VachaShah VachaShah changed the title Implementing a Writeable for Protobuf Integrating Protobuf with OpenSearch Transport Apr 22, 2023
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Vacha Shah <[email protected]>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.repositories.azure.AzureBlobContainerRetriesTests.testReadBlobWithRetries

@codecov-commenter
Copy link

codecov-commenter commented Apr 25, 2023

Codecov Report

Merging #7131 (d2c709d) into main (0763d02) will decrease coverage by 0.17%.
The diff coverage is 0.00%.

❗ Current head d2c709d differs from pull request most recent head 3566241. Consider uploading reports for the commit 3566241 to get more accurate results

@@             Coverage Diff              @@
##               main    #7131      +/-   ##
============================================
- Coverage     70.72%   70.55%   -0.17%     
+ Complexity    59568    59508      -60     
============================================
  Files          4862     4873      +11     
  Lines        285530   285947     +417     
  Branches      41153    41201      +48     
============================================
- Hits         201936   201750     -186     
- Misses        66980    67650     +670     
+ Partials      16614    16547      -67     
Impacted Files Coverage Δ
...ensearch/common/io/stream/ProtobufStreamInput.java 0.00% <0.00%> (ø)
...nsearch/common/io/stream/ProtobufStreamOutput.java 0.00% <0.00%> (ø)
.../org/opensearch/tasks/ProtobufCancellableTask.java 0.00% <0.00%> (ø)
...c/main/java/org/opensearch/tasks/ProtobufTask.java 0.00% <0.00%> (ø)
...org/opensearch/tasks/ProtobufTaskAwareRequest.java 0.00% <0.00%> (ø)
...main/java/org/opensearch/tasks/ProtobufTaskId.java 0.00% <0.00%> (ø)
...in/java/org/opensearch/tasks/ProtobufTaskInfo.java 0.00% <0.00%> (ø)
...rg/opensearch/tasks/ProtobufTaskResourceStats.java 0.00% <0.00%> (ø)
.../java/org/opensearch/tasks/ProtobufTaskResult.java 0.00% <0.00%> (ø)
...opensearch/transport/ProtobufTransportMessage.java 0.00% <0.00%> (ø)
... and 1 more

... and 477 files with indirect coverage changes

@github-actions
Copy link
Contributor

github-actions bot commented May 8, 2023

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Vacha Shah <[email protected]>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Vacha Shah <[email protected]>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@opensearch-trigger-bot
Copy link
Contributor

This PR is stalled because it has been open for 30 days with no activity. Remove stalled label or comment or this will be closed in 7 days.

@opensearch-trigger-bot opensearch-trigger-bot bot added the stalled Issues that have stalled label Jul 30, 2023
@VachaShah
Copy link
Collaborator Author

Will reopen a new one with the latest changes and some code clean up.

@VachaShah VachaShah closed this Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stalled Issues that have stalled
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants