Run olap scenario tests on local ydb cluster #12512
Conversation
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
There was a problem hiding this comment.
Мне не нравится идея затащить Harness кластер прямо в ydb_cluster. Конфигурация кластера может быть разная для разных тестов, передавать это сюдя будет неудобно. Буквально недавно я решал полностью аналогичную задачу для нагрузочных тестов, вот тут код, посмотри: https://github.com/ydb-platform/ydb/blob/main/ydb/tests/functional/tpc/conftest.py
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
Кроме предыдущего. Я бы разделил тесты, запускаемые локально и тесты, которые запускаются на кластере, эти множества могут не совпадать. Делил бы я их через запуск в разных ya.make c переиспользованием кода тестов. Примеры опять же из нагрузочных. Локально: https://github.com/ydb-platform/ydb/blob/main/ydb/tests/functional/tpc/ya.make , на кластере: https://github.com/ydb-platform/ydb/blob/main/ydb/tests/olap/load/ya.make |
ydb/tests/olap/lib/ydb_cluster.py
Outdated
| def _start_ydb_cluster(): | ||
| config = KikimrConfigGenerator(extra_feature_flags=[]) | ||
| cluster = KiKiMR(configurator=config) | ||
| cluster.start() |
There was a problem hiding this comment.
И кстати, при таком старте не создается db, а без нее ydb работает странно. Например не создается таблетка статистики. Пример как стартовать есть в нагрузочных тестах выше.
There was a problem hiding this comment.
Довольно много тестов сейчас создают кластер именно таким способом. Если это некорректно, то нужно править их все. Можно создать тикет на это.
There was a problem hiding this comment.
Ну конкретно на отсутствие таблетки со статистикой мы с Монстром наткнулись. Я не могу сказать, что ещё не создалось. Может и ничего критичного
ydb/tests/olap/lib/ydb_cluster.py
Outdated
| ydb_endpoint = get_external_param('ydb-endpoint', 'grpc://ydb-olap-testing-vla-0002.search.yandex.net:2135') | ||
| ydb_database = get_external_param('ydb-db', 'olap-testing/kikimr/testing/acceptance-2').lstrip('/') | ||
|
|
||
| ydb_endpoint = get_external_param('ydb-endpoint', "") |
There was a problem hiding this comment.
Обычно в питоне пишут что-то типа
ydb_endpoint = get_external_param('ydb-endpoint', None)
А потом что-то типа
if ydb_endpoint is not None:
...
ydb/tests/olap/lib/ydb_cluster.py
Outdated
| def _start_ydb_cluster(): | ||
| config = KikimrConfigGenerator(extra_feature_flags=[]) | ||
| cluster = KiKiMR(configurator=config) | ||
| cluster.start() |
There was a problem hiding this comment.
А где stop для кластера?
There was a problem hiding this comment.
Я, конечно, допишу stop. А что бывает, если до stop не дойдёт выполнение? Кластера будут висеть вечно?
There was a problem hiding this comment.
Я думаю его убьет ya make в неопределенный момент времени (но не уверен)
There was a problem hiding this comment.
Не нашёл в текущей реализации хелперов, куда можно вставить stop, чтобы он гарантированно вызывался. Можно переписать все хелперы со статических классов на обычные инстансы, тогда можно будет останавливать в teardown_class. Но это требует больших изменений, чем этот PR.
Я в питоне не спец, если подскажетет, куда можно вставить stop, вставлю.
Проверил, что при завершении запуска через ya test запущенный кластер тоже ydb завершается.
There was a problem hiding this comment.
Сделал остановку с помощью atexit
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
ydb/tests/olap/lib/ydb_cluster.py
Outdated
| ydb_database = get_external_param('ydb-db', 'olap-testing/kikimr/testing/acceptance-2').lstrip('/') | ||
|
|
||
| ydb_endpoint = get_external_param('ydb-endpoint', "") | ||
| ydb_database = get_external_param('ydb-db', "/Root").lstrip('/') |
There was a problem hiding this comment.
Тут же некорректно кажется написано, если этот дефолт на случай "так называется база в KiKiMR(...)", надо явно спросить название у KiKiMR
There was a problem hiding this comment.
Поправил. Убрал странный дефолт
ydb/tests/olap/lib/ydb_cluster.py
Outdated
| endpoint = cls.ydb_endpoint | ||
| if not endpoint: | ||
| if cls._ydb_cluster is not None: | ||
| raise "Double temporary cluster initialization attempt" |
There was a problem hiding this comment.
Я если честно не понял на какой это случай сделано
There was a problem hiding this comment.
Проверка инварианта: сюда попадать не должны
zverevgeny
force-pushed
the
olap_scenario_tests_on_local_ydb
branch
from
c383531 to
565629a
Compare
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
Тогда непонятна зона ответсвенности класса YdbCluster. Сейчас это "синглтон", предоставляющий доступ к какому-то реальному ydb кластеру. Правильно ли я понимаю, что ты предлагаешь разделить этот класс, по-сути, на два: |
Пока нет необходимости в разделении. Хотим уметь запускать все тесты и локально и на долгоживущем кластере. |
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
ydb/tests/olap/lib/ydb_cluster.py
Outdated
| self._database = config.domain_name | ||
| self._temp_ydb_cluster = cluster | ||
| LOGGER.info(f'Using YDB, endpoint:{self._endpoint}, database:{self._database}') | ||
| atexit.register(self.__del__) |
There was a problem hiding this comment.
Это какое-то хакерство, предлагаю сделать явный метод stop
ydb/tests/olap/lib/ydb_cluster.py
Outdated
| self._endpoint = endpoint | ||
| self._database = database | ||
| else: | ||
| config = KikimrConfigGenerator(extra_feature_flags=[]) |
There was a problem hiding this comment.
extra_feature_flags лишнее, оно скорее всего и так в дефолте пустое
ydb/tests/olap/lib/ydb_cluster.py
Outdated
| def get_ydb_driver(cls): | ||
| if cls._ydb_driver is None: | ||
| if cls._ydb_cluster_instance is not None: | ||
| raise 'Double cluster initialization attempt' |
There was a problem hiding this comment.
Я чет все равно не понимаю на какой случай этот raise. На какой?
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
ydb/tests/olap/lib/ydb_cluster.py
Outdated
| raise | ||
|
|
||
| @classmethod | ||
| def reset(cls, ydb_endpoint, ydb_database): |
There was a problem hiding this comment.
Еще mon_port надо выставлять.
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
Conflicts: .github/config/muted_ya.txt ydb/tests/olap/scenario/ya.make
)" This reverts commit 71f8e3a.
Conflicts: .github/config/muted_ya.txt ydb/tests/olap/scenario/ya.make
Conflicts: .github/config/muted_ya.txt ydb/tests/olap/scenario/ya.make
3 participants
Changelog category