-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: report daily participants to public stats #133
Conversation
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
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.
..otherwise LGTM!
lib/public-stats.js
Outdated
*/ | ||
const updateDailyParticipants = async (pgClient, participants) => { | ||
debug('Updating daily participants (%s seen)', participants.length) | ||
for (const participantAddress of participants) { |
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.
Performance wise, are we ok with running 2 SQL queries for every participant, in series? And if it fails midway, will we have inconsistent data? Playing devil's advocate here
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.
Great questions! 💯
I'll think about this tomorrow.
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.
Yeah, I was focused so much on storage efficiency and the querying side, that I completely neglected the writing side.
With ~2k participants per round, my current implementation would run 4k SQL queries. That would take forever to complete.
Thanks for flagging this early! 😍
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.
Cool, I found a neat trick: We can use SELECT UNNEST($1::TEXT[])
to build a query that accepts a JavaScript array parameter and converts the items into result rows.
Then I can change INSERT...VALUES
to INSERT...SELECT
to leverage this mechanism.
The changes were a bit more involved (see 7a2d14d), but I expect the performance to be excellent.
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
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.
Great work 😍
Create a new
spark_stats
table to keep track of daily participants:This table allows us to correctly calculate monthly participants too:
TODO:
Links: