Closed satishrudderstack closed 2 months ago
[!IMPORTANT]
Review skipped
Auto reviews are disabled on this repository.
Please check the settings in the CodeRabbit UI or the
.coderabbit.yaml
file in this repository. To trigger a single review, invoke the@coderabbitai review
command.You can disable this status message by setting the
reviews.review_status
tofalse
in the CodeRabbit configuration file.
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
Attention: Patch coverage is 69.84127%
with 114 lines
in your changes missing coverage. Please review.
Project coverage is 74.35%. Comparing base (
d263cc5
) to head (e3425f7
).
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
How do we ensure atomicity of steps 3 & 4 in the above diagram? viz. send aggregated reports to reporting service
and delete sent reports
?
As per my understanding, we used to ensure idempotency using timestamp along with grouping labels as report id. Just wondering how it's affected with the introduction of Flush/Aggregation window
.
How do we ensure atomicity of steps 3 & 4 in the above diagram? viz.
send aggregated reports to reporting service
anddelete sent reports
?As per my understanding, we used to ensure idempotency using timestamp along with grouping labels as report id. Just wondering how it's affected with the introduction of
Flush/Aggregation window
.
On Reporting Service, we are planning to have hash id generated from the payload for each report and have it as a id
column on Timescale table. If we receive the same report on Reporting Service, we ignore the report and respond with 200. We have similar logic for metrics
table
From Rudder Server side, we are calculating the flush window first and we are deleting all the reports in the flush window if all the aggregated reports were all sent successfully. In the aggregated report, we have min(reported_at)
for the group as timestamp which will be sent to reporting.
Now, for the case where some reports were sent and there is an error for some reports while sending, we error out and don't delete the reports. We will retry the entire window later and reporting will ignore the reports which it consumed already.
FYI, we don't consider most recent data where inserts are happening. We have config recentExclusionWindowInSeconds
Nice point @snarkychef . I have added some code so that our flush window doesn't change on retries
Description
Motivation for common flusher
Flusher
Flush steps
start
,end
) to flush.start
will be from db.end
will be min(now() - 30s, end-of-current-hour). We don't want to flush reports where inserts are happening. We always try to flush full window durations which is configured viaflushWindow
unless window is crossing multiple hour boundariesFeatures
Linear Ticket
https://linear.app/rudderstack/issue/OBS-504/reporting-client-for-tracked-users completes OBS-504
Security