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

Use ClientWriteResponse to replace AddLearnerResponse #679

Closed
drmingdrmer opened this issue Feb 17, 2023 · 1 comment · Fixed by #680
Closed

Use ClientWriteResponse to replace AddLearnerResponse #679

drmingdrmer opened this issue Feb 17, 2023 · 1 comment · Fixed by #680
Assignees

Comments

@drmingdrmer
Copy link
Member

In openraft adds a learner is done by committing a membership config log, which is almost the same as committing any log.

AddLearnerResponse contains a field matched to indicate the replication state to the learner, which is not included in ClientWriteResponse. This information can be retrieved via Raft::metrics().

Therefore to keep the API simple, replace AddLearnerResponse with ClientWriteResponse.

AddLearnerResponse:

pub struct AddLearnerResponse<NID: NodeId> {
/// The log id of the membership that contains the added learner.
pub membership_log_id: Option<LogId<NID>>,
/// The last log id that matches leader log.
pub matched: Option<LogId<NID>>,
}

ClientWriteResponse:

openraft/openraft/src/raft.rs

Lines 1266 to 1275 in 4a85ee9

pub struct ClientWriteResponse<C: RaftTypeConfig> {
/// The id of the log that is applied.
pub log_id: LogId<C::NodeId>,
/// Application specific response data.
pub data: C::R,
/// If the log entry is a change-membership entry.
pub membership: Option<Membership<C::NodeId, C::Node>>,
}

@github-actions
Copy link

👋 Thanks for opening this issue!

Get help or engage by:

  • /help : to print help messages.
  • /assignme : to assign this issue to you.

drmingdrmer added a commit to drmingdrmer/openraft that referenced this issue Feb 17, 2023
In openraft adds a learner is done by committing a membership config
log, which is almost the same as committing any log.

`AddLearnerResponse` contains a field `matched` to indicate the
replication state to the learner, which is not included in
`ClientWriteResponse`. This information can be retrieved via
`Raft::metrics()`.

Therefore to keep the API simple, replace `AddLearnerResponse` with
`ClientWriteResponse`.

Behavior change: adding a learner always commit a new membership config
log, no matter if it already exists in membership.
To avoid duplicated add, an application should check existence first by
examining `Raft::metrics()`

- Fix: datafuselabs#679

Upgrade tips:

- Replace AddLearnerResponse with ClientWriteResponse
- Replace AddLearnerError with ClientWriteError

Passes the application compilation.

See the changes in examples/.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant