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

[ISSUE #202] refine the supporting document for cluster deploy #230

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

HudsonShi
Copy link

…ver' documentation

Fixes #224

Motivation

Refined the supporting document for cluster load balancing and failover
The previous documentation of cluster load balancing and failover is incomplete

Modifications

*Add the supporting document for cluster load balancing and failover in the runtime instruction *

Documentation

  • Does this pull request introduce a new feature?
  • no

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. You should add new content to the current version of the document, rather than modifying the historical document.
  2. According to [ISSUE #202] refine the supporting document for cluster LB and failover #224 (review), you still have the following items pending:
    a. you should use "EventMesh" instead of "event-mesh".
    b. you can add multiple subheadings such as "Using the Registry Center" or "Using a Distributed Consistency Protocol," taking Nacos and JRaft as examples, respectively, to introduce the differences in their support levels. Their usage and configuration differences can be written in another document, placed under the Installation and Deployment section.
    c. The existing content is far from sufficient to explain the cluster features of EventMesh.
  3. You should run the npm run serve command locally to test if the pages you've added display correctly. From what I know, you also need to modify files like categories.json for the new page to display properly.

Comment on lines 13 to 15
- Implementing JRaft eliminates the need for Nacos as a registry center.
- Existing SDK pattern relies on static IP insertion in `eventmesh.properties`.
- Node failure results in service failure.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 14 confuses me; it describes the current state of JRaft, which is unrelated to the section titled "Impact on Registry Center".

Comment on lines 16 to 18
- **Solutions:**
- **Gateway Proxy Layer:** A future solution for the new architecture. Like Nginx.
- **JRaft Protocol:** Enables dynamic scaling and cluster discovery.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The generated traces of LLM in these lines are quite noticeable. For formatting, I suggest using multi-level Markdown headings. Each level of heading can contain a list and corresponding explanations in plain text. There's no need to nest multiple levels of unordered lists together, as this will help with anchor jumping in the page directory.

Copy link
Member

@Pil0tXia Pil0tXia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will try to attend the biweekly meeting next time where we can discuss the full EventMesh cluster deployment details, which should be beneficial to you.

  • Just a friendly reminder, Markdown syntax requires a blank line between the title and the body text.
  • This document needs to be in both Chinese and English languages.

Comment on lines +106 to +117
{
type: 'category',
label: 'Distributed Cluster Deployment',
collapsible: true,
collapsed: false,
items: [
{
type: 'autogenerated',
dirName: 'distributed-cluster-deployment-guide',
},
],
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to place this chapter under the Design Document category.

Once you have written the quick start guide for cluster deployment, this document will need to be placed under the Installation and Deployment category.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will fix it in the 'Design Document' category

Comment on lines +2 to +3
## **Background Knowledge and Current State:**
* Kafka utilizes KRaft for insync replication of message from main broker to following brokers.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a gentle correction, it should be "In-Sync" replication, not "insync" replication. The meaning of these two words in English is completely opposite.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will fix the typo

Comment on lines +18 to +19
## **Using the Reverse Proxy:**
**Current State:** Nginx can be the reverse proxy of EventMesh. In the SDK we set the Nginx IP address, Nginx will transfer the subscription address to a EventMesh runtime.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section requires additional information on nacos.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need your advisory in Nacos deployment.

Comment on lines +20 to +21
### **Solutions: Add Gateway Proxy Layer:** A next generation of functional solution for the new architecture. Like Nginx.
**Current Flaws**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The title is quite lengthy and also, Nginx is not exactly the next-gen solution for the new architecture, but rather a compromise implementation for load balancing as of now.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will fix the title into 'A compromise implementation for load balancing for now'

@@ -0,0 +1,24 @@
# Distributed Consistency Protocol for EventMesh
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Distributed Consistency Protocol is applicable only to protocols akin to KRaft/JRaft and the like. Reverse Proxy does not fall under the purview of the Distributed Consistency Protocol.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will fix the title into 'Distributed Consistency Protocol or Reverse proxy'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can split them into two documents, or use a more general title, such as "Cluster Design."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants