Skip to content

Add sleep function for testing#11626

Merged
maytasm merged 3 commits intoapache:masterfrom
jihoonson:sleep-function
Aug 24, 2021
Merged

Add sleep function for testing#11626
maytasm merged 3 commits intoapache:masterfrom
jihoonson:sleep-function

Conversation

@jihoonson
Copy link
Contributor

@jihoonson jihoonson commented Aug 23, 2021

Description

This PR adds a new function, sleep, that makes the current thread sleep for the given amount of seconds. This function is applied per row, and thus Druid does not guarantee the consistent sleep time across queries. Instead, the actual sleep time can vary depending on how much parallelism is used for each query. As a result, this function is intended to use only for testing when you want to keep some query running for a long period.

Currently, the sleep SQL function is evaluated during the query planning when it accepts a number literal. I think this is OK for now as the function is supposed to be used only in tests.


Key changed/added classes in this PR
  • Sleep

This PR has:

  • been self-reviewed.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.

Copy link
Contributor

@abhishekagarwal87 abhishekagarwal87 left a comment

Choose a reason for hiding this comment

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

LGTM

@maytasm maytasm merged commit 78b4be4 into apache:master Aug 24, 2021
@clintropolis
Copy link
Member

clintropolis commented Aug 24, 2021

I don't really understand the reason for this to exist in the core Druid expression system, both expressions and SQL operators can be defined in extensions if this is needed for some sort of test, and it seems a potential risk to allow queries to run basically indefinitely(?) and hold resources.

@maytasm
Copy link
Contributor

maytasm commented Aug 24, 2021

I don't really understand the reason for this to exist in the core Druid expression system, both expressions and SQL operators can be defined in extensions if this is needed for some sort of test, and it seems a potential risk to allow queries to run basically indefinitely(?) and hold resources.

It's undocumented so I think there is less chance of anyone actually using this in production in non-test scenario. I think refactoring this into an extension for test purposes codes is also a good idea.

@jihoonson
Copy link
Contributor Author

I don't really understand the reason for this to exist in the core Druid expression system, both expressions and SQL operators can be defined in extensions if this is needed for some sort of test, and it seems a potential risk to allow queries to run basically indefinitely(?) and hold resources.

I agree with your concern and that's why I didn't document it. My intention was using this function in only tests (both unit tests and integration tests) since I don't see how this function can be useful in production in any case. A feature flag seems even safer than undocumented feature. I will make a follow-up PR for it.

@jihoonson jihoonson mentioned this pull request Aug 24, 2021
2 tasks
@clintropolis clintropolis added this to the 0.22.0 milestone Sep 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants