Skip to content

Conversation

WenyXu
Copy link
Contributor

@WenyXu WenyXu commented Aug 12, 2025

Closes #275

This PR introduces a new ProduceResult struct that provides enhanced information about produce operations, including both the offsets of produced records and the encoded request size in bytes.

New Types

  • ProduceResult: A new struct containing:
    • offsets: Vec<i64>: The offsets of the produced records
    • encoded_request_size: usize: The size of the request encoded in bytes
  • ResponseBodyWithMetadata<R>: A generic wrapper for response bodies that includes metadata about the encoded request size

API Changes

  • Producer trait: Updated produce() method to return ProduceResult instead of just Vec
  • PartitionClient: Modified to return ProduceResult with request size information
  • Messenger: Added request_with_metadata() method that returns response with size metadata

  • I've read the contributing section of the project CONTRIBUTING.md.
  • Signed CLA (if not already signed).

@WenyXu WenyXu marked this pull request as draft August 12, 2025 17:21
@WenyXu WenyXu force-pushed the feat/produce_result branch from 39d93c0 to bc582e9 Compare August 12, 2025 17:24
@WenyXu WenyXu marked this pull request as ready for review August 12, 2025 17:27
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the end2end test should assert this new field as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in b1b5c99


/// The result of the produce call.
#[derive(Debug, Default)]
pub struct ProduceResult {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub struct ProduceResult {
#[non_exhaustive]
pub struct ProduceResult {

makes it easier to extend this later w/o introducing breaking changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in a12992f

src/messenger.rs Outdated
self.version_ranges = ranges;
}

pub async fn request<R>(&self, msg: R) -> Result<R::ResponseBody, RequestError>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this method is technically public, I doubt that anybody really uses it. So I think request should just return the metadata as well, in the same way that request_with_version_ranges does. This keeps the API kinda lean and avoids having all sorts of permutations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in a12992f

}
}

/// A response body with metadata about the request.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// A response body with metadata about the request.
/// A response body with metadata about the request & response.

I think we can make this a bit more generic for future additions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in a12992f

}
}

/// The result of the produce call.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// The result of the produce call.
/// The result of the [produce](ProducerClient::produce) call.

This makes it clearer which API you're referencing here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in a12992f

@WenyXu WenyXu force-pushed the feat/produce_result branch from b1b5c99 to a12992f Compare August 13, 2025 10:56
Copy link
Collaborator

@crepererum crepererum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you

@crepererum crepererum merged commit a62120b into influxdata:main Aug 13, 2025
11 checks passed
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.

Request to add encoded_size in produce method return value
2 participants