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

fix: Set timeout for pull query forwarded requsts to 20s #9823

Merged
merged 2 commits into from Mar 17, 2023

Conversation

AlanConfluent
Copy link
Member

@AlanConfluent AlanConfluent commented Mar 9, 2023

Description

Sets a timeout for pull query requests which are forwarded to 20s by default, though allows it to be set by the client.

Testing done

Unit tests, plus was able to test manually.

Reviewer checklist

  • Ensure docs are updated if necessary. (eg. if a user visible feature is being added or changed).
  • Ensure relevant issues are linked (description should include text like "Fixes #")

@AlanConfluent AlanConfluent requested a review from a team as a code owner March 9, 2023 22:27
if (configOverrides.containsKey(KsqlConfig.KSQL_QUERY_PULL_FORWARDING_TIMEOUT_MS_CONFIG)) {
return (Long) configOverrides.get(KsqlConfig.KSQL_QUERY_PULL_FORWARDING_TIMEOUT_MS_CONFIG);
}
return ksqlConfig.getLong(KsqlConfig.KSQL_QUERY_PULL_FORWARDING_TIMEOUT_MS_CONFIG);
Copy link
Member

Choose a reason for hiding this comment

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

shouldnt this just be returning KsqlConfig.KSQL_QUERY_PULL_FORWARDING_TIMEOUT_MS_DEFAULT if no override is found

Copy link
Member Author

Choose a reason for hiding this comment

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

If you do that, you won't be able to set this value in the config file. If you do ksqlConfig.getLong(KsqlConfig.KSQL_QUERY_PULL_FORWARDING_TIMEOUT_MS_CONFIG);, it will attempt to find it in the config file, and if not, default to KSQL_QUERY_PULL_FORWARDING_TIMEOUT_MS_DEFAULT.

Copy link
Member

Choose a reason for hiding this comment

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

ah okay i see, makes sense!

@AlanConfluent AlanConfluent changed the title fix: Set timeout for pull query forwarded requsts to 15s fix: Set timeout for pull query forwarded requsts to 20s Mar 17, 2023
@AlanConfluent AlanConfluent merged commit 7ac28eb into confluentinc:master Mar 17, 2023
2 checks passed
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.

None yet

2 participants