Skip to content

Make ClientWriteResponse.data optional to reflect Raft semantics #1462

@drmingdrmer

Description

@drmingdrmer

Summary

Make ClientWriteResponse.data field optional (Option<C::R>) to better reflect Raft semantics where only normal log entries have application response data.

Current Behavior

The ClientWriteResponse structure currently requires a data field for all responses:

pub struct ClientWriteResponse<C: RaftTypeConfig> {
    pub log_id: LogIdOf<C>,
    pub data: C::R,              // always required
    pub membership: Option<Membership<C>>,
}

This forces state machine implementations to return dummy/default values for entries that have no meaningful response data:

async fn apply<I>(&mut self, entries: I) -> Result<(), StorageError<C>> {
    for (entry, responder) in entries {
        let response = match entry.payload {
            EntryPayload::Blank => {
                // Must create dummy response even though blank entries have no data
                ClientResponse(None)
            }
            EntryPayload::Normal(ref data) => {
                // Only normal entries have actual response data
                let result = self.process(data);
                ClientResponse(Some(result))
            }
            EntryPayload::Membership(ref mem) => {
                // Must create dummy response even though membership changes have no data
                ClientResponse(None)
            }
        };
        responder.send(response);
    }
    Ok(())
}

Proposed Change

Change the data field to be optional:

pub struct ClientWriteResponse<C: RaftTypeConfig> {
    pub log_id: LogIdOf<C>,
    pub data: Option<C::R>,     // optional
    pub membership: Option<Membership<C>>,
}

Updated state machine implementation:

async fn apply<I>(&mut self, entries: I) -> Result<(), StorageError<C>> {
    for (entry, responder) in entries {
        let response = match entry.payload {
            EntryPayload::Blank => {
                // No dummy value needed
                None
            }
            EntryPayload::Normal(ref data) => {
                // Only create response for normal entries
                let result = self.process(data);
                Some(result)
            }
            EntryPayload::Membership(ref mem) => {
                // No dummy value needed
                None
            }
        };
        responder.send(response);
    }
    Ok(())
}

Updated client code:

let result = raft.client_write(request).await?;

// Clients can distinguish "no data" from "data is None"
match result.data {
    Some(data) => {
        // Normal entry with application response
        process_response(data);
    }
    None => {
        // Blank or membership entry - check membership field
        if let Some(membership) = result.membership {
            handle_membership_change(membership);
        }
    }
}

Benefits

  1. Semantic clarity: The API explicitly represents that response data is optional, matching Raft protocol semantics
  2. Cleaner state machine code: Eliminates the need to construct dummy/default response values for blank and membership entries
  3. Better client experience: Clients can clearly distinguish between "no response data" and "response data is present but may be None/empty"
  4. More idiomatic Rust: Using Option<T> to represent "may not have a value" is the idiomatic way

Implementation

Required changes:

  1. Core API (openraft/src/raft/message/client_write.rs):

    • Change pub data: C::R to pub data: Option<C::R>
    • Update response() method to return Option<&C::R>
  2. Internal implementation (openraft/src/storage/v2/apply_responder_inner.rs):

    • Change send() method signature from send(self, response: C::R) to send(self, response: Option<C::R>)
    • Update response construction to wrap response in data field
  3. All state machine implementations:

    • Update to return Option<C::R> instead of C::R to responder.send()
    • Remove dummy value construction for blank/membership entries
  4. Client code:

    • Update to handle Option<C::R> when accessing response.data

Migration Guide

For state machine implementations:

// Before:
responder.send(ClientResponse(None));  // dummy for blank/membership

// After:
responder.send(None);  // no dummy needed
// Before:
responder.send(response);  // response: C::R

// After:
responder.send(Some(response));  // response: C::R

For client code:

// Before:
let data = result.data;  // data: C::R

// After:
let data = result.data?;  // data: C::R, or handle None
// or
if let Some(data) = result.data { ... }

Version

This is a breaking change suitable for 0.10.0 release.

Metadata

Metadata

Assignees

No one assigned

    Labels

    C-featureCategory: feature request or enhancement

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions