Skip to content

Removed TableInfo.majors field #3921

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.

Sign up for GitHub

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

Jump to bottom
Merged
dlmarion merged 1 commit into from
Nov 2, 2023
Merged

Removed TableInfo.majors field #3921

dlmarion merged 1 commit into from
Nov 2, 2023

Conversation

@dlmarion
Copy link
Contributor

@dlmarion dlmarion commented Nov 1, 2023

Fixes #3609

@dlmarion dlmarion self-assigned this Nov 1, 2023
@dlmarion dlmarion linked an issue Nov 1, 2023 that may be closed by this pull request
Remove TableInfo.major field from thrift struct #3609
Closed
@dlmarion
Copy link
Contributor Author

dlmarion commented Nov 1, 2023 •
edited
Loading

Before this change, the Monitor would likely show 0(0) for major compactions on the tserver page. I'm not sure what it will show now, it could even be broken. However, we might need to re-imagine how the Monitor is going to work anyway.

Edit: Looks like this issue might be covered in #3608

@keith-turner
Copy link
Contributor

keith-turner commented Nov 2, 2023 •
edited
Loading

Before this change, the Monitor would likely show 0(0) for major compactions on the tserver page. I'm not sure what it will show now, it could even be broken. However, we might need to re-imagine how the Monitor is going to work anyway.

Interesting, we do need to decide what we want to do with that column in the monitor. We could still gather per table information on queued and running compactions, but it would be done in a very different way that in used to. May need a follow on issue for that.

keith-turner
keith-turner approved these changes Nov 2, 2023
View reviewed changes
Copy link
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.

Thinking before this and its issue is closed out, need to open a new issue to decide what do with the column in the monitor.

@dlmarion
Copy link
Contributor Author

dlmarion commented Nov 2, 2023

Thinking before this and its issue is closed out, need to open a new issue to decide what do with the column in the monitor.

Do you think this is covered by #3608?

@keith-turner keith-turner mentioned this pull request Nov 2, 2023
Open
@keith-turner
Copy link
Contributor

keith-turner commented Nov 2, 2023

Do you think this is covered by #3608?

I added a comment to the issue about what still needs to be done.

@dlmarion dlmarion merged commit 818229b into apache:elasticity Nov 2, 2023
@dlmarion dlmarion deleted the 3609-remove-tableinfo-major-field branch November 2, 2023 16:02
@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

Reviewers

@keith-turner keith-turner keith-turner approved these changes

Assignees

@dlmarion dlmarion

Labels

None yet

Projects

Accumulo: Elasticity
Status: ✅ Done

Milestone

4.0.0

Development

Successfully merging this pull request may close these issues.

Remove TableInfo.major field from thrift struct

3 participants

@dlmarion @keith-turner @ctubbsii