-
Notifications
You must be signed in to change notification settings - Fork 723
add Full cycle local backup restore test #26574
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
⚪
🟢 |
⚪
🟢 |
🟢 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Добавь в родительский ya.make что бы система подхватила тесты (ybd/tests/functional/ya.make)
Тесты не проходят локально, но есть возможность, что это на моей стороне что-то: $ ya make -ttt -k --test-disable-timeout --build relwithdebinfo
...
[fail] basic_user_scenarios.py::TestFullCycleLocalBackupRestore::test_full_cycle_local_backup_restore [default-linux-x86_64-relwithdebinfo] (25.38s)
ydb/tests/functional/backup_collection/basic_user_scenarios.py:251: in test_full_cycle_local_backup_restore
assert backup_res2.exit_code == 0, "BACKUP (2) failed"
E AssertionError: BACKUP (2) failed
E assert 1 == 0
E + where 1 = <yatest.common.process._Execution object at 0x7f2f1af2ca60>.exit_code
Log: /home/innokentii/ydbwork2/ydb/ydb/tests/functional/backup_collection/test-results/py3test/basic_user_scenarios/chunk0/testing_out_stuff/basic_user_scenarios.py.TestFullCycleLocalBackupRestore.test_full_cycle_local_backup_restore.log
Logsdir: /home/innokentii/ydbwork2/ydb/ydb/tests/functional/backup_collection/test-results/py3test/basic_user_scenarios/chunk0/testing_out_stuff
...
------ FAIL: 1 - FAIL ydb/tests/functional/backup_collection
Total 3 suites:
2 - GOOD
1 - FAIL
Total 4 tests:
3 - GOOD
1 - FAIL
Failed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Так же возможно стоит проверять некоторые минимальные инварианты в скимшарде, что бэкап коллекция существует или таблицы в ней и т.п.
|
||
def parse_yql_table(text): | ||
if not text: | ||
return [] | ||
|
||
lines = text.splitlines() | ||
rows = [] | ||
border_re = re.compile(r'^[\s┌┬┐└┴┘├┼┤─]+$') | ||
for ln in lines: | ||
ln = ln.rstrip("\r\n") | ||
if not ln: | ||
continue | ||
if border_re.match(ln): | ||
continue | ||
if '│' not in ln: | ||
continue | ||
parts = [p.strip() for p in ln.split('│')] | ||
if parts and parts[0] == '': | ||
parts = parts[1:] | ||
if parts and parts[-1] == '': | ||
parts = parts[:-1] | ||
rows.append(parts) | ||
return rows |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Точно ли нам нужен ручной парсер? Не стоит ли воспользоваться SDK?
create_res = yatest.common.execute( | ||
[backup_bin(), "--endpoint", "grpc://localhost:%d" % self.cluster.nodes[1].grpc_port, | ||
"--database", "/Root", "yql", "--script", create_collection_sql], | ||
check_exit_code=False, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Возможно стоит вынести для читаемости
def _execute_yql(self, script):
cmd = [
backup_bin(),
"--endpoint",
f"grpc://localhost:{self.cluster.nodes[1].grpc_port}",
"--database",
self.root_dir,
"yql",
"--script",
script,
]
return yatest.common.execute(cmd, check_exit_code=False)
Стоит основную функцию сделать чуть более "высокоуровневой". Что-то вроде: def test_full_cycle_local_backup_restore(self):
# Setup
collection_src, t1, t2 = self._setup_test_collections()
# Create and backup
self._create_initial_backup(collection_src, t1, t2)
snapshot1 = self._capture_snapshot(t1)
# Modify and backup again
self._modify_and_backup(collection_src)
snapshot2 = self._capture_snapshot(t1)
# Export backups
export_dir = self._export_backups(collection_src)
# Restore and verify
self._verify_restore(export_dir, snapshot1, snapshot2) |
create_collection_sql = ( | ||
f"CREATE BACKUP COLLECTION `{collection_src}`\n" | ||
f" ( TABLE `{full_t1}`\n" | ||
f" , TABLE `{full_t2}`\n" | ||
f" )\n" | ||
"WITH\n" | ||
" ( STORAGE = 'cluster'\n" | ||
" , INCREMENTAL_BACKUP_ENABLED = 'false'\n" | ||
" );\n" | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Лучше так
query = f"""
CREATE BACKUP COLLECTION `{collection_src}`
( TABLE `{full_t1}
, TABLE `{full_t2}
)
WITH
...
"""
full_t1 = f"/Root/{t1}" | ||
full_t2 = f"/Root/{t2}" | ||
|
||
session = self.driver.table_client.session().create() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with self.driver.table_client.session().create() as session:
create_table_with_data(session, t1)
# ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
не уверен, что в тестах это прям актуально
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
И ещё немного коментов
у меня тоже с оптимизированной сборкой падает... Total 3 suites: |
Changelog entry
Create test of full cycle local backup restore
...
Changelog category
Description for reviewers
Create test of full cycle local backup restore
...