Skip to content

Commit 5c576bb

Browse files
jan-auerclaude
andcommitted
ref(service): Simplify Bigtable mutation construction
Push v2::Mutation wrapping into builders and a helper so call sites are clean. Builders now return fixed-size arrays; callers needing Vec use .into(). Co-Authored-By: Claude <noreply@anthropic.com>
1 parent b7ad0b9 commit 5c576bb

1 file changed

Lines changed: 48 additions & 72 deletions

File tree

objectstore-service/src/backend/bigtable.rs

Lines changed: 48 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -305,16 +305,29 @@ fn metadata_filter() -> v2::RowFilter {
305305
}
306306
}
307307

308+
fn mutation(mutation: mutation::Mutation) -> v2::Mutation {
309+
v2::Mutation {
310+
mutation: Some(mutation),
311+
}
312+
}
313+
314+
/// Creates a `DeleteFromRow` mutation wrapped in the outer [`v2::Mutation`] envelope.
315+
fn delete_row_mutation() -> v2::Mutation {
316+
mutation(mutation::Mutation::DeleteFromRow(
317+
mutation::DeleteFromRow {},
318+
))
319+
}
320+
308321
/// Builds the three mutations that write an object row: clear existing data,
309322
/// then set the payload and metadata cells.
310323
///
311324
/// Used by both [`BigTableBackend::put_row`] (unconditional write) and
312325
/// [`BigTableBackend::put_non_tombstone`] (conditional write).
313-
fn build_write_mutations(
326+
fn object_mutations(
314327
metadata: &Metadata,
315328
payload: Vec<u8>,
316329
now: SystemTime,
317-
) -> Result<[mutation::Mutation; 3]> {
330+
) -> Result<[v2::Mutation; 3]> {
318331
let (family, timestamp_micros) = match metadata.expiration_policy {
319332
ExpirationPolicy::Manual => (FAMILY_MANUAL, -1),
320333
ExpirationPolicy::TimeToLive(ttl) => (FAMILY_GC, ttl_to_micros(ttl, now)?),
@@ -326,19 +339,19 @@ fn build_write_mutations(
326339

327340
Ok([
328341
// NB: We explicitly delete the row to clear metadata on overwrite.
329-
mutation::Mutation::DeleteFromRow(mutation::DeleteFromRow {}),
330-
mutation::Mutation::SetCell(mutation::SetCell {
342+
delete_row_mutation(),
343+
mutation(mutation::Mutation::SetCell(mutation::SetCell {
331344
family_name: family.to_owned(),
332345
column_qualifier: COLUMN_PAYLOAD.to_owned(),
333346
timestamp_micros,
334347
value: payload,
335-
}),
336-
mutation::Mutation::SetCell(mutation::SetCell {
348+
})),
349+
mutation(mutation::Mutation::SetCell(mutation::SetCell {
337350
family_name: family.to_owned(),
338351
column_qualifier: COLUMN_METADATA.to_owned(),
339352
timestamp_micros,
340353
value: metadata_bytes,
341-
}),
354+
})),
342355
])
343356
}
344357

@@ -360,10 +373,7 @@ struct TombstoneMeta {
360373
///
361374
/// Used by both [`BigTableBackend::put_tombstone_row`] (unconditional write) and the
362375
/// TTI bump path in tiered reads.
363-
fn build_tombstone_mutations(
364-
tombstone: &Tombstone,
365-
now: SystemTime,
366-
) -> Result<[mutation::Mutation; 3]> {
376+
fn tombstone_mutations(tombstone: &Tombstone, now: SystemTime) -> Result<[v2::Mutation; 3]> {
367377
let (family, timestamp_micros) = match tombstone.expiration_policy {
368378
ExpirationPolicy::Manual => (FAMILY_MANUAL, -1),
369379
ExpirationPolicy::TimeToLive(ttl) => (FAMILY_GC, ttl_to_micros(ttl, now)?),
@@ -375,20 +385,20 @@ fn build_tombstone_mutations(
375385
};
376386

377387
Ok([
378-
mutation::Mutation::DeleteFromRow(mutation::DeleteFromRow {}),
379-
mutation::Mutation::SetCell(mutation::SetCell {
388+
delete_row_mutation(),
389+
mutation(mutation::Mutation::SetCell(mutation::SetCell {
380390
family_name: family.to_owned(),
381391
column_qualifier: COLUMN_REDIRECT.to_owned(),
382392
timestamp_micros,
383393
value: tombstone.target.as_storage_path().to_string().into_bytes(),
384-
}),
385-
mutation::Mutation::SetCell(mutation::SetCell {
394+
})),
395+
mutation(mutation::Mutation::SetCell(mutation::SetCell {
386396
family_name: family.to_owned(),
387397
column_qualifier: COLUMN_TOMBSTONE_META.to_owned(),
388398
timestamp_micros,
389399
value: serde_json::to_vec(&tombstone_meta)
390400
.map_err(|cause| Error::serde("failed to serialize tombstone", cause))?,
391-
}),
401+
})),
392402
])
393403
}
394404

@@ -630,23 +640,16 @@ impl BigTableBackend {
630640
})
631641
}
632642

633-
async fn mutate<I>(
643+
async fn mutate(
634644
&self,
635645
path: Vec<u8>,
636-
mutations: I,
646+
mutations: impl Into<Vec<v2::Mutation>>,
637647
action: &'static str,
638-
) -> Result<v2::MutateRowResponse>
639-
where
640-
I: IntoIterator<Item = mutation::Mutation>,
641-
{
642-
let mutations = mutations
643-
.into_iter()
644-
.map(|m| v2::Mutation { mutation: Some(m) })
645-
.collect();
648+
) -> Result<v2::MutateRowResponse> {
646649
let request = v2::MutateRowRequest {
647650
table_name: self.table_path.clone(),
648651
row_key: path,
649-
mutations,
652+
mutations: mutations.into(),
650653
..Default::default()
651654
};
652655

@@ -665,7 +668,7 @@ impl BigTableBackend {
665668
payload: Vec<u8>,
666669
action: &'static str,
667670
) -> Result<v2::MutateRowResponse> {
668-
let mutations = build_write_mutations(metadata, payload, SystemTime::now())?;
671+
let mutations = object_mutations(metadata, payload, SystemTime::now())?;
669672
self.mutate(path, mutations, action).await
670673
}
671674

@@ -675,7 +678,7 @@ impl BigTableBackend {
675678
tombstone: &Tombstone,
676679
action: &'static str,
677680
) -> Result<v2::MutateRowResponse> {
678-
let mutations = build_tombstone_mutations(tombstone, SystemTime::now())?;
681+
let mutations = tombstone_mutations(tombstone, SystemTime::now())?;
679682
self.mutate(path, mutations, action).await
680683
}
681684

@@ -771,10 +774,7 @@ impl Backend for BigTableBackend {
771774
tracing::debug!("Deleting from Bigtable backend");
772775

773776
let path = id.as_storage_path().to_string().into_bytes();
774-
let mutations = [mutation::Mutation::DeleteFromRow(
775-
mutation::DeleteFromRow {},
776-
)];
777-
self.mutate(path, mutations, "delete").await?;
777+
self.mutate(path, [delete_row_mutation()], "delete").await?;
778778

779779
Ok(())
780780
}
@@ -792,10 +792,8 @@ impl HighVolumeBackend for BigTableBackend {
792792
tracing::debug!("Conditional put to Bigtable backend");
793793

794794
let path = id.as_storage_path().to_string().into_bytes();
795-
let false_mutations = build_write_mutations(metadata, payload.to_vec(), SystemTime::now())?
796-
.into_iter()
797-
.map(|m| v2::Mutation { mutation: Some(m) })
798-
.collect();
795+
let false_mutations =
796+
object_mutations(metadata, payload.to_vec(), SystemTime::now())?.into();
799797

800798
let request = v2::CheckAndMutateRowRequest {
801799
table_name: self.table_path.clone(),
@@ -900,18 +898,13 @@ impl HighVolumeBackend for BigTableBackend {
900898
tracing::debug!("Conditional delete from Bigtable backend");
901899

902900
let path = id.as_storage_path().to_string().into_bytes();
903-
let delete_mutation = v2::Mutation {
904-
mutation: Some(mutation::Mutation::DeleteFromRow(
905-
mutation::DeleteFromRow {},
906-
)),
907-
};
908901

909902
let request = v2::CheckAndMutateRowRequest {
910903
table_name: self.table_path.clone(),
911904
row_key: path.clone(),
912905
predicate_filter: Some(tombstone_predicate()),
913906
true_mutations: vec![], // Tombstone matched → leave intact.
914-
false_mutations: vec![delete_mutation], // No tombstone → delete.
907+
false_mutations: vec![delete_row_mutation()], // No tombstone → delete.
915908
..Default::default()
916909
};
917910

@@ -965,27 +958,10 @@ impl HighVolumeBackend for BigTableBackend {
965958
let path = id.as_storage_path().to_string().into_bytes();
966959
let now = SystemTime::now();
967960

968-
let write_mutations: Vec<v2::Mutation> = match write {
969-
TieredWrite::Tombstone(tombstone) => {
970-
let mutations = build_tombstone_mutations(&tombstone, now)?;
971-
mutations
972-
.into_iter()
973-
.map(|m| v2::Mutation { mutation: Some(m) })
974-
.collect()
975-
}
976-
TieredWrite::Object(metadata, payload) => {
977-
let mutations = build_write_mutations(&metadata, payload.to_vec(), now)?;
978-
mutations
979-
.into_iter()
980-
.map(|m| v2::Mutation { mutation: Some(m) })
981-
.collect()
982-
}
983-
TieredWrite::Delete => {
984-
let delete = mutation::Mutation::DeleteFromRow(mutation::DeleteFromRow {});
985-
vec![v2::Mutation {
986-
mutation: Some(delete),
987-
}]
988-
}
961+
let write_mutations = match write {
962+
TieredWrite::Tombstone(tombstone) => tombstone_mutations(&tombstone, now)?.into(),
963+
TieredWrite::Object(m, p) => object_mutations(&m, p.to_vec(), now)?.into(),
964+
TieredWrite::Delete => vec![delete_row_mutation()],
989965
};
990966

991967
let (predicate_filter, true_mutations, false_mutations, success_on_match) = match current {
@@ -1159,7 +1135,7 @@ mod tests {
11591135
now: SystemTime,
11601136
) -> Result<()> {
11611137
let path = id.as_storage_path().to_string().into_bytes();
1162-
let mutations = build_write_mutations(metadata, payload.to_vec(), now)?;
1138+
let mutations = object_mutations(metadata, payload.to_vec(), now)?;
11631139
backend.mutate(path, mutations, "test-setup").await?;
11641140
Ok(())
11651141
}
@@ -1171,7 +1147,7 @@ mod tests {
11711147
now: SystemTime,
11721148
) -> Result<()> {
11731149
let path = id.as_storage_path().to_string().into_bytes();
1174-
let mutations = build_tombstone_mutations(tombstone, now)?;
1150+
let mutations = tombstone_mutations(tombstone, now)?;
11751151
backend.mutate(path, mutations, "test-setup").await?;
11761152
Ok(())
11771153
}
@@ -1203,12 +1179,12 @@ mod tests {
12031179
};
12041180

12051181
let path = id.as_storage_path().to_string().into_bytes();
1206-
let mutations = [mutation::Mutation::SetCell(mutation::SetCell {
1182+
let mutations = [mutation(mutation::Mutation::SetCell(mutation::SetCell {
12071183
family_name: family.to_owned(),
12081184
column_qualifier: COLUMN_METADATA.to_owned(),
12091185
timestamp_micros,
12101186
value: meta.into_bytes(),
1211-
})];
1187+
}))];
12121188

12131189
backend.mutate(path, mutations, "test-setup").await?;
12141190

@@ -1223,18 +1199,18 @@ mod tests {
12231199
) -> Result<()> {
12241200
let path = id.as_storage_path().to_string().into_bytes();
12251201
let mutations = [
1226-
mutation::Mutation::SetCell(mutation::SetCell {
1202+
mutation(mutation::Mutation::SetCell(mutation::SetCell {
12271203
family_name: FAMILY_MANUAL.to_owned(),
12281204
column_qualifier: COLUMN_REDIRECT.to_owned(),
12291205
timestamp_micros: -1,
12301206
value: b"".to_vec(), // empty — legacy format
1231-
}),
1232-
mutation::Mutation::SetCell(mutation::SetCell {
1207+
})),
1208+
mutation(mutation::Mutation::SetCell(mutation::SetCell {
12331209
family_name: FAMILY_MANUAL.to_owned(),
12341210
column_qualifier: COLUMN_TOMBSTONE_META.to_owned(),
12351211
timestamp_micros: -1,
12361212
value: b"{}".to_vec(),
1237-
}),
1213+
})),
12381214
];
12391215

12401216
backend.mutate(path, mutations, "test-setup").await?;

0 commit comments

Comments
 (0)