Skip to content

KAFKA-12904: Corrected the timeout for config validation REST API resource#10834

Merged
rhauch merged 1 commit into
apache:trunkfrom
rhauch:KAFKA-12904
Jun 22, 2021
Merged

KAFKA-12904: Corrected the timeout for config validation REST API resource#10834
rhauch merged 1 commit into
apache:trunkfrom
rhauch:KAFKA-12904

Conversation

@rhauch

@rhauch rhauch commented Jun 7, 2021

Copy link
Copy Markdown
Contributor

The constant is specified in milliseconds, and so the MILLISECOND time unit should be used instead of SECONDS. See #8069/KAFKA-9374.

Inspected other uses of the same constant and other changes in the original PR, and found no other errors. This kind of change is actually difficult to test for, so this relies upon existing unit and integration tests.

Users may run into this whenever validating a connector configuration where the connector implementation takes more than the 90 seconds to actually validate the configuration.

  • Without this fix, the PUT /connector-plugins/(string:name)/config/validate REST requests might not return 500 Internal Server Error and may block (the request thread) for a long period of time.
  • With this fix, the PUT /connector-plugins/(string:name)/config/validate REST requests might not return 500 Internal Server Error if the connector does not complete the validation of a connector configuration within 90 seconds.

The user will not see a difference between the behavior before or after this fix if/when the connectors complete validation of connector configurations before 90 seconds, since the method will return those results to the client.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

…ource

The constant is specified in milliseconds, and so the MILLISECOND time unit should be used instead of SECONDS.
@rhauch rhauch added the connect label Jun 7, 2021
@rhauch
rhauch requested a review from kkonstantine June 7, 2021 15:50

@kkonstantine kkonstantine left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Should we add how or when we'd expect users to hit this? Here and on the jira maybe. The description is brief at the moment.

@chia7712 chia7712 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@rhauch
rhauch merged commit b285662 into apache:trunk Jun 22, 2021
rhauch added a commit that referenced this pull request Jun 22, 2021
…ource (#10834)

The constant is specified in milliseconds, and so the MILLISECOND time unit should be used instead of SECONDS.

Author: Randall Hauch <rhauch@gmail.com>
Reviewer: Konstantine Karantasis <konstantine@confluent.io>
rhauch added a commit that referenced this pull request Jun 22, 2021
…ource (#10834)

The constant is specified in milliseconds, and so the MILLISECOND time unit should be used instead of SECONDS.

Author: Randall Hauch <rhauch@gmail.com>
Reviewer: Konstantine Karantasis <konstantine@confluent.io>
rhauch added a commit that referenced this pull request Jun 22, 2021
…ource (#10834)

The constant is specified in milliseconds, and so the MILLISECOND time unit should be used instead of SECONDS.

Author: Randall Hauch <rhauch@gmail.com>
Reviewer: Konstantine Karantasis <konstantine@confluent.io>
xdgrulez pushed a commit to xdgrulez/kafka that referenced this pull request Dec 22, 2021
…ource (apache#10834)

The constant is specified in milliseconds, and so the MILLISECOND time unit should be used instead of SECONDS.

Author: Randall Hauch <rhauch@gmail.com>
Reviewer: Konstantine Karantasis <konstantine@confluent.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants