-
Notifications
You must be signed in to change notification settings - Fork 89
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
Reduce queries from unfillable updater #3107
Reduce queries from unfillable updater #3107
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
apps/cowswap-frontend/src/legacy/state/orders/updaters/UnfillableOrdersUpdater.ts
Outdated
Show resolved
Hide resolved
apps/cowswap-frontend/src/legacy/state/orders/updaters/UnfillableOrdersUpdater.ts
Outdated
Show resolved
Hide resolved
@anxolin build still failing because of the new error type |
apps/cowswap-frontend/src/legacy/state/orders/updaters/UnfillableOrdersUpdater.ts
Outdated
Show resolved
Hide resolved
apps/cowswap-frontend/src/legacy/state/orders/updaters/UnfillableOrdersUpdater.ts
Outdated
Show resolved
Hide resolved
I still need to rebase the other branch |
1610fee
to
2aee35c
Compare
const enoughBalance = hasEnoughBalanceAndAllowance({ | ||
account, | ||
amount: currencyAmount, | ||
balances: balancesRef.current, |
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.
Now balances uses a ref to not re-render the function that often, but always use the latest version of the balances
@@ -11,7 +11,7 @@ export const ContractDeploymentBlocks: Partial<Record<ChainId, number>> = { | |||
export const MARKET_OPERATOR_API_POLL_INTERVAL = ms`2s` | |||
// We can have lots of limit orders and it creates a high load, so we poll them no so ofter as market orders | |||
export const LIMIT_OPERATOR_API_POLL_INTERVAL = ms`15s` | |||
export const PENDING_ORDERS_PRICE_CHECK_POLL_INTERVAL = ms`15s` | |||
export const PENDING_ORDERS_PRICE_CHECK_POLL_INTERVAL = ms`30s` |
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.
I don't think we need to poll this often
updatePending() | ||
const interval = setInterval(updatePending, PENDING_ORDERS_PRICE_CHECK_POLL_INTERVAL) | ||
updatePendingRef.current() | ||
const interval = setInterval(() => updatePendingRef.current(), PENDING_ORDERS_PRICE_CHECK_POLL_INTERVAL) |
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.
updatePending function is the culpit of the re-renders that cause too many queries
Co-authored-by: Leandro <[email protected]>
2aee35c
to
77d6972
Compare
Summary
This PR reduces drastically the number of quotes we do.
Why?
Because the Unfillable Orders Updater was being "re-renderer" often. The re-rendering was making us to re-start the polling count (reset the timer), but also we were eagerly querying the quote!!
Ok, but why so many re-renders
It turns out, the updater had a
useEffect
who create the polling logic. ThisuseEffect
depended on a function to do the actual update. This function changes often. However, even if this function changes, we don't want to re-fetch again.Soluton, proposed in this PR
Instead of depending on the function itself, we depend on a reference to it.
This way, we don't need to re-render the
useEffect
and the polling works as expected.Before it would send hundreds of requests, specially in case we start to get some 404
Now we do way less in this case: