Skip to content

Conversation

ArbaazKhan1
Copy link
Contributor

Added command option FindCompactionTmpFIles to admin commands.

closes issue #3986

@ArbaazKhan1 ArbaazKhan1 changed the base branch from main to elasticity April 16, 2024 21:35
@ArbaazKhan1 ArbaazKhan1 changed the title Accumulo 3986 Run FindCompactionTmpFiles utility via accumulo Admin Apr 16, 2024
@EdColeman
Copy link
Contributor

Should there be a check to see if the system is up or if there running compactions? If this was run on a system with active compactions, would it find and delete files that were currently being written to be the compaction process?

As a stand-alone utility it is a little harder to run the command - if elevated to an admin utility, would that increase the chances that it would be run on an active system?

@dlmarion
Copy link
Contributor

Should there be a check to see if the system is up or if there running compactions? If this was run on a system with active compactions, would it find and delete files that were currently being written to be the compaction process?

As a stand-alone utility it is a little harder to run the command - if elevated to an admin utility, would that increase the chances that it would be run on an active system?

FindCompactionTmpFiles first finds the tmp files, then removes from that set any file that is referenced from currently executing compactions.

Copy link
Contributor

@dlmarion dlmarion left a comment

Choose a reason for hiding this comment

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

This looks good. I think you may want to add some tests to FindCompactionTmpFilesIT to test it using the Admin pathway. You can look at DumpConfigIT an an example on how to call Admin and pass the parameters.

@ctubbsii ctubbsii added this to the 4.0.0 milestone Jul 12, 2024
@dlmarion dlmarion changed the base branch from elasticity to main August 26, 2024 12:16
@ctubbsii
Copy link
Member

ctubbsii commented Sep 4, 2024

May want to wait on merging this until after #4807 is merged, because I suspect there will be conflicts, or at least some related changes.

@kevinrr888
Copy link
Member

I remembered this was supposed to wait until #4807 was merged. #4807 has since been merged. I don't think tests have been added, maybe that needs to be addressed before this is merged

@kevinrr888
Copy link
Member

Any update on this? Think it just needs an IT

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Run FindCompactionTmpFiles utility via accumulo command line utility

5 participants