-
Notifications
You must be signed in to change notification settings - Fork 69
Allow to run relayer without Redis #2176
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
base: master
Are you sure you want to change the base?
Conversation
Running redis should be optional
@@ -144,7 +144,9 @@ PRIORITY_FEE_SCALER_10=0.1 | |||
# https://redis.io/docs/getting-started/ | |||
# Under the hood, the relayer will cache JSON-rpc request data from requests | |||
# like `eth_getBlock` in the Redis DB. | |||
REDIS_URL="redis://127.0.0.1:6379" | |||
# Redis connection URL. Set to "" to disable Redis. | |||
# If you need Redis functionality, set to redis default "redis://127.0.0.1:6379" |
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'm OK with being able to disable redis by overriding this to "", but I don't think it should be the default. Performance without redis will suffer a lot, and redis is very easy to host. I'm more in favour of changing the docs than the defaults.
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 can do that. i think what many want before doing anything is testing a simple run on sepolia of the relayer. for that ideally you don't want to think about redis. ideally you add your rpc providers, do a yarn build and run it. let me know what you think, this is just my outside perspective..
also noticed you have to overwrite the HUB_CHAIN_ID=11155111, right now its not in env example, should we add it?
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.
One thing to be aware of is that both the HubPoolClient and SpokePoolClient instances have some subset of logs queries where they must search all the way back to genesis of the relevant contract. This can make it really slow to start up without redis, since the relayer has to repeat those searches on each run. My concern is that this is more likely to be a putoff for users, especially relatively to the marginal time it takes to install redis and enable it as a service.
+1 on adding the HUB_CHAIN_ID description to the docs.
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.
ok i see! so just running sepolia two networks without redis not really feasible. shall i move the redis section in env example out of "advanced configs" than?
will add HUB_CHAIN_ID
btw im still trying to get it to work but the clients (even with one run) fails with rate limits.. do i have to be on a an alchemy paying plan to test this out?
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.
You can drop NODE_MAX_CONCURRENCY
to reduce the chance of rate-limiting, and you also increase NODE_RETRIES
and/or NODE_RETRY_DELAY
to make it more resilient to rate-limiting. You can append _<chainId>
to any of the above to make it chain-specific.
But in general, you will need a paid plan to run this bot. Redis is also a huge help - if you can succeed in an initial update and get the historical state stored in redis then that will give you a big leg up.
@@ -72,7 +72,8 @@ export const REDIS_URL = process.env.REDIS_URL || REDIS_URL_DEFAULT; | |||
// Make the redis client for a particular url essentially a singleton. | |||
const redisClients: { [url: string]: RedisClient } = {}; | |||
|
|||
export async function getRedis(logger?: winston.Logger, url = REDIS_URL): Promise<RedisClient | undefined> { | |||
export async function getRedis(logger?: winston.Logger, url?: string): Promise<RedisClient | undefined> { | |||
if (!url) return undefined; |
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.
This makes sense, but you will have a degraded experience without proper caching. As an alternative: maybe we can roll out a more robust docker-compose.yml
file to allow the user to spin up redis caching more conveniently.
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 mean not sure if needed redis is fine to be set up. i was just looking for a very easy set up to just give the relayer one successfull run within 5 minutes of cloning the repo
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.
Redis is mentioned in advanced configurations only. By default one should be able to run the relayer without.