8000 Follow raftlog update by yuezato · Pull Request #344 · frugalos/frugalos · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Follow raftlog update #344

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

Draft
wants to merge 20 commits into
base: develop
Choose a base branch
from
Draft

Conversation

yuezato
Copy link
Member
@yuezato yuezato commented Oct 21, 2021

Ready for Review(の状態通知ですが、出し直します。)

Types of changes

Please check one of the following:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • New release (merge to both master and develop!)

Description of changes

Behavior

RaftlogのIo-traitが新たなメソッド delete_suffix_from を要求し、
これに対応する実装として Storage::delete_suffix_from を追加しています。

このメソッドは、LogIndex from 以降全て `[from..∞)' のsuffix要素を削除するためのものです。

これに対応する実装として、追加するコードでは以下のことをします:

  1. メモリ中のlog suffixのcacheである log_suffix を削除し
  2. DeleteSuffixRange futureを作成し、その中でCannylsのrange削除を呼び出します。

Purpose

Raftlog側のissue frugalos/raftlog#18 を解決するための
PR frugalos/raftlog#42 によって
RaftlogのIOのインタフェイスが変更になったので、その追従をします。

インタフェイスの変更は、新しいメソッドの追加によるもので
既存コードの変更は不要ですが、コードの追加が必要になります。
参考: https://github.com/frugalos/raftlog/blob/25d663b3e8eda35224cd666e1f1ef05b44ace884/src/io.rs#L89-L91

How to test this PR

追加したメソッドは、Raftlogから特定ケースで呼ばれるのですが、
この特定ケースをFrugalosレベルから再現するのは現時点では困難です。

従って、frugalos_raft内部でのユニットテストを提供しています。
実装に対して確認したいことがある場合は、可能な限りこのPRをマージする前に、
テストの追加ということで対応したいです。

現時点では以下の項目をテストしています:

Checklists

  • コードの追加

  • 実装コメントの付与

  • テスト---範囲削除が想定通り動くかだけを直接的に(frugalos_raftの中だけで)確認します

  • エラーハンドリング

  • Run cargo fmt --all.

  • Run cargo clippy --all --all-targets.

brly and others added 20 commits September 19, 2019 19:09
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.

5 participants
0