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

Stricter checks before transitioning table state #4165

Merged
merged 2 commits into from
Jan 17, 2024

Conversation

kevinrr888
Copy link
Member

Closes #4132

Prior to these changes, users were able to manually transition a table from the NEW state to another state. More specifically, calls to TableOperations.online() and TableOperations.offline() when the table was in the NEW state were acceptable. Users should not be able to transition a table from the NEW state as this should only be done by the system upon successfully creating/cloning/importing the table.

Changes:

  • Added new param expectedCurrStates to TableManager.transitionTableState()
  • Users can now only manually change a table from [ONLINE/OFFLINE]->[ONLINE/OFFLINE] (FateServiceHandler)
  • Added field expectedCurrStates to ChangeTableState
  • Table clones, creations, and imports now explicitly expect the current state before completion to be NEW (FinishCloneTable, FinishCreateTable, FinishImportTable)
  • Table deletions now explicitly expect the current state before deletion to be only ONLINE or OFFLINE (DeleteTable)

Closes apache#4132
Prior to these changes, users were able to manually transition a table from the NEW state to another state. More specifically, calls to TableOperations.online() and TableOperations.offline() when the table was in the NEW state were acceptable. Users should not be able to transition a table from the NEW state as this should only be done by the system upon successfully creating/cloning/importing the table.
Changes:
- Added new param expectedCurrStates to TableManager.transitionTableState()
- Users can now only manually change a table from [ONLINE/OFFLINE]->[ONLINE/OFFLINE] (FateServiceHandler)
- Added field expectedCurrStates to ChangeTableState
- Table clones, creations, and imports now explicitly expect the current state before completion to be NEW (FinishCloneTable, FinishCreateTable, FinishImportTable)
- Table deletions now explicitly expect the current state before deletion to be only ONLINE or OFFLINE (DeleteTable)
@DomGarguilo
Copy link
Member

Overall looks good. The only suggestion I can think of for now is to use EnumSet<TableState> for the expected set if possible.

@kevinrr888
Copy link
Member Author

Overall looks good. The only suggestion I can think of for now is to use EnumSet<TableState> for the expected set if possible.

Good suggestion. Changed

Copy link
Member

@DomGarguilo DomGarguilo left a comment

Choose a reason for hiding this comment

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

LGTM

@DomGarguilo DomGarguilo merged commit cefa3a4 into apache:2.1 Jan 17, 2024
8 checks passed
@ctubbsii ctubbsii modified the milestones: 3.1.0, 2.1.3 Jul 12, 2024
@kevinrr888 kevinrr888 deleted the 2.1-feature-4132 branch November 1, 2024 15:08
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.

Investigate making tableOperations.online() more strict with table state checks
4 participants