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

Replace GRPC with Dubbo-go in OpenIM Project #869

Closed
9 tasks done
cubxxw opened this issue Aug 15, 2023 — with Slack · 5 comments
Closed
9 tasks done

Replace GRPC with Dubbo-go in OpenIM Project #869

cubxxw opened this issue Aug 15, 2023 — with Slack · 5 comments
Assignees
Labels
feature Categorizes issue or PR as related to a new feature. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines.
Milestone

Comments

Copy link
Contributor

cubxxw commented Aug 15, 2023

Background

As our OpenIM project continues to evolve, we are constantly exploring ways to optimize our tech stack for performance, scalability, and maintainability. With this in mind, we're considering moving away from our current GRPC implementation in favor of Dubbo-go.

Motivation

  • Performance Enhancements: Dubbo-go is known for its high performance and efficient RPC communication.
  • Scalability: Dubbo-go provides out-of-the-box service discovery and other features that can help our project scale with ease.
  • Rich Ecosystem: Dubbo-go has a vast ecosystem, which can be beneficial for future integrations and extensions.

Proposed Changes

  1. Evaluation Phase: Before making any drastic changes, evaluate the pros and cons of this migration. It includes performance testing, evaluating maintainability, and checking compatibility with our existing systems.
  2. Pilot Phase: Start with a small module to replace GRPC with Dubbo-go. This will help in understanding any unforeseen challenges and mitigating them before a full-fledged migration.
  3. Migration Phase: After successful completion of the pilot phase, we can systematically replace GRPC with Dubbo-go across all modules.

Expected Outcomes

  • Enhanced system performance and scalability.
  • Smooth integration with other tools and services in the Dubbo-go ecosystem.
  • Reduction in potential bottlenecks and challenges faced with GRPC.
Checklist
  • internal/rpc/auth/auth.go ✅ Commit ac71c28
  • internal/rpc/conversation/conversation.go ✅ Commit f491d46
  • pkg/rpcclient/auth.go ✅ Commit 146b766
  • pkg/rpcclient/conversation.go ✅ Commit 51bea8f
  • pkg/rpcclient/notification/common.go ⚠️ No Changes Made
  • pkg/rpcclient/notification/conversation.go ✅ Commit 74c114d
  • internal/rpc/auth/doc.go ✅ Commit f75632b
  • pkg/rpcclient/doc.go ✅ Commit da3afac
  • pkg/rpcclient/notification/doc.go ✅ Commit f348faf
@cubxxw cubxxw added good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. feature Categorizes issue or PR as related to a new feature. labels Aug 15, 2023 — with Slack
@cubxxw cubxxw added this to the v3.3 milestone Aug 15, 2023 — with Slack
@cubxxw cubxxw self-assigned this Aug 15, 2023
@kubbot
Copy link
Contributor

kubbot commented Aug 15, 2023

This issue is available for anyone to work on. Make sure to reference this issue in your pull request. ✨ Thank you for your contribution! ✨
Join slack 🤖 to connect and communicate with our developers.
If you wish to accept this assignment, please leave a comment in the comments section: /accept.🎯

@cubxxw cubxxw added the sweep label Aug 15, 2023
@cubxxw cubxxw removed their assignment Aug 15, 2023
@openimsdk openimsdk deleted a comment from sweep-ai bot Aug 15, 2023
@kubbot
Copy link
Contributor

kubbot commented Oct 14, 2023

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@sweep-ai
Copy link
Contributor

sweep-ai bot commented Oct 14, 2023

Here's the PR! #1227.

⚡ Sweep Free Trial: I'm creating this ticket using GPT-4. You have 4 GPT-4 tickets left for the month and 2 for the day. For more GPT-4 tickets, visit our payment portal.

Actions (click)

  • ↻ Restart Sweep
Install Sweep Configs: Pull Request

Step 1: 🔎 Searching

I found the following snippets in your repository. I will now analyze these snippets and come up with a plan.

Some code snippets I looked at (click to expand). If some file is missing from here, you can mention the path in the ticket description.

Step 2: ⌨️ Coding

  • internal/rpc/auth/auth.go ✅ Commit ac71c28
• Replace the GRPC import statement with the Dubbo-go import statement.
• Update the service definition to use Dubbo-go.
• Update the method implementations to use Dubbo-go.
  • internal/rpc/conversation/conversation.go ✅ Commit f491d46
• Replace the GRPC import statement with the Dubbo-go import statement.
• Update the service definition to use Dubbo-go.
• Update the method implementations to use Dubbo-go.
  • pkg/rpcclient/auth.go ✅ Commit 146b766
• Replace the GRPC import statement with the Dubbo-go import statement.
• Update the service definition to use Dubbo-go.
• Update the method implementations to use Dubbo-go.
  • pkg/rpcclient/conversation.go ✅ Commit 51bea8f
• Replace the GRPC import statement with the Dubbo-go import statement.
• Update the service definition to use Dubbo-go.
• Update the method implementations to use Dubbo-go.
  • pkg/rpcclient/notification/common.go ⚠️ No Changes Made
• Replace the GRPC import statement with the Dubbo-go import statement.
• Update the service definition to use Dubbo-go.
• Update the method implementations to use Dubbo-go.
  • pkg/rpcclient/notification/conversation.go ✅ Commit 74c114d
• Replace the GRPC import statement with the Dubbo-go import statement.
• Update the service definition to use Dubbo-go.
• Update the method implementations to use Dubbo-go.
  • internal/rpc/auth/doc.go ✅ Commit f75632b
• Update the package documentation to reflect the change to Dubbo-go.
  • pkg/rpcclient/doc.go ✅ Commit da3afac
• Update the package documentation to reflect the change to Dubbo-go.
  • pkg/rpcclient/notification/doc.go ✅ Commit f348faf
• Update the package documentation to reflect the change to Dubbo-go.

Step 3: 🔁 Code Review

Here are my self-reviews of my changes at sweep/replace-grpc-with-dubbo.

Here is the 1st review

The changes made in the pull request are mostly correct and in line with the plan. However, there are a few potential issues that need to be addressed:
  • In the files 'internal/rpc/auth/auth.go' and 'internal/rpc/conversation/conversation.go', the 'Start' function no longer takes a 'server' parameter. This might break any code that calls this function with a 'server' parameter. Please ensure that all calls to the 'Start' function are updated accordingly.

  • In the files 'pkg/rpcclient/auth.go', 'pkg/rpcclient/conversation.go', and 'pkg/rpcclient/notification/conversation.go', the 'conn' field in the respective structs has been changed from 'grpc.ClientConnInterface' to 'dubbo-go.ClientConnInterface'. This might break any code that uses the 'conn' field. Please ensure that all uses of the 'conn' field are updated accordingly.

Please make these changes and update the pull request.

I finished incorporating these changes.


🎉 Latest improvements to Sweep:

  • Sweep can now passively improve your repository! Check out Rules to learn more.

💡 To recreate the pull request edit the issue title or description. To tweak the pull request, leave a comment on the pull request.
Join Our Discord

@kubbot
Copy link
Contributor

kubbot commented Mar 1, 2024

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@cubxxw cubxxw closed this as completed Mar 2, 2024
@cubxxw cubxxw self-assigned this Mar 2, 2024
@cubxxw
Copy link
Contributor Author

cubxxw commented Mar 2, 2024

I am currently not considering this idea at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Categorizes issue or PR as related to a new feature. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants