feat(storage): implement appendable upload transport#6002
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces the internal infrastructure for Appendable Object Write (Bidi Write), including a Connector for establishing connections, a background Worker to manage the gRPC stream, and an AppendableObjectWriterTransport adapter. Feedback highlights a critical issue in connector.rs where the preconditions if_metageneration_match and if_metageneration_not_match are dropped when transitioning from WriteObjectSpec to AppendObjectSpec after a successful initial response, which could cause race conditions or incorrect writes during reconnection attempts.
| AppendObjectSpecState::Append(AppendObjectSpec { | ||
| bucket: s | ||
| .resource | ||
| .as_ref() | ||
| .map(|r| r.bucket.clone()) | ||
| .unwrap_or_default(), | ||
| object: s | ||
| .resource | ||
| .as_ref() | ||
| .map(|r| r.name.clone()) | ||
| .unwrap_or_default(), | ||
| generation: new_generation.unwrap_or(0), | ||
| write_handle: m.write_handle.clone(), | ||
| ..Default::default() | ||
| }) |
There was a problem hiding this comment.
When transitioning from WriteObjectSpec to AppendObjectSpec upon a successful initial response, the preconditions if_metageneration_match and if_metageneration_not_match are currently dropped (set to None via ..Default::default()).
If a subsequent append fails and the connector attempts to reconnect/reopen, these preconditions will be missing from the retry request, which could lead to race conditions or incorrect writes if the object's metadata has changed.
We should preserve these preconditions from the original WriteObjectSpec.
AppendObjectSpecState::Append(AppendObjectSpec {
bucket: s
.resource
.as_ref()
.map(|r| r.bucket.clone())
.unwrap_or_default(),
object: s
.resource
.as_ref()
.map(|r| r.name.clone())
.unwrap_or_default(),
generation: new_generation.unwrap_or(0),
write_handle: m.write_handle.clone(),
if_metageneration_match: s.if_metageneration_match,
if_metageneration_not_match: s.if_metageneration_not_match,
..Default::default()
})
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #6002 +/- ##
==========================================
+ Coverage 97.73% 97.74% +0.01%
==========================================
Files 244 248 +4
Lines 61096 62135 +1039
==========================================
+ Hits 59710 60733 +1023
- Misses 1386 1402 +16 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This PR comes after #6001.