feat: add hand-written Prometheus remote write request parser #1628

BaldDemian · 2025-08-22T08:15:10Z

Rationale

A hand-written remote write request parser to replace prost, with the primary goal of achieving zero-allocation parsing.

Detailed Changes

The new remote_write directory contains the core implementation of the hand-written parser.
Add benchmarks.
Add a .proto file in pb_types.
Fix CI errors.

Test Plan

Unit tests can be found at the end of the pb_reader.rs.
A comprehensive equivalence test is available in equivalence_test.rs, which validates the correctness of the hand-written parser by comparing its output with that of the prost auto-generated parser.

fix ci fix ci fix ci fix ci

Copilot

Pull Request Overview

This PR adds a hand-written Prometheus remote write request parser optimized for zero-allocation parsing to replace prost. The implementation leverages object pooling, custom data structures, and optional unsafe optimizations to achieve better performance while maintaining compatibility with the existing protobuf format.

Key changes:

Hand-written protobuf parser with zero-allocation parsing capabilities
Object pooling mechanism using deadpool for memory reuse
Optional unsafe optimization feature for zero-copy bytes splitting

Reviewed Changes

Copilot reviewed 24 out of 36 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/remote_write/src/pooled_parser.rs	Main parser interface providing sync/async decode methods
src/remote_write/src/pb_reader.rs	Core protobuf reading implementation with manual parsing logic
src/remote_write/src/pooled_types.rs	Pooled data structures and object pool management
src/remote_write/src/repeated_field.rs	Custom RepeatedField implementation for memory optimization
src/remote_write/tests/equivalence_test.rs	Comprehensive test comparing hand-written parser with prost
src/pb_types/protos/remote_write.proto	Protobuf schema definition for remote write requests
src/benchmarks/	Benchmark infrastructure and memory analysis tools

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-08-24T02:47:39Z

src/remote_write/src/pooled_types.rs

+impl PooledLabel {
+    pub fn clear(&mut self) {
+        self.name.clear();
+        self.value.clear();
+    }
+}


The PooledLabel struct has both a Clear trait implementation (line 40-45) and a duplicate clear method (line 48-52). This duplication is unnecessary since the Clear trait already provides the same functionality.

Suggested change

impl PooledLabel {

pub fn clear(&mut self) {

self.name.clear();

self.value.clear();

}

}

// impl PooledLabel {

// pub fn clear(&mut self) {

// self.name.clear();

// self.value.clear();

// }

// }

Copilot · 2025-08-24T02:47:39Z

src/remote_write/src/pooled_types.rs

+impl PooledTimeSeries {
+    pub fn clear(&mut self) {
+        self.labels.clear();
+        self.samples.clear();
+        self.exemplars.clear();
+    }
+}


The PooledTimeSeries struct has both a Clear trait implementation (line 149-155) and a duplicate clear method (line 158-163). This duplication is unnecessary since the Clear trait already provides the same functionality.

Suggested change

impl PooledTimeSeries {

pub fn clear(&mut self) {

self.labels.clear();

self.samples.clear();

self.exemplars.clear();

}

}

// impl PooledTimeSeries {

// pub fn clear(&mut self) {

// self.labels.clear();

// self.samples.clear();

// self.exemplars.clear();

// }

// }

Copilot · 2025-08-24T02:47:40Z

src/remote_write/src/pooled_types.rs

+impl PooledWriteRequest {
+    pub fn clear(&mut self) {
+        self.timeseries.clear();
+        self.metadata.clear();
+    }
+}


The PooledWriteRequest struct has both a Clear trait implementation (line 171-176) and a duplicate clear method (line 179-183). This duplication is unnecessary since the Clear trait already provides the same functionality.

Suggested change

impl PooledWriteRequest {

pub fn clear(&mut self) {

self.timeseries.clear();

self.metadata.clear();

}

}

// Removed redundant inherent clear method; Clear trait implementation is sufficient.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull Request Overview

Copilot reviewed 24 out of 36 changed files in this pull request and generated 4 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-24T08:02:13Z

src/remote_write/src/repeated_field.rs

+}
+
+impl<T> Clear for Option<T> {
+    fn clear(&mut self) {


The Clear trait documentation mentions 'make it equivalent to newly created object' but the implementation for RepeatedField at line 82 only sets len to 0 without clearing the actual data in the backing Vec. While this is correct for the pooling use case, the trait documentation should clarify that 'clear' means resetting to a logically empty state, not necessarily deallocating or zeroing memory.

Please update the comments as suggested by copilot.

Copilot · 2025-10-24T08:02:13Z

src/remote_write/src/pb_reader.rs

+    pub fn skip_field(&mut self, wire_type: u8) -> Result<()> {
+        match wire_type {
+            WIRE_TYPE_VARINT => {
+                // For varint, read and discard the value..


Corrected double period '..' to single period '.'.

Suggested change

// For varint, read and discard the value..

// For varint, read and discard the value.

Copilot · 2025-10-24T08:02:14Z

src/benchmarks/build.rs

+    }
+
+    // Fix package namespace conflicts and inner attributes using sed.
+    let _ = Command::new("sed")


The build script silently ignores errors from sed commands by using let _. If sed fails, the generated code may be incorrect but the build continues. Consider propagating these errors or at least logging warnings when sed commands fail.

Copilot · 2025-10-24T08:02:14Z

src/benchmarks/remote_write_memory_bench.py

+# specific language governing permissions and limitations
+# under the License.
+
+#!/usr/bin/env python3


The shebang line appears after the license header rather than as the first line of the file. For proper script execution, the shebang should be the very first line. Move line 18 to line 1.

jiacai2050 · 2025-10-24T08:22:11Z

src/benchmarks/src/bin/pooled_parser_mem.rs

+static ALLOC: Jemalloc = Jemalloc;
+
+#[tokio::main(flavor = "current_thread")]
+async fn main() -> Result<(), Box<dyn std::error::Error>> {


All those files under bin have similar structure, could you refactor then into one file and use params to choose which method to decode the data?

jiacai2050 · 2025-10-24T08:37:29Z

src/remote_write/src/pooled_parser.rs

+//! decide how to make use of the parsed `Bytes` and whether to apply UTF-8
+//! validation.
+//!
+//! ## Basic Usage


I prefer we remove all comments below L26 , the API is not stable now, and very likely the comments become out of date when we refactor the API.

jiacai2050 · 2025-10-24T08:38:14Z

src/remote_write/src/pooled_parser.rs

+    /// This method will allocate a new [`PooledWriteRequest`] instance since it
+    /// is unable to use the object pool in sync functions.
+    ///
+    /// # Example


Delete comment below, the reason is the same as before.

jiacai2050 · 2025-10-24T08:39:37Z

src/remote_write/src/pooled_parser.rs

+    /// # Ok(())
+    /// # }
+    /// ```
+    pub fn decode(&self, buf: Bytes) -> Result<PooledWriteRequest, String> {


I prefer we use anyhow as the Error type for this crate, it has more features like backtrace.

jiacai2050 · 2025-10-24T08:41:00Z

src/remote_write/src/pooled_parser.rs

+    /// # Ok(())
+    /// # }
+    /// ```
+    pub async fn decode_async(


We don't need this async functions, the encode/decode is a CPU-bound work, use async has little benefits.

jiacai2050 · 2025-10-24T08:44:23Z

src/remote_write/src/pooled_types.rs

+use crate::repeated_field::{Clear, RepeatedField};
+
+#[derive(Debug, Clone)]
+pub struct PooledLabel {


Name this Label, same to other structures in this file.

We shouldn't add the Pooled prefix just because it's used in Pool, it could be used in a non-pool case.

jiacai2050 · 2025-10-24T08:45:45Z

src/remote_write/src/repeated_field.rs

+}
+
+impl<T> Clear for Option<T> {
+    fn clear(&mut self) {


Please update the comments as suggested by copilot.

jiacai2050 · 2025-10-24T08:46:48Z

src/remote_write/tests/equivalence_test.rs

+        metadata: Vec::new(),
+    };
+
+    for pooled_ts in pooled_request.timeseries.iter() {


Suggested change

for pooled_ts in pooled_request.timeseries.iter() {

for pooled_ts in &pooled_request.timeseries {

The same changes apply to for loops below also.

jiacai2050 · 2025-10-24T08:47:48Z

src/remote_write/README.md

+
+This crate parses the protobuf `string` type as Rust `Bytes` instances instead of `String` instances to avoid allocation, and it **does not** perform UTF-8 validation when parsing. Also, it **does not** check the semantics of the parsed data, such as [Labels](https://prometheus.io/docs/specs/prw/remote_write_spec/#labels). Therefore, it is up to the caller to decide how to make use of the parsed `Bytes` and whether to apply extra validation or error handling.
+
+### Basic Usage


Delete this section

jiacai2050 · 2025-10-24T08:54:27Z

src/remote_write/src/pb_reader.rs

+    PooledTimeSeries, PooledWriteRequest,
+};
+
+const WIRE_TYPE_VARINT: u8 = 0;


I prefer define this wire type as enum like this

#[repr(u8)] enum WriteType { ... }

jiacai2050 · 2025-10-24T08:55:41Z

src/remote_write/src/pb_reader.rs

+const FIELD_NUM_METADATA_UNIT: u32 = 5;
+
+// Taken from https://github.com/v0y4g3r/prom-write-request-bench/blob/step6/optimize-slice/src/bytes.rs under Apache License 2.0.
+#[allow(dead_code)]


Delete those dead code functions.

jiacai2050 · 2025-10-24T08:58:40Z

src/remote_write/src/pb_reader.rs

+    /// for better performance.
+    #[inline(always)]
+    pub fn read_varint(&mut self) -> Result<u64> {
+        if !self.data.has_remaining() {


After you define Error as anyhow, you can simplify it like

ensure!(data.has_remaining(), "no enough byte for varbint")

BaldDemian added 3 commits August 20, 2025 20:34

feat: add hand-written Prometheus remote write request parser

5341e05

fix ci

110ee26

fix ci fix ci fix ci fix ci

docs: update remote write experiment results

7e20fbb

BaldDemian changed the title ~~Add hand-written remote write request parser as a new crate~~ feat: add hand-written Prometheus remote write request parser Aug 22, 2025

github-actions bot added the feature New feature or request label Aug 22, 2025

fix(remote write): skip unknown fields instead of throwing errors

243936d

jiacai2050 requested review from Copilot and jiacai2050 August 24, 2025 02:46

Copilot AI reviewed Aug 24, 2025

View reviewed changes

chore(remote write): remove legacy Clear methods in pooled_types

f3b8167

jiacai2050 requested a review from Copilot October 24, 2025 08:01

Copilot AI reviewed Oct 24, 2025

View reviewed changes

jiacai2050 requested changes Oct 24, 2025

View reviewed changes

jiacai2050 reviewed Oct 24, 2025

View reviewed changes

BaldDemian added 2 commits October 28, 2025 18:28

chore(remote write): small updates

33a2ce7

fix ci

53c74f5

jiacai2050 approved these changes Oct 28, 2025

View reviewed changes

BaldDemian added 2 commits October 28, 2025 21:37

Merge branch 'main' into main

82234a7

fix ci

ee003ef

jiacai2050 merged commit 9cec563 into apache:main Oct 28, 2025
6 checks passed

	// For varint, read and discard the value..
	// For varint, read and discard the value.

	for pooled_ts in pooled_request.timeseries.iter() {
	for pooled_ts in &pooled_request.timeseries {


		This crate parses the protobuf `string` type as Rust `Bytes` instances instead of `String` instances to avoid allocation, and it does not perform UTF-8 validation when parsing. Also, it does not check the semantics of the parsed data, such as [Labels](https://prometheus.io/docs/specs/prw/remote_write_spec/#labels). Therefore, it is up to the caller to decide how to make use of the parsed `Bytes` and whether to apply extra validation or error handling.

		### Basic Usage

feat: add hand-written Prometheus remote write request parser #1628

feat: add hand-written Prometheus remote write request parser #1628

Uh oh!

Conversation

BaldDemian commented Aug 22, 2025

Rationale

Detailed Changes

Test Plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jiacai2050 Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jiacai2050 Oct 24, 2025 •

edited

Loading