Skip to content

Increase query cancellation timeout in the router#16656

Merged
cryptoe merged 9 commits intoapache:masterfrom
findingrish:increase_cancellation_timeout_router
Aug 7, 2024
Merged

Increase query cancellation timeout in the router#16656
cryptoe merged 9 commits intoapache:masterfrom
findingrish:increase_cancellation_timeout_router

Conversation

@findingrish
Copy link
Copy Markdown
Contributor

Currently the timeout duration is hardcoded to 500 ms, this leads to frequent query cancellation failures when the Brokers are under load.

Increasing this value to 1 second.

private static final String PROPERTY_SQL_ENABLE_DEFAULT = "false";

private static final int CANCELLATION_TIMEOUT_MILLIS = 500;
private static final int CANCELLATION_TIMEOUT_MILLIS = 1000;
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.

Should we bump it up to 5 seconds ?
The reason is for a cluster that is fully loaded, a cancel call would help ease the pressure on the broker.
cc @abhishekagarwal87?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

How did you arrive at the 5 second number?

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.

Guesstimate mostly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated to 5 seconds.

private static final String PROPERTY_SQL_ENABLE_DEFAULT = "false";

private static final int CANCELLATION_TIMEOUT_MILLIS = 500;
private static final int CANCELLATION_TIMEOUT_MILLIS = 1000;
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.

Nit: let's avoid magic number 😄

Suggested change
private static final int CANCELLATION_TIMEOUT_MILLIS = 1000;
private static final int CANCELLATION_TIMEOUT_MILLIS = TimeUnit.SECONDS.toMillis(1);;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

@cryptoe cryptoe merged commit c6a7ab0 into apache:master Aug 7, 2024
@kfaraz kfaraz added this to the 31.0.0 milestone Oct 4, 2024
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.

4 participants