apache / dolphinscheduler

Apache DolphinScheduler is the modern data orchestration platform. Agile to create high performance workflow with low-code
https://dolphinscheduler.apache.org/
Apache License 2.0
12.73k stars 4.58k forks source link

[DSIP-9][registry] add raft registration plugin #16352

Open MatthewAden opened 2 months ago

MatthewAden commented 2 months ago

Purpose of the pull request

This is a project for the OSPP 2024. This PR is close #10874

Brief change log

Verify this pull request

Unit Test Results:

image

Pseudo-Cluster Deployment Status:

3081722177424_ pic 3091722177435_ pic

Cluster Test

Test conditions: 2 master nodes, 1 worker node OS: mac Steps: Start node1, node2, and node3 in sequence

boring-cyborg[bot] commented 2 months ago

Thanks for opening this pull request! Please check out our contributing guidelines. (https://github.com/apache/dolphinscheduler/blob/dev/docs/docs/en/contribute/join/pull-request.md)

MatthewAden commented 1 month ago

If there are no master can this work? We should select the leader from the whole cluster rather than select the leader from master, I didn't see any design related to this PR, you should provide the newly design doc related to this PR.

Hi, this is an OSPP project. My mentor is @zhuxt2015. The design plan for the raft plugin has been discussed. The design link is Add new registry plugin based on raft. According to the discussed plan, masters will form a raft cluster, and the leader will only be elected among the masters, just like in ZK. For the three master nodes, at least two must be alive to provide services normally.

sonarcloud[bot] commented 2 weeks ago

Please retry analysis of this Pull-Request directly on SonarCloud

SbloodyS commented 5 days ago

Please add mysql and postgresql with raft registry cluster test in CI. You can refer to https://github.com/apache/dolphinscheduler/tree/dev/.github/workflows/cluster-test

ruanwenjun commented 1 day ago

If there are no master can this work? We should select the leader from the whole cluster rather than select the leader from master, I didn't see any design related to this PR, you should provide the newly design doc related to this PR.

Hi, this is an OSPP project. My mentor is @zhuxt2015. The design plan for the raft plugin has been discussed. The design link is Add new registry plugin based on raft. According to the discussed plan, masters will form a raft cluster, and the leader will only be elected among the masters, just like in ZK. For the three master nodes, at least two must be alive to provide services normally.

If the cluster does have master, then the cluster will not work? This is unacceptable.

zhuxt2015 commented 1 day ago

If there are no master can this work? We should select the leader from the whole cluster rather than select the leader from master, I didn't see any design related to this PR, you should provide the newly design doc related to this PR.

Hi, this is an OSPP project. My mentor is @zhuxt2015. The design plan for the raft plugin has been discussed. The design link is Add new registry plugin based on raft. According to the discussed plan, masters will form a raft cluster, and the leader will only be elected among the masters, just like in ZK. For the three master nodes, at least two must be alive to provide services normally.

If the cluster does have master, then the cluster will not work? This is unacceptable.

Just like zk plugin, if the zk cluster goes down, the ds cluster will also go down.

ruanwenjun commented 1 day ago

If there are no master can this work? We should select the leader from the whole cluster rather than select the leader from master, I didn't see any design related to this PR, you should provide the newly design doc related to this PR.

Hi, this is an OSPP project. My mentor is @zhuxt2015. The design plan for the raft plugin has been discussed. The design link is Add new registry plugin based on raft. According to the discussed plan, masters will form a raft cluster, and the leader will only be elected among the masters, just like in ZK. For the three master nodes, at least two must be alive to provide services normally.

If the cluster does have master, then the cluster will not work? This is unacceptable.

Just like zk plugin, if the zk cluster goes down, the ds cluster will also go down.

If so, big -1 for this plugin, this doesn't help with SLA, I can not find any reason why we wouldn't use jdbc registry plugin instead of this plugin.

SbloodyS commented 1 day ago

If so, big -1 for this plugin, this doesn't help with SLA, I can not find any reason why we wouldn't use jdbc registry plugin instead > of this plugin.

+1, High availability and stability are the first things we should ensure. Any new function should not be based on lowering the standards of these two.

zhuxt2015 commented 1 day ago

If so, big -1 for this plugin, this doesn't help with SLA, I can not find any reason why we wouldn't use jdbc registry plugin instead > of this plugin.

+1, High availability and stability are the first things we should ensure. Any new function should not be based on lowering the standards of these two.

This plugin does not reduce the stability and availability of the ds cluster, and will maintain the same impact on the cluster as other plugins.

SbloodyS commented 1 day ago

This plugin does not reduce the stability and availability of the ds cluster, and will maintain the same impact on the cluster as other plugins.

If the cluster does have master, then the cluster will not work.

This is against zk and jdbc registry since neither of these two types will cause the worker server and alert server to fail to operate normally because of the downtime of the master server.

zhuxt2015 commented 1 day ago

This plugin does not reduce the stability and availability of the ds cluster, and will maintain the same impact on the cluster as other plugins.

If the cluster does have master, then the cluster will not work.

This is against zk and jdbc registry since neither of these two types will cause the worker server and alert server to fail to operate normally because of the downtime of the master server.

Raft plugin won‘t do that either.

SbloodyS commented 17 hours ago

Raft plugin won‘t do that either.

Based on this PR, this problem exists. Why do you say this?

zhuxt2015 commented 16 hours ago

Raft plugin won‘t do that either.

Based on this PR, this problem exists. Why do you say this?

I’m sorry that I didn‘t explain it clearly. I mean we will fix this problem.