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

Wait for service availability at the ingress before running tests #267

Closed
wants to merge 3 commits into from

Conversation

tillrohrmann
Copy link
Contributor

This commit makes the RestateDeployer wait for the availability of registered
services at the ingress. The deployer checks whether the ingress is serving
the registered services via calling grpc.health.v1.Health/Check and checking
that the response contains status = SERVING.

This fixes #266.

@@ -1,5 +1,5 @@
# Timeout config
junit.jupiter.execution.timeout.testable.method.default=10 s
junit.jupiter.execution.timeout.testable.method.default=15 s
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This timeout has been increased by 5 seconds because it can take up to 5s until the new schema information has been propagated from the schema registry to the ingress.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is gonna kill the execution time of tests... Could we have a way to significantly lower down the polling time, to say 100ms, for tests just by configuring an env variable here https://github.com/restatedev/e2e/blob/main/tests/build.gradle.kts#L51?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also isn't 5 seconds generally too much? Also for the local dev experience...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added a commit which eagerly pushes schema information changes to the worker. This will make the schema information schema propagation appear almost instantaneously. Will revert this change then.

This commit makes the RestateDeployer wait for the availability of registered
services at the ingress. The deployer checks whether the ingress is serving
the registered services via calling grpc.health.v1.Health/Check and checking
that the response contains status = SERVING.

This fixes restatedev#266.
@tillrohrmann
Copy link
Contributor Author

ping @slinkydeveloper

@slinkydeveloper
Copy link
Collaborator

Is this still relevant?

@tillrohrmann
Copy link
Contributor Author

I think technically yes once we go distributed. Right now it just happens to not be needed because of the single process setup. I guess we no longer have the health service which we can call to check for the registered services. Is there another way at the Ingress to figure out whether a service is known with the new ingress?

@slinkydeveloper
Copy link
Collaborator

I guess we no longer have the health service which we can call to check for the registered services. Is there another way at the Ingress to figure out whether a service is known with the new ingress?

GET /restate/health returns back the list of available components

@tillrohrmann
Copy link
Contributor Author

Closing since the PR is currently not needed and quite out of sync with the current code base.

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.

Let RestateDeployer wait until the schema information has propagated to the ingress
2 participants