Copilot commented on code in PR #247:
URL: https://github.com/apache/fluss-rust/pull/247#discussion_r2769748473
##########
crates/fluss/src/client/table/upsert.rs:
##########
@@ -111,12 +112,12 @@ pub struct UpsertWriter {
table_path: Arc<TablePath>,
writer_client: Arc<WriterClient>,
partition_field_getter: Option<PartitionGetter>,
- primary_key_encoder: Box<dyn KeyEncoder>,
+ primary_key_encoder: Mutex<Box<dyn KeyEncoder>>,
target_columns: Option<Arc<Vec<usize>>>,
// Use primary key encoder as bucket key encoder when None
- bucket_key_encoder: Option<Box<dyn KeyEncoder>>,
+ bucket_key_encoder: Option<Mutex<Box<dyn KeyEncoder>>>,
write_format: WriteFormat,
- row_encoder: Box<dyn RowEncoder>,
+ row_encoder: Mutex<Box<dyn RowEncoder>>,
Review Comment:
Consider using `std::sync::Mutex` instead of `tokio::sync::Mutex` for the
encoders. The encoder locks are never held across await points in the current
implementation (lines 302, 304, and 311-317 all complete synchronously), so
`std::sync::Mutex` would be more efficient. `std::sync::Mutex` has lower
overhead since it doesn't need to be async-aware. However, if you anticipate
future changes where locks might be held across await points,
`tokio::sync::Mutex` is the safer choice.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
