Fix server restart after stress test#13743
Merged
alexey-milovidov merged 3 commits intomasterfrom Aug 15, 2020
Merged
Conversation
robot-clickhouse
added
the
pr-not-for-changelog
| * </source> | ||
| */ | ||
| void buildSourceConfiguration(AutoPtr<Document> doc, AutoPtr<Element> root, const ASTFunctionWithKeyValueArguments * source, const ASTDictionarySettings * settings) | ||
| void buildSourceConfiguration( |
Member
Author
There was a problem hiding this comment.
Whitespaces only.
| bool complex = DictionaryFactory::instance().isComplex(dictionary_layout->layout_type); | ||
|
|
||
| auto all_attr_names_and_types = buildDictionaryAttributesConfiguration(xml_document, structure_element, query.dictionary_attributes_list, pk_attrs); | ||
| auto all_attr_names_and_types = buildDictionaryAttributesConfiguration( |
Member
Author
There was a problem hiding this comment.
Whitespaces only.
|
|
||
| CREATE DICTIONARY dict (n UInt64, col String DEFAULT '42') PRIMARY KEY n | ||
| SOURCE(CLICKHOUSE(HOST 'localhost' PORT 9440 SECURE 1 USER 'default' TABLE 'url' DB 'test_01083')) LIFETIME(1) LAYOUT(CACHE(SIZE_IN_CELLS 1)); | ||
| SOURCE(CLICKHOUSE(HOST 'localhost' PORT 9440 SECURE 1 USER 'default' TABLE 'url')) LIFETIME(1) LAYOUT(CACHE(SIZE_IN_CELLS 1)); |
Member
Author
There was a problem hiding this comment.
This is the significant change.
Member
Author
|
Member
Author
|
Member
Author
|
Stress tests Ok. |
This was referenced Aug 15, 2020
Member
Author
|
It did not solve the issue completely. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changelog category (leave one):
You may notice that the last two weeks some stress test runs fail sporadically with the "Server failed to start" message.
It started to happen because we have added more checks to the stress test and it does not indicate any new bugs.
Stress test is a check that runs all the queries from functional tests with random order and with multiple threads without checking the results. It checks the following:
We have added one more check:
This check will fail if incorrect tables are spontaneously created during the test, and after server is restarted, it will find these incorrect tables and refuse to start without manual cleanup.
The check is reasonable, because manual cleanup should not be required in general and it's often impossible in managed ClickHouse setups such as Yandex Cloud.
But the check started to fail. There were several causes:
The first was due to leftover
.sql.tmpfile after unsuccessful table creation: #13557The second was due to tables created with
CREATE ... AS table_function(...).It is possible to create a table as
remote,clusterormergetable function and the function will be re-evaluated during server startup. The order of tables initialization is undefined and it's possible that themergetable function will not find the corresponding tables, throw an exception and server will refuse to start. This issue is minor.In this pull request, I've edited the
01083_expressions_in_engine_arguments.sqltest, so that all tables will refer only the current database. And current database is destroyed after the test, so no leftovers can remain after the test and server will restart successfully.This modification required a new feature - the possibility to create a dictionary with ClickHouse source in current database if database was not specified (in previous versions, default database was used instead of current database).
The root cause of the issue is not fixed.