-
Notifications
You must be signed in to change notification settings - Fork 5
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 connection timeout #456
base: main
Are you sure you want to change the base?
Conversation
e33059a
to
83e2b0b
Compare
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 am not sure how to test there changes as they should not have an impackt on any local runs. Approving to not block testing this using other infrastructure.
@@ -58,10 +70,14 @@ def get_solver_rewards( | |||
|
|||
# Here, we use the convention that we run the prod query for the first connection | |||
# and the barn query to all other connections | |||
log.info("Setting tcp_keepalives_idle to 900 for prod connection") | |||
self.connections[0].execute(text("SET tcp_keepalives_idle = 900;")) |
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 would probably introduce a constant for this value.
results.append( | ||
self.exec_query(query=batch_reward_query_prod, engine=self.connections[0]) | ||
) | ||
for engine in self.connections[1:]: | ||
log.info("Running barn query on other connections (in get_solver_rewards") |
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.
What is the long term plan here? It seems that not setting the time out for barn is fine. But it also makes it a bit more difficult to understand what is happening here.
Currently the solver-rewards dry-run script returns fails in some environments due to the connection timing out. This PR adds some logging and explicitly sets the
tcp_keepalives_idle
value to 900 seconds.