Skip to content
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

Implement --restore-table-mapping for restore and restore_remote #944

Merged
merged 9 commits into from
Jul 4, 2024

Conversation

nithin-vunet
Copy link
Contributor

@nithin-vunet nithin-vunet commented Jul 1, 2024

Hi,
This PR implements --restore-table-mapping for restore commands like restore and restore_remote

fix #937

@Slach Slach self-requested a review July 2, 2024 04:00
@Slach
Copy link
Collaborator

Slach commented Jul 2, 2024

thanks for contribution

almost looks good
but we definitely need to pass integration tests

first of all

rm -f ./test/testflows/clickhouse_backup/tests/snapshots/cli.py.cli.snapshot
cp -v ./test/testflows/.env.example ./test/testflows/.env
change required environment variables
bash -xe ./test/testflows/run.sh

then clean all containers

for id in $(docker ps -a -q); do docker stop "$id"; docker rm -f "$id"; done;

and run

cp -v ./test/integration/.env.example ./test/integration/.env
./test/integration/run.sh

it requires docker and docker-compose

change ./test/integration/integration_test.go add
TestRestoreTableMapping similar with TestRestoreDatabaseMapping
or rename TestRestoreDatabaseMapping to TestRestoreMapping and extend it to cover --restore-table-mapping use cases

Copy link
Collaborator

@Slach Slach left a comment

Choose a reason for hiding this comment

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

need pass to integration tests

@nithin-vunet
Copy link
Contributor Author

cp -v ./test/integration/.env.example ./test/integration/.env
./test/integration/run.sh

When I run the integration tests, clickhouse server doesn't go healthy at all.
I figured it is due to the below volume map for clickhouse container.

      - ./dynamic_settings.sh:/docker-entrypoint-initdb.d/dynamic_settings.sh

Any idea what could be done?

@nithin-vunet
Copy link
Contributor Author

Also noticed that the testflows are failing, specifically at the gcs backup flows. Are these due to the code changes?

@Slach
Copy link
Collaborator

Slach commented Jul 3, 2024

cp -v ./test/integration/.env.example ./test/integration/.env

It will not work
you need to provide actual for variables values

@Slach
Copy link
Collaborator

Slach commented Jul 3, 2024

Also noticed that the testflows are failing, specifically at the gcs backup flows. Are these due to the code changes?

these tests just requires
provide
./test/testflows/.env

look to
./test/testflows/.env.example

you need provide actual values

@Slach
Copy link
Collaborator

Slach commented Jul 3, 2024

ok. let me fix testflows failures after change default config

@nithin-vunet
Copy link
Contributor Author

Where I do find the actual values to enter in the env file?

@Slach
Copy link
Collaborator

Slach commented Jul 3, 2024

@nithin-vunet
ok. now testflows tests passed

for integration test you could just create empty ./test/integration/.env

and run following command

CLICKHOUSE_VERSION=1.1.54394 LOG_LEVEL=debug RUN_TESTS=TestRestoreDatabaseMapping ./test/integration/run.sh

Distributed table d1 not restored properly and still reference to database1, because you changed regexp
here
https://github.com/Altinity/clickhouse-backup/pull/944/files#diff-e97997e8a71f01e4c09b9c689c4b1153f1aa283f98fb86b55971577c70cb8a13R304

2024/07/03 17:10:36.613524  info Check result database-2
2024/07/03 17:10:36.613561 debug SELECT count() FROM `database-2`.t1 logger=integration-test
2024/07/03 17:10:36.623627 debug SELECT count() FROM `database-2`.d1 logger=integration-test
    integration_test.go:2198:
                Error Trace:    /home/slach/src/github.com/altinity/clickhouse-backup/test/integration/integration_test.go:2198
                                                        /home/slach/src/github.com/altinity/clickhouse-backup/test/integration/integration_test.go:2240
                Error:          Received unexpected error:
                                code: 81, message: Database database1 doesn't exist
                Test:           TestRestoreDatabaseMapping

@nithin-vunet
Copy link
Contributor Author

@Slach Thanks for the instructions. I was able to fix the failed checks. Updated TestRestoreDatabaseMapping to TestRestoreMapping to include test for table mappings as well.

@nithin-vunet nithin-vunet requested a review from Slach July 4, 2024 05:04
@coveralls
Copy link

coveralls commented Jul 4, 2024

Pull Request Test Coverage Report for Build 9785351536

Details

  • 111 of 145 (76.55%) changed or added relevant lines in 8 files are covered.
  • 264 unchanged lines in 10 files lost coverage.
  • Overall coverage decreased (-1.9%) to 66.086%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pkg/clickhouse/clickhouse.go 4 6 66.67%
pkg/backup/table_pattern.go 49 59 83.05%
pkg/server/server.go 3 13 23.08%
pkg/backup/restore.go 49 61 80.33%
Files with Coverage Reduction New Missed Lines %
pkg/backup/restore.go 2 67.62%
pkg/backup/backuper.go 2 65.12%
cmd/clickhouse-backup/main.go 2 77.19%
pkg/storage/object_disk/object_disk.go 5 70.95%
pkg/status/status.go 9 70.2%
pkg/config/config.go 10 71.31%
pkg/storage/general.go 23 61.46%
pkg/server/server.go 23 56.5%
pkg/storage/s3.go 32 43.88%
pkg/storage/gcs.go 156 0.0%
Totals Coverage Status
Change from base Build 9774929954: -1.9%
Covered Lines: 8722
Relevant Lines: 13198

💛 - Coveralls

@nithin-vunet
Copy link
Contributor Author

@raspreet-vunet thanks for your contribution and inspiration

@Slach Slach added this to the 2.5.20 milestone Jul 4, 2024
@Slach Slach merged commit 23f8cab into Altinity:master Jul 4, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implementation --restore-table-mapping
3 participants