Skip to content

Update TabletManagementIteratorIT for merge and wal checks#3923

Merged
cshannon merged 6 commits into
apache:elasticityfrom
cshannon:accumulo-3908
Nov 7, 2023
Merged

Update TabletManagementIteratorIT for merge and wal checks#3923
cshannon merged 6 commits into
apache:elasticityfrom
cshannon:accumulo-3908

Conversation

@cshannon
Copy link
Copy Markdown
Contributor

@cshannon cshannon commented Nov 3, 2023

This updates tests in TabletManagementIteratorIT to verify that the iterator properly detects tablets needing attention if WALs exist on tablets that are part of a merge operation. It also will test that if op type is DELETING that the tablet does not care if there are WALs.

This PR only updates the tests in TabletManagementIteratorIT and doesn't try and do anything with TGW because after the refactoring in #3904 both the iterator and TGW share the same code for checking the goal state (where the WAL checks are done)

However, another possible follow on test improvement would be to create a test for the TabletGoalState class where all the logic was refactored to inside of compute() to test the different cases for computing the resulting state. This could likely just be a normal unit test with mocking and not an IT.

This updates tests in TabletManagementIteratorIT to verify that the
iterator properly detects tablets needing attention if WALs exist on
tablets that are part of a merge operation
@cshannon cshannon requested a review from keith-turner November 3, 2023 12:33
@cshannon cshannon self-assigned this Nov 3, 2023
Copy link
Copy Markdown
Contributor

@keith-turner keith-turner left a comment

Choose a reason for hiding this comment

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

Would be nice to check split in addition to merge in the test.

@cshannon cshannon merged commit 48151e5 into apache:elasticity Nov 7, 2023
@cshannon
Copy link
Copy Markdown
Contributor Author

cshannon commented Nov 7, 2023

Would be nice to check split in addition to merge in the test.

I added split as well on the latest update

@cshannon cshannon deleted the accumulo-3908 branch November 7, 2023 13:08
@ctubbsii ctubbsii added this to the 4.0.0 milestone Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

3 participants