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.

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

Fixes #24 - Automatically delete *.bin files after dump creation process #173

Closed
wants to merge 1 commit into from
Closed

Fixes #24 - Automatically delete *.bin files after dump creation process #173

wants to merge 1 commit into from
+111 −18

Conversation

mawiesne
Copy link
Contributor

@mawiesne mawiesne commented Jul 7, 2018 •
edited by reckart
Loading

  • Adds a "path/file deletion watch dog" which guarantees to remove files upon JVM shutdown to avoid left behind (intermediate) files
  • Adapts the SimpleBinaryDumpWriter implementation to register all created *.bin at the DeleteFilesAtShutdown monitor
  • Reduces CnP code by adding helper methods to SimpleBinaryDumpWriter

@daxenberger / @reckart Please review the changes in this branch I pushed to solve #24.

Note: This needs proper, manual testing as there isn't any JUnit test for this. This should - ideally - be tested on different OSes, i.e. Linux/Windows/MacOS and under OpenJDK/Oracle JDK 8.

Fixes #24 - Automatically delete *.bin files after dump creation process

@mawiesne mawiesne added this to the 1.2.0 milestone Jul 7, 2018
@ukp-svc-jenkins
Copy link

20% (+20.07%) vs master 0%

@reckart reckart changed the title Fixes #24 - Automatically delete *.bin files after dump creation process #24 - Automatically delete *.bin files after dump creation process Jul 8, 2018
@reckart
Copy link
Member

reckart commented Jul 8, 2018

Why not simply call File.deleteOnExit()?

@reckart
Copy link
Member

reckart commented Jul 8, 2018

I'm not even sure that DumpWriter should really clean up after itself. What if somebody really just wants to create these files and doesn't want them to get deleted at shutdown? I thing that DumpWriter should implement some clean-up method which the code that makes use of the DumpWriter can call at the end. Then the code which makes use of the DumpWriter should be changed to call this method after it does not need the data produced by the DumpWriter anymore.

@ukp-svc-jenkins
Copy link

21% (+21.0%) vs master 0%

daxenberger
daxenberger reviewed Jul 9, 2018
View reviewed changes
Copy link
Member

@daxenberger daxenberger left a comment

Choose a reason for hiding this comment

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

Thanks for attending to this. I can imagine only very few scenarios in which one would need the *.bin files after the dump creation - but I would also suggest to retain a possibility if explicitly requested. Either via the suggestion by @reckart, or via an additional flag/parameter.

@daxenberger daxenberger mentioned this pull request Jul 9, 2018
Closed
@mawiesne
Copy link
Contributor Author

mawiesne commented Jul 9, 2018

@daxenberger I agree: .bin files are not of much interest to most (>90%) users. Please specify an optional parameter in the original issue (#24) as a comment. I could then rework the PR to reflect this idea, so users can decide whether to keep the files or not.

@mawiesne mawiesne changed the title #24 - Automatically delete *.bin files after dump creation process Fixes #24 - Automatically delete *.bin files after dump creation process Jul 10, 2018
@ukp-svc-jenkins
Copy link

21% (+21.02%) vs master 0%

@mawiesne
Copy link
Contributor Author

Note: this is still in a WIP state. See comments/discussion in #24

@ukp-svc-jenkins
Copy link

21% (+21.04%) vs master 0%

@reckart reckart added the ⚠️WIP Work in progress. Do no merge. label Jul 17, 2018
@reckart
Copy link
Member

reckart commented Jul 17, 2018

I have added a WIP label and marked this PR with it.

@reckart reckart removed their request for review July 17, 2018 12:16
@mawiesne
Copy link
Contributor Author

thx, that's helpful for other cases as well.

@ukp-svc-jenkins
Copy link

23% (+0.04%) vs master 23%

@ukp-svc-jenkins
Copy link

23% (0.0%) vs master 23%

@ukp-svc-jenkins
Copy link

23% (0.0%) vs master 23%

@ukp-svc-jenkins
Copy link

24% (0.0%) vs master 24%

@tgalery
Copy link
Contributor

tgalery commented Oct 5, 2018

Any updates ?

@mawiesne
Copy link
Contributor Author

mawiesne commented Oct 7, 2018

@tgalery Vietnam..., lack of time. I'll try to make progress with this PR during the next 1 or 2 weeks.

@ukp-svc-jenkins
Copy link

24% (0.0%) vs master 24%

@mawiesne
#24 - Automatically delete *.bin files after dump creation process
a435d8b 
- Adds a "path/file deletion watch dog" which guarantees to remove files upon JVM shutdown to avoid left behind (intermediate) files
- Adapts the  `SimpleBinaryDumpWriter` implementation to register all created `*.bin` at the `DeleteFilesAtShutdown` monitor
- Reduces CnP code by adding helper methods to `SimpleBinaryDumpWriter`
@mawiesne mawiesne force-pushed the enhancement/24-delete-.bin-files-after-dump-creation branch from 99b8db8 to a435d8b Compare October 19, 2023 07:49
@mawiesne mawiesne closed this Oct 19, 2023
@reckart reckart deleted the enhancement/24-delete-.bin-files-after-dump-creation branch October 19, 2023 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daxenberger daxenberger daxenberger left review comments

@reckart reckart reckart left review comments

Assignees

@daxenberger daxenberger

Labels
🆕Enhancement Module: DataMachine ⚠️WIP Work in progress. Do no merge.
Projects
None yet
Milestone
1.2.0
Development

Successfully merging this pull request may close these issues.

[DataMachine] Automatically delete *.bin files after dump creation process
5 participants
@mawiesne @ukp-svc-jenkins @reckart @tgalery @daxenberger