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

feat: update workers service in proxy #1107

Merged

Conversation

SantiagoPittella
Copy link
Collaborator

This PR adds a new service to the proxy server, which is in charge of updating the workers in the load balancer. This allows of removing the responsibility from the proxy itself.

closes #1003

@SantiagoPittella SantiagoPittella force-pushed the santiagopittella-update-workers-endpoint-in-proxy branch from 45c7cfb to 671e116 Compare January 24, 2025 20:35
Copy link
Contributor

@bobbinth bobbinth left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you. I left some comments inline.

CHANGELOG.md Outdated Show resolved Hide resolved
bin/proving-service/src/commands/mod.rs Outdated Show resolved Hide resolved
bin/proving-service/src/commands/update_workers.rs Outdated Show resolved Hide resolved
bin/proving-service/src/proxy/update_workers.rs Outdated Show resolved Hide resolved
bin/proving-service/src/proxy/update_workers.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@bobbinth bobbinth left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you! I left one small comment inline.

bin/proving-service/src/proxy/update_workers.rs Outdated Show resolved Hide resolved
@SantiagoPittella SantiagoPittella force-pushed the santiagopittella-update-workers-endpoint-in-proxy branch from 87dd71d to 5353d9d Compare January 28, 2025 14:51
WORKER_UNHEALTHY.inc_by(unhealthy_workers as u64);

// Sleep for the defined interval before the next health check
sleep(self.health_check_frequency).await;
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: It sounds like this is not the frequency (ie, health checks per unit of time) but more like an interval or period so maybe the variable can be renamed

@SantiagoPittella SantiagoPittella merged commit beddc2a into next Jan 30, 2025
12 checks passed
@SantiagoPittella SantiagoPittella deleted the santiagopittella-update-workers-endpoint-in-proxy branch January 30, 2025 20:11
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.

3 participants