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

Respect fallbackPollIntervalInMs configuration #2707

Closed
alexlopashev opened this issue Aug 1, 2023 · 0 comments · Fixed by #2709
Closed

Respect fallbackPollIntervalInMs configuration #2707

alexlopashev opened this issue Aug 1, 2023 · 0 comments · Fixed by #2709

Comments

@alexlopashev
Copy link

#1564 introduced fallbackPollIntervalInMs configuration property to allow customization of schema updates polling period.

UplinkSupergraphManager validates this value and sets a floor value of 10 seconds to prevent unnecessary load of Uplink API.

However, initial value of pollIntervalMs can be overridden by SupergraphSdlUpdate response data, particularly these lines (https://github.com/apollographql/federation/blob/main/gateway-js/src/supergraphManagers/UplinkSupergraphManager/index.ts#L230):

if (result?.minDelaySeconds) {
    this.pollIntervalMs = result.minDelaySeconds * 1000;
}

So far it looks like minDelaySeconds is always set to be 30 seconds. This behavior prevents us to define a larger update window, and probably is a simple oversight.

We suggest updating lines in question to:

if (result?.minDelaySeconds) {
    this.pollIntervalMs = Math.max(this.pollIntervalMs, result.minDelaySeconds * 1000);
}

This way we can define larger update windows, while still respecting semantics of minDelaySeconds.

Additionally, I wonder if Apollo team sees value in allowing disabling schema updates altogether, e.g. by providing fallbackPollIntervalInMs < 0. This would be handy for our experiments, but might not be as useful for majority of users. with a workaround of providing a very large value instead.

One important note: because setTimeout is used to schedule polling requests, it might be useful to document that fallbackPollIntervalInMs must not exceed value of 2,147,483,647 ms (about 24.8 days): https://developer.mozilla.org/en-US/docs/Web/API/setTimeout#maximum_delay_value

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 a pull request may close this issue.

1 participant