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

Refactor:Refactor curp fast read implementation #270

Closed
Phoenix500526 opened this issue May 12, 2023 · 2 comments · Fixed by #297
Closed

Refactor:Refactor curp fast read implementation #270

Phoenix500526 opened this issue May 12, 2023 · 2 comments · Fixed by #297
Assignees
Labels
discussion Discussion is needed enhancement New feature or request
Milestone

Comments

@Phoenix500526
Copy link
Collaborator

Currently, on performance grounds, xline provides a fast-read mechanism(ReadState) to a client, which can prevent a leader node from becoming a bottleneck in an xline cluster. Thanks to the implementation of the read-state part, a command has to go through three stages from prepare to execute and then to after_sync, just for some notification reasons, even though it fails in its earlier stage. This doesn't make sense. We should refactor the implementation of the ReadState part.

@themanforfree
Copy link
Collaborator

Passing LogIndex to the prepare and execute stage of a command can trigger barriers in advance when failures occur, and it is no longer necessary to call other stages for failed commands.

impl<S> CurpCommandExecutor<Command> for CommandExecutor<S>
where
    S: StorageApi,
{
    fn prepare(
        &self,
        cmd: &Command,
        index: LogIndex,
    ) -> Result<<Command as CurpCommand>::PR, Self::Error> {
        let wrapper = cmd.request();
        if let Err(e) = self.auth_storage.check_permission(wrapper) {
            self.id_barrier.trigger(cmd.id());
            self.index_barrier.trigger(index);
            return Err(e);
        }
        let revision = match wrapper.request.backend() {
            RequestBackend::Auth => {
                if wrapper.request.skip_auth_revision() {
                    -1
                } else {
                    self.auth_rev.next()
                }
            }
            RequestBackend::Kv | RequestBackend::Lease => {
                if wrapper.request.skip_general_revision() {
                    -1
                } else {
                    self.general_rev.next()
                }
            }
        };
        Ok(revision)
    }

    async fn execute(
        &self,
        cmd: &Command,
        index: LogIndex,
    ) -> Result<<Command as CurpCommand>::ER, Self::Error> {
        let wrapper = cmd.request();
        let res = match wrapper.request.backend() {
            RequestBackend::Kv => self.kv_storage.execute(wrapper),
            RequestBackend::Auth => self.auth_storage.execute(wrapper),
            RequestBackend::Lease => self.lease_storage.execute(wrapper),
        };
        match res {
            Ok(res) => Ok(res),
            Err(e) => {
                self.id_barrier.trigger(cmd.id());
                self.index_barrier.trigger(index);
                Err(e)
            }
        }
    }

    ...
}

@themanforfree
Copy link
Collaborator

state transition diagram
#254 (comment)

@mergify mergify bot closed this as completed in #297 Jun 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Discussion is needed enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants