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

fix: get strategies data from DB directly #718

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/graphql/operations/strategy.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { strategiesObj } from '../../helpers/strategies';
import { strategies } from '../../helpers/strategies';

export default async function (parent, { id }) {
return strategiesObj[id];
export default function (parent, { id }) {
return strategies.find(strategy => strategy.id === id);
}
34 changes: 20 additions & 14 deletions src/helpers/strategies.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import snapshot from '@snapshot-labs/snapshot.js';
import { spaces } from './spaces';
import log from './log';
import { capture } from '@snapshot-labs/snapshot-sentry';
import { URL } from 'url';
import db from '../helpers/mysql';
import log from './log';

export let strategies: any[] = [];
export let strategiesObj: any = {};

const scoreApiURL: URL = new URL(process.env.SCORE_API_URL ?? 'https://score.snapshot.org');
scoreApiURL.pathname = '/api/strategies';
Expand All @@ -21,24 +20,31 @@ async function loadStrategies() {
return true;
}

Object.values(spaces).forEach((space: any) => {
const ids = new Set<string>(space.strategies.map(strategy => strategy.name));
ids.forEach(id => {
if (res[id]) {
res[id].spacesCount = (res[id].spacesCount || 0) + 1;
}
});
});
const results = new Map(
(
await db.queryAsync(`
SELECT
Copy link
Member

Choose a reason for hiding this comment

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

This can be an heavy request isn't? How long does it take to resolve? How often is it called?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

~40ms to resolve, on my dataset of 13k spaces.

It's still not 100% live query, as it's only run along the fetch strategies query from score url, so once every 1min

Copy link
Member

Choose a reason for hiding this comment

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

Ok

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you want more refactoring, and use 100% live counters (and not cached for 1min) ?

This will bring more consistency, as no other counters are cached and lagging behind.

By enforcing only known strategies when submitting via sequencer, we could also get rid of the condition, checking if the strategy is a valid one from score-api, and have true live data from the strategies operation. (can also get rid of the /helpers/startegies.ts file)

Copy link
Member

@bonustrack bonustrack Oct 18, 2023

Choose a reason for hiding this comment

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

I prefer not using such query as live query, also not sure how you would get ride of strategies helpers, this is loading strategies metadata that are not available from the hub

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true, forgot about the other strategies data. We get less than 1 request/min on the hub for the strategies object, should have very minimal impact on performance

Copy link
Member

Choose a reason for hiding this comment

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

Takes around 2.8s on prod, will increase when we get more space 🤔

s.name as id,
COUNT(s.name) as count
FROM
spaces,
JSON_TABLE(
spaces.settings,
'$.strategies[*]' COLUMNS (name VARCHAR(40) PATH '$.name')
) s
GROUP BY s.name
ORDER BY count DESC;
`)
).map(r => [r.id, r.count])
);

strategies = Object.values(res)
.map((strategy: any) => {
strategy.id = strategy.key;
strategy.spacesCount = strategy.spacesCount || 0;
strategy.spacesCount = results.get(strategy.id) || 0;
return strategy;
})
.sort((a, b): any => b.spacesCount - a.spacesCount);

strategiesObj = Object.fromEntries(strategies.map(strategy => [strategy.id, strategy]));
}

async function run() {
Expand Down