Skip to content

Comments

--skip-drop-table by default#363

Merged
winebarrel merged 1 commit intoridgepole:1.0from
chumaltd:skip_drop_table_by_default
Dec 18, 2021
Merged

--skip-drop-table by default#363
winebarrel merged 1 commit intoridgepole:1.0from
chumaltd:skip_drop_table_by_default

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
Member

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.

3 participants