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

feat(meta): Failover handling meta service #6466

Closed
wants to merge 237 commits into from

Conversation

CAJan93
Copy link
Contributor

@CAJan93 CAJan93 commented Nov 18, 2022

I hereby agree to the terms of the Singularity Data, Inc. Contributor License Agreement.

What's changed and what's your intention?

TODOs:

  • Unittests
  • Dummy follower service
  • How do we test that failover handling works?
  • Delete draft file

Checklist

  • I have written necessary rustdoc comments
  • I have added necessary unit tests and integration tests
  • All checks passed in ./risedev check (or alias, ./risedev c)

Documentation

If your pull request contains user-facing changes, please specify the types of the changes, and create a release note. Otherwise, please feel free to remove this section.

Types of user-facing changes

#5943

Please keep the types that apply to your changes, and remove those that do not apply.

  • Installation and deployment
  • Connector (sources & sinks)
  • SQL commands, functions, and operators
  • RisingWave cluster configuration changes
  • Other (please specify in the release note below)

Release note

Please create a release note for your changes. In the release note, focus on the impact on users, and mention the environment or conditions where the impact may occur.

Refer to a related PR or issue link (optional)

@codecov
Copy link

codecov bot commented Nov 18, 2022

Codecov Report

Merging #6466 (aa20b19) into main (8b46da1) will increase coverage by 0.04%.
The diff coverage is n/a.

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

@@            Coverage Diff             @@
##             main    #6466      +/-   ##
==========================================
+ Coverage   73.20%   73.25%   +0.04%     
==========================================
  Files        1024     1023       -1     
  Lines      163741   163598     -143     
==========================================
- Hits       119870   119844      -26     
+ Misses      43871    43754     -117     
Flag Coverage Δ
rust 73.25% <0.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/common/src/config.rs 58.91% <0.00%> (-3.07%) ⬇️
src/frontend/src/handler/create_sink.rs 93.75% <0.00%> (-2.83%) ⬇️
src/frontend/src/handler/util.rs 83.10% <0.00%> (-2.21%) ⬇️
src/expr/src/vector_op/cast.rs 87.92% <0.00%> (-1.88%) ⬇️
src/frontend/src/handler/mod.rs 63.47% <0.00%> (-1.53%) ⬇️
src/frontend/src/handler/explain.rs 52.88% <0.00%> (-0.89%) ⬇️
...ontend/src/optimizer/plan_node/stream_hash_join.rs 85.71% <0.00%> (-0.60%) ⬇️
src/frontend/src/handler/create_source.rs 66.66% <0.00%> (-0.14%) ⬇️
src/frontend/src/handler/create_index.rs 88.41% <0.00%> (-0.12%) ⬇️
src/frontend/planner_test/src/lib.rs 73.55% <0.00%> (-0.09%) ⬇️
... and 25 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@CAJan93 CAJan93 marked this pull request as draft December 15, 2022 12:35
if was_leader {
// && !is_leader {
// TODO: enable this again to give leader chance to claim lease again?
panic!(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

fencing mechanism

@yezizp2012
Copy link
Member

Besides, Why are you maintaining this PR now? 🤔
It seems that you will need to submit all the commits to both the PR you're working on and this one. I think the whole HA tasks is well split, if for test purpose you can test in the PR you're working on. And this PR contains some confusing changes that was introduced maybe in the very early stage. It is difficult or increases your workload to keep up with the main or your new PR.
Well, just some friendly advice. Hope it can save some of your time.

@CAJan93
Copy link
Contributor Author

CAJan93 commented Dec 21, 2022

Besides, Why are you maintaining this PR now? 🤔

I am keeping this PR more or less up to date to test how the current implementations work with the other things we will implement. Furthermore I am using this PR to copy paste things from into the new PRs.

Totally agree with you. Only keeping this PR around for myself. We will not merge it and I am focusing on the actual feature PRs and not on this one.

@CAJan93 CAJan93 closed this Feb 15, 2023
@xxchan xxchan deleted the jm/multi-meta-etcd-grafana branch May 14, 2023 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants