Skip to content

tests: make aggregate_state_exception_memory_leak deterministic#38754

Merged
alexey-milovidov merged 1 commit intoClickHouse:masterfrom
azat:tests-aggregate_state_exception_memory_leak-fix
Jul 9, 2022
Merged

tests: make aggregate_state_exception_memory_leak deterministic#38754
alexey-milovidov merged 1 commit intoClickHouse:masterfrom
azat:tests-aggregate_state_exception_memory_leak-fix

Conversation

@azat
Copy link
Copy Markdown
Member

@azat azat commented Jul 3, 2022

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

Sometimes if the 30 seconds is not enough for the query to fail (i.e. under TSan),
the error will be ignored and the test will fail.

Drop that loops away, since the query fails everytime and convert it to simple .sql test.

Follow-up for: #11496 (cc @alexey-milovidov )

@robot-ch-test-poll robot-ch-test-poll added the pr-not-for-changelog This PR should not be mentioned in the changelog label Jul 3, 2022
@azat azat force-pushed the tests-aggregate_state_exception_memory_leak-fix branch from 7b0aac0 to 51f60f5 Compare July 3, 2022 16:11
@alexey-milovidov
Copy link
Copy Markdown
Member

alexey-milovidov commented Jul 3, 2022

But the whole point of this test was to check for a memory leak by triggering OOM by running the query in a loop, and specifically the case when it throws an exception.

@alexey-milovidov alexey-milovidov self-assigned this Jul 3, 2022
@azat azat marked this pull request as draft July 3, 2022 21:11
@azat azat force-pushed the tests-aggregate_state_exception_memory_leak-fix branch from 51f60f5 to d1d717d Compare July 5, 2022 10:46
@azat azat marked this pull request as ready for review July 5, 2022 10:46
@azat
Copy link
Copy Markdown
Member Author

azat commented Jul 5, 2022

But the whole point of this test was to check for a memory leak by triggering OOM by running the query in a loop, and specifically the case when it throws an exception.

I see, thanks. Fixed the patch.

@alexey-milovidov alexey-milovidov merged commit a7d5107 into ClickHouse:master Jul 9, 2022
@azat azat deleted the tests-aggregate_state_exception_memory_leak-fix branch July 9, 2022 05:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-not-for-changelog This PR should not be mentioned in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants