Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Append-entries support partial success return value #822

Closed
drmingdrmer opened this issue May 2, 2023 · 7 comments · Fixed by #831
Closed

Append-entries support partial success return value #822

drmingdrmer opened this issue May 2, 2023 · 7 comments · Fixed by #831
Assignees
Labels
A-network Area: network

Comments

@drmingdrmer
Copy link
Member

drmingdrmer commented May 2, 2023

An application can use this feature to send part of the log entries and return to the caller only the ones that were successfully replicated to a remote node.

Why

If there are too many log entries and the RPCOption.ttl is not sufficient, an application can opt to only send a portion of the entries. Thus replication can still make progress.

Changes

For better compatibility, simply add a new variant to the return value type of the append-entries RPC. Applications that do not need partial success can continue to function as is. However, for those applications that do use this feature, they can return PartialSuccess instead of Success to provide Openraft with information on which log entries have been successfully replicated.

Here are the changes that are being suggested:

 pub enum AppendEntriesResponse<NID: NodeId> {
     Success,
+    PartialSuccess(Option<LogId<NID>>),
     Conflict,
     HigherVote(Vote<NID>),
 }

Requirement is discussed in:

@drmingdrmer drmingdrmer self-assigned this May 2, 2023
@github-actions
Copy link

github-actions bot commented May 2, 2023

👋 Thanks for opening this issue!

Get help or engage by:

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

@drmingdrmer drmingdrmer added the A-network Area: network label May 2, 2023
@drmingdrmer
Copy link
Member Author

@schreter

@schreter
Copy link
Collaborator

schreter commented May 2, 2023

Thanks. Following my comment in the timeout PR, the timeout already on the first entry would be then represented as PartialSuccess(None), right?

@drmingdrmer

This comment was marked as outdated.

@drmingdrmer
Copy link
Member Author

Thanks. Following my comment in the timeout PR, the timeout already on the first entry would be then represented as PartialSuccess(None), right?

@schreter
PartialSuccess(None) should not be returned, if no progress is made, it should return an RPCError.

drmingdrmer added a commit to drmingdrmer/openraft that referenced this issue May 8, 2023
If there are too many log entries and the `RPCOption.ttl` is not
sufficient, an application can opt to only send a portion of the
entries, with `AppendEntriesResponse::PartialSuccess(Option<LogId>)`, to
inform Openraft with the last replicated log id. Thus replication can
still make progress.

For example, it tries to send log entries `[1-2..3-10]`, the application
is allowed to send just `[1-2..1-3]` and return `PartialSuccess(1-3)`,

### Caution

The returned matching log id must be **greater than or equal to** the
first log id(`AppendEntriesRequest::prev_log_id`) of the entries to
send. If no RPC reply is received, `RaftNetwork::send_append_entries`
**must** return an `RPCError` to inform Openraft that the first log
id(`AppendEntriesRequest::prev_log_id`) may not match on the remote
target node.

- Fix: databendlabs#822
@schreter
Copy link
Collaborator

schreter commented May 8, 2023

PartialSuccess(None) should not be returned, if no progress is made, it should return an RPCError.

Ack, thanks.

1 similar comment
@schreter
Copy link
Collaborator

schreter commented May 8, 2023

PartialSuccess(None) should not be returned, if no progress is made, it should return an RPCError.

Ack, thanks.

drmingdrmer added a commit to drmingdrmer/openraft that referenced this issue May 10, 2023
If there are too many log entries and the `RPCOption.ttl` is not
sufficient, an application can opt to only send a portion of the
entries, with `AppendEntriesResponse::PartialSuccess(Option<LogId>)`, to
inform Openraft with the last replicated log id. Thus replication can
still make progress.

For example, it tries to send log entries `[1-2..3-10]`, the application
is allowed to send just `[1-2..1-3]` and return `PartialSuccess(1-3)`,

### Caution

The returned matching log id must be **greater than or equal to** the
first log id(`AppendEntriesRequest::prev_log_id`) of the entries to
send. If no RPC reply is received, `RaftNetwork::send_append_entries`
**must** return an `RPCError` to inform Openraft that the first log
id(`AppendEntriesRequest::prev_log_id`) may not match on the remote
target node.

- Fix: databendlabs#822
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-network Area: network
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants