You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by "brent-statsig (via GitHub)" <gi...@apache.org> on 2025/10/28 22:23:11 UTC
[PR] RFC - Split serialization logic from writing logic to make concurrent writes faster [avro-rs]
brent-statsig opened a new pull request, #326:
URL: https://github.com/apache/avro-rs/pull/326
Working on adding better ability to concurrently process avro - this moves the heavy work of serializing + compressing to not require a `mut` anymore. This allows users to wrap the serialization logic in whatever async/processing runtime they want easily, without introducing any opinions or bloat.
- `serialize_ser` does the heavy lifting, returning a private struct
- `extend_avro_serialized_buffer` accepts this struct, writing it directly to the writer and performing all avro bookkeeping.
Let me know what you think :) its a lil messy factoring wise, but I think the abstraction is fairly reasonable, effective, and safe.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RFC - Split serialization logic from writing logic to make concurrent writes faster [avro-rs]
Posted by "Kriskras99 (via GitHub)" <gi...@apache.org>.
Kriskras99 commented on PR #326:
URL: https://github.com/apache/avro-rs/pull/326#issuecomment-3460458562
I think this can be useful to have. However I don't entirely agree with the design.
I think the `AvroSerializedBuffer` should have it's own append function. It's also relatively easily now to create a buffer with one schema and use it for another. It would also need functions for appending `Value`s
Maybe more something like this?:
```rust
pub struct AvroSerializedBuffer<'a> {
schema: &'a Schema,
// We don't want this to be a reference to the writer, as that would block any writing,
// But cloning this can be expensive (there's a Vec inside)
resolved_schema: ResolvedSchema<'a>,
codec: Codec
num_values: usize,
// We want to do compression in the buffer not in the writer, but want to do it as late as possible
// for maximum compression. So we need to track what we already have compressed.
compressed_up_to: usize,
}
```
Then we can compare the schema, resolved schema and codec when adding it back to the writer.
@martin-g what do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RFC - Split serialization logic from writing logic to make concurrent writes faster [avro-rs]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #326:
URL: https://github.com/apache/avro-rs/pull/326#discussion_r2471810427
##########
avro/src/writer.rs:
##########
@@ -63,6 +63,11 @@ pub struct Writer<'a, W: Write> {
user_metadata: HashMap<String, Value>,
}
+pub struct AvroSerializedBuffer {
+ buffer: Vec<u8>,
+ num_values: usize,
+}
Review Comment:
The public struct `AvroSerializedBuffer` lacks documentation. Add a doc comment explaining its purpose as a container for pre-serialized Avro data that can be passed to `extend_avro_serialized_buffer`. Also consider documenting why the fields are private (to enforce construction only through `serialize_ser`).
##########
avro/src/writer.rs:
##########
@@ -304,6 +309,69 @@ impl<'a, W: Write> Writer<'a, W> {
Ok(num_bytes)
}
+ /**
+ * Writes a previously serialized bundle of rows directly to the writer (see self::serialize_ser).
Review Comment:
The reference `self::serialize_ser` is incorrect. Use proper Rust doc link syntax like `[serialize_ser](Self::serialize_ser)` or just `serialize_ser` to properly reference the method.
```suggestion
* Writes a previously serialized bundle of rows directly to the writer (see [serialize_ser](Self::serialize_ser)).
```
##########
avro/src/writer.rs:
##########
@@ -304,6 +309,69 @@ impl<'a, W: Write> Writer<'a, W> {
Ok(num_bytes)
}
+ /**
+ * Writes a previously serialized bundle of rows directly to the writer (see self::serialize_ser).
+ * This will not flush any intermediate buffers - only write the provided buffer
+ * directly to the underlying writer.
+ */
Review Comment:
Use Rust-style doc comments (`///`) instead of C-style block comments (`/** */`) for consistency with the rest of the codebase. Rust doc comments are the idiomatic way to document public APIs.
```suggestion
/// Writes a previously serialized bundle of rows directly to the writer (see self::serialize_ser).
/// This will not flush any intermediate buffers - only write the provided buffer
/// directly to the underlying writer.
```
##########
avro/src/writer.rs:
##########
@@ -304,6 +309,69 @@ impl<'a, W: Write> Writer<'a, W> {
Ok(num_bytes)
}
+ /**
+ * Writes a previously serialized bundle of rows directly to the writer (see self::serialize_ser).
+ * This will not flush any intermediate buffers - only write the provided buffer
+ * directly to the underlying writer.
+ */
+ pub fn extend_avro_serialized_buffer(
+ &mut self,
+ avro_serialized_buffer: AvroSerializedBuffer,
+ ) -> AvroResult<usize> {
+ let mut num_bytes = self.maybe_write_header()?;
+ let buffer = avro_serialized_buffer.buffer;
+ let stream_len = buffer.len();
+ let num_values = avro_serialized_buffer.num_values;
+
+ num_bytes += self.append_raw(&num_values.into(), &Schema::Long)?
+ + self.append_raw(&stream_len.into(), &Schema::Long)?
+ + self
+ .writer
+ .write(buffer.as_ref())
+ .map_err(Details::WriteBytes)?
+ + self.append_marker()?;
+
+ self.writer.flush().map_err(Details::FlushWriter)?;
+
+ Ok(num_bytes)
+ }
+
+ /**
+ * Serialize an iterator of serde::Serialize objects into an AvroSerializedBuffer. This call
+ * does not need a `mut` self - so it is safe to call from multiple threads to prepare data
+ * for writing.
+ */
Review Comment:
Use Rust-style doc comments (`///`) instead of C-style block comments (`/** */`) for consistency with the rest of the codebase. Rust doc comments are the idiomatic way to document public APIs.
##########
avro/src/writer.rs:
##########
@@ -63,6 +63,11 @@ pub struct Writer<'a, W: Write> {
user_metadata: HashMap<String, Value>,
}
+pub struct AvroSerializedBuffer {
+ buffer: Vec<u8>,
+ num_values: usize,
+}
Review Comment:
The new public type `AvroSerializedBuffer` should be exported in `avro/src/lib.rs` in the `pub use writer::` block (around line 911) to make it accessible to library consumers. Currently users cannot import this type even though it's returned by the public `serialize_ser` method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org