-
Notifications
You must be signed in to change notification settings - Fork 289
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Write the dag relationship in batch #4304
Changes from 8 commits
5bb965e
1b9985c
595c9a1
79a421a
de84d7b
262ba00
29e4454
e4f72af
a4b0e54
aa0d4e1
1c2cc70
baf1e9e
0d3077a
36ff46d
d6dcf30
39593df
61bd695
188fd4a
8f01900
81dbbf5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,7 @@ | |
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
use crate::{ | ||
batch::WriteBatch, | ||
batch::{WriteBatch, WriteBatchWithColumn}, | ||
errors::StorageInitError, | ||
metrics::{record_metrics, StorageMetrics}, | ||
storage::{ColumnFamilyName, InnerStore, KeyCodec, RawDBStorage, ValueCodec, WriteOp}, | ||
|
@@ -414,6 +414,53 @@ impl InnerStore for DBStorage { | |
}) | ||
} | ||
|
||
fn write_batch_with_column(&self, batch: WriteBatchWithColumn) -> Result<()> { | ||
let mut db_batch = DBWriteBatch::default(); | ||
batch.data.into_iter().for_each(|data| { | ||
let cf_handle = self.get_cf_handle(&data.column); | ||
for (key, write_op) in data.row_data.rows { | ||
match write_op { | ||
WriteOp::Value(value) => db_batch.put_cf(cf_handle, key, value), | ||
WriteOp::Deletion => db_batch.delete_cf(cf_handle, key), | ||
}; | ||
} | ||
}); | ||
record_metrics( | ||
"db", | ||
"write_batch_column", | ||
"write_batch", | ||
self.metrics.as_ref(), | ||
) | ||
.call(|| { | ||
self.db | ||
.write_opt(db_batch, &Self::default_write_options())?; | ||
Ok(()) | ||
}) | ||
} | ||
Comment on lines
+417
to
+439
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Reduce code duplication between batch write methods The implementations of Extract the common logic into a private helper method: + fn write_batch_with_column_internal(
+ &self,
+ batch: WriteBatchWithColumn,
+ write_opts: WriteOptions,
+ ) -> Result<()> {
+ let mut db_batch = DBWriteBatch::default();
+ batch.data.into_iter().for_each(|data| {
+ let cf_handle = self.get_cf_handle(&data.column);
+ for (key, write_op) in data.row_data.rows {
+ match write_op {
+ WriteOp::Value(value) => db_batch.put_cf(cf_handle, key, value),
+ WriteOp::Deletion => db_batch.delete_cf(cf_handle, key),
+ };
+ }
+ });
+ record_metrics(
+ "db",
+ "write_batch_column",
+ "write_batch",
+ self.metrics.as_ref(),
+ )
+ .call(|| {
+ self.db.write_opt(db_batch, &write_opts)?;
+ Ok(())
+ })
+ }
+
fn write_batch_with_column(&self, batch: WriteBatchWithColumn) -> Result<()> {
- let mut db_batch = DBWriteBatch::default();
- // ... existing implementation
+ self.write_batch_with_column_internal(batch, Self::default_write_options())
}
fn write_batch_with_column_sync(&self, batch: WriteBatchWithColumn) -> Result<()> {
- let mut db_batch = DBWriteBatch::default();
- // ... existing implementation
+ self.write_batch_with_column_internal(batch, Self::sync_write_options())
} Also applies to: 441-462 |
||
|
||
fn write_batch_with_column_sync(&self, batch: WriteBatchWithColumn) -> Result<()> { | ||
let mut db_batch = DBWriteBatch::default(); | ||
batch.data.into_iter().for_each(|data| { | ||
let cf_handle = self.get_cf_handle(&data.column); | ||
for (key, write_op) in data.row_data.rows { | ||
match write_op { | ||
WriteOp::Value(value) => db_batch.put_cf(cf_handle, key, value), | ||
WriteOp::Deletion => db_batch.delete_cf(cf_handle, key), | ||
}; | ||
} | ||
}); | ||
record_metrics( | ||
"db", | ||
"write_batch_column", | ||
"write_batch", | ||
self.metrics.as_ref(), | ||
) | ||
.call(|| { | ||
self.db.write_opt(db_batch, &Self::sync_write_options())?; | ||
Ok(()) | ||
}) | ||
} | ||
jackzhhuang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
fn get_len(&self) -> Result<u64> { | ||
unimplemented!() | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Optimize HashMap initialization and error handling
A few suggestions for improvement:
Consider this improved implementation:
📝 Committable suggestion