Skip to content
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

provide an example of buffered metrics #878

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

ssube
Copy link
Contributor

@ssube ssube commented Dec 4, 2023

When computing metrics, run queries against the read replica and buffer the results in the JS worker before writing them back.

This does not address recreateRankTable.

Copy link
Contributor

@lrojas94 lrojas94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm gonna assume this is really a proof of concept rather than a mergable PR. It's certainly a good idea to do this in all likelyhood, and I see how it will improve performance. The only nit here is that replicas can lag behind. Wouldn't that be an issue?

@@ -27,7 +27,7 @@ export function createMetricProcessor({
async update() {
if (!clickhouse) return;
const [lastUpdate, setLastUpdate] = await getJobDate(`metric:${name.toLowerCase()}`);
const ctx = { db: dbWrite, ch: clickhouse, lastUpdate };
const ctx = { db: dbRead, dbWrite, ch: clickhouse, lastUpdate };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By doing this you'll globally break all other metrics as they rely on db being dbWrite 😬 this can be done, but you must apply this fix to everything.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I was trying to avoid the larger/noisier refactor to dbRead, to keep this example minimal, but that will need to be updated for this to work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants