-
Notifications
You must be signed in to change notification settings - Fork 25
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
Start tracking incoming message counts by flow #5679
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5679 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 569 570 +1
Lines 25870 25881 +11
=========================================
+ Hits 25870 25881 +11 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
8aaf518
to
a8c434f
Compare
a8c434f
to
dffc048
Compare
SELECT n.flow_id, format('msgsin:date:%s', n.created_on::date), count(*), FALSE FROM newtab n | ||
INNER JOIN oldtab o ON o.id = n.id | ||
WHERE n.direction = 'I' AND o.flow_id IS NULL AND n.flow_id IS NOT NULL | ||
GROUP BY 1, 2; |
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.
maybe this should be a single row trigger since we don't write incoming messages in bulk... then it could at least write the 3 flows_flowactivitycount rows in bulk...
FlowNodeCount.squash() | ||
FlowRunStatusCount.squash() | ||
FlowCategoryCount.squash() | ||
FlowStartCount.squash() |
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.
splitting up these tasks so we can see the performance of the new counts.. including FlowStartCount
with new counts because still not sure if it needs to be changed
SELECT n.flow_id, count(*) AS msgs FROM newtab n INNER JOIN oldtab o ON o.id = n.id | ||
WHERE n.direction = 'I' AND o.flow_id IS NULL AND n.flow_id IS NOT NULL | ||
GROUP BY 1 | ||
) s; |
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.
@ericnewcomer @norkans7 tweaked this so that the 3 count rows are added with a single insert.. only downside here is that NOW()
can't be mocked in tests.
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.
Also can't fake historical messages but we don't want to ever do that again anyway
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.
What index is used for WHERE n.direction = 'I' AND o.flow_id IS NULL AND n.flow_id IS NOT NULL
?
Do we need a new index for that?
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.
@norkans7 this is querying on tiny in memory tables created by the trigger - it's not hitting the real msg table
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.
Okay
No description provided.