feat(cogs): create inventory-tracker crate for storage oplog - #588
feat(cogs): create inventory-tracker crate for storage oplog#588matt-codecov wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #588 +/- ##
==========================================
+ Coverage 87.99% 88.13% +0.14%
==========================================
Files 96 100 +4
Lines 15955 16448 +493
==========================================
+ Hits 14040 14497 +457
- Misses 1915 1951 +36
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
jan-auer
left a comment
jan-auer
left a comment
There was a problem hiding this comment.
Initial review. Some of the below comments can be left for follow-up to get a first version in, as long as they don't change the overall concept and can be fixed in isolation. Particularly, shutdown is a larger topic.
| /// Enqueues one record. | ||
| /// | ||
| /// `key` controls which partition receives the message. | ||
| fn send(&self, key: &[u8], payload: Vec<u8>) -> Result<(), Self::Error>; |
There was a problem hiding this comment.
At least in arroyo / librdkafka, my understanding is that because of message batching sending a message can also fail after they were initially recorded. With the fire-and-forget style send API, these errors will not be returned anymore.
For us, this is likely fine - we want non-blocking best effort submission. Though we should pick one of the below options:
- Acknowledge this in the doc comment and call out that not all errors can be captured
- Make the signature infallible and move the responsibility for handling errors to the producer implementation.
There was a problem hiding this comment.
The on_delivery_failure argument to the Kafka producer should give callers a hook into failed messages. Failures here are when, like, the local queue is full or something.
01463b9 to
71877b3
Compare
jan-auer
left a comment
jan-auer
left a comment
There was a problem hiding this comment.
Looks good. See the open comments before merge, please.
Either in this PR or in a follow up, I'd suggest to add metrics, especially for errors.
71877b3 to
d180831
Compare
2 participants
Ref FS-210
Related to getsentry/sentry-kafka-schemas#497
Creates the
inventory-trackercrate in our project that we will use to emit Kafka messages for a change stream that we will turn into a COGS pipeline.This crate isn't Objectstore-specific at all. If/when another service wants to use it, we can move it to a separate repository and set up a release process for it. I just couldn't be bothered if nobody's asking for it.
PR(s) that use this new crate will be along soon.