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

--skip-drop-table by default #363

Merged
merged 1 commit into from Dec 18, 2021

Conversation

chumaltd
Copy link
Contributor

@chumaltd chumaltd commented Sep 4, 2021

Hi, I'd like to have --skip-drop-table option by default. For dropping, --drop-table.
#105 proposed about this, but looks not discussed.

For Data safety

Obviously, resulting tail risk differs between default options.

  • Missing --skip-drop-table causes permanent DATA LOST.
  • Missing --drop-table requires just rerun.

For Table Partitioning

I tested Table Partitioning on PostgreSQL.
With --skip-drop-table option, ridgepole works mostly well.

Table Partitioning has multiple tables called "partitions" that actual data resides.
Partitions schema derives from a single abstruct "partitioned table", so ridgepole doesn't have chance to migrate "partitions".

With --skip-drop-table, we can migrate "partitioned table" as just plain tables and then followed by its partitions.
(I don't mean that ridgepole can migrate a plain table into a partitioned table. It requires manual operations anyway.)
With partitioning, we have many arbitrary partitions, which faces DATA LOST risk with current ridgepole default behavior.

Table partitioning is just a case, and we will have situations that ridgepole should let several tables go.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 1200239096

  • 0 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-19.3%) to 77.983%

Totals Coverage Status
Change from base Build 1176533959: -19.3%
Covered Lines: 928
Relevant Lines: 1190

💛 - Coveralls

@coveralls
Copy link

coveralls commented Sep 4, 2021

Pull Request Test Coverage Report for Build 1200239096

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 97.236%

Totals Coverage Status
Change from base Build 1176533959: 0.0%
Covered Lines: 1161
Relevant Lines: 1194

💛 - Coveralls

@winebarrel winebarrel self-requested a review September 7, 2021 02:08
@winebarrel winebarrel changed the base branch from 0.9 to 1.0 December 18, 2021 09:53
@winebarrel
Copy link
Collaborator

Thank you!

@winebarrel winebarrel merged commit 463f8dc into ridgepole:1.0 Dec 18, 2021
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.

None yet

3 participants